Re: Right way to configure a driver? (sysfs, ioctl, proc, configfs,....)

[Neil Brown <neilb@suse.de>]

On Wednesday February 1, greg@kroah.com wrote:
> On Wed, Feb 01, 2006 at 03:54:22PM +0100, Jan Engelhardt wrote:
> > >> 
> > >> I guess I could pass three values on the same file, like this:
> > >> $ echo "5  1000  500" > meminfo
> > >> 
> > >> I know that breaks the sysfs golden-rule, but how can I pass those
> > >> values _atomically_ then? Having three different files wouldn't be
> > >> atomic...
> > >
> > >That's what configfs was created for.  I suggest using that for things
> > >like this, as sysfs is not intended for it.
> > >
> > Can't we just somewhat merge all the duplicated functionality between procfs,
> > sysfs and configfs...
> 
> What "duplicated functionality"?  They all do different, unique
> things.

So why do you recommend configfs for something that is *almost* what
sysfs does well.

sysfs both exports information, and allows changes to some (not all)
of that information.
But as soon as someone wants an atomic change, which is conceptually a
very small difference, you say "use configfs" which is conceptually a
very big difference in interface.

Configfs - as I read the doco - is not really about providing generic
atomic configuration changes.

Configfs is for *Creating* kernel objects.
The basic sequence is:
  mkdir /config/subsystem/objectname
     # where you choose 'objectname' to be whatever you want.
  echo value > /config/subsystem/objectname/param1
  echo value > /config/subsystem/objectname/param2
  echo value > /config/subsystem/objectname/param3
  echo value > /config/subsystem/objectname/param4

and then the object is ready to go.
Notice that there is *NO* 'commit' step.  There is nothing here that
makes anything atomic.

So saying 'use configfs for atomic updates' doesn't seem rational...

To be fair, configfs is meant to have 'committable items', but they are
'currently unimplemented'.
If you have a 'committable' item, then the sequence instead would go
something like:

   mkdir /config/subsystem/pending/objectname
   echo value > /config/subsystem/pending/objectname/param1
   ...
   mv /config/subsystem/pending/objectname /config/subsystem/live

This does provide atomic updates but, apart from not being
implemented, it only allows atomic updates at object creation time.

If I have a live object, and I want to change some attributes
atomically, configfs DOES NOT LET ME DO THAT.

Conversely, it is quite easy to do this with sysfs.
As you have control over the 'read' and 'write' routines for each
attribute, you simply:
  - in your object, store the real attribute and a 'pending' copy.
  - define a special attribute, maybe called 'commit' such that:
     writing 'clear'  copies the real attributes in to the pending
        copy as well
     writing 'commit' copies the 'pending' copies into the real
        attributes atomically
  - when you write to an attribute, it updates the 'pending' copy.

So to do a atomic update, you:

 1/ get a flock lock on the directory (do sysfs directories support
    flock?)
 2/ write 'clear' to 'commit
 3/ make your changes
 4/ write 'commit' to 'commit
 5/ unflock.

Obvious the 'flock' could be replaced by lockfiles in /tmp or whatever
you want.

This doesn't mean that we don't need configfs (though I'm not yet
convinced).  The point of configfs seems to be *Creating* objects.
Maybe it is a good thing to use for this purpose (though if those
objects end up appearing in sysfs, it would seem like unnecessary
duplication). 
 
> 
> Patches are always welcome...
> 

True, patches are good.
But they don't stop people from recommending the wrong tool for the
job :-)
And they aren't needed to support atomic updates in sysfs.

Maybe what would be good is support for 'mkdir' in sysfs.
I would really like to be able to use 'mkdir' to create md devices,
but '/sys/block' is too flat.  If it had
  /sys/block/sd/scsi-block-devices
  /sys/block/hd/ide-block-devices
  /sys/block/loop/loop-block-devices
it would also have
  /sys/block/md/md-block-devices
and it would make sense to do a 
  mkdir /sys/block/md/0
or whatever to create a new md device.  But I don't think it makes
sense to 
  mkdir /sys/block/md0
because someone would have to parse the device name ('md0') to decide
which module to hand it off to.... oh well :-(

NeilBrown

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/