Re: [PATCHSET 4/4] sysfs: implement new features

[Greg KH]

On Thu, Sep 20, 2007 at 05:31:37PM +0900, Tejun Heo wrote:
> Hello, all.
> 
> This is the fourth patchset of four sysfs update patchset series[1]
> and to be applied on top of the third patchset[2].
> 
> This patchset implements the following new features.
> 
> * Notify pollers on file deactivation.

This looks nice.

> * Name-formatting for symlinks.  e.g. symlink pointing to
>   /dira/dirb/leaf can be named as "symlink:%1-%0" and it will show up
>   as "symlink:dirb-leaf".  This only applies when new interface is
>   used.

Is this really necessary?  It looks like we are adding a "special" type
of parser here that no one uses.

> * Autoremoval of symlinks when target is removed.  This only applies
>   when new interface is used.

Nice.

> * Autorenaming of symlinks according to the name format string when
>   target or one of its ancestors is renamed or moved.  This only
>   applies when new interface is used.

Nice.

> * Plugged operations.  Sysfs users can plug top node and build subtree
>   gradually without revealing the process to userland.  When subtree
>   is fully constructed, the top node can be unplugged and userland
>   will see completely built subtree appearing at once.  If subtree
>   creation fails in the process, the whole subtree can be removed by
>   simply removing the top node.  There won't be any userland
>   noticeable event.  This is to be combined with uevent_suppress
>   mechanism of driver model.

Hm, but why?  Can't we do this today with the attribute groups?

> * Batch error handling.  A plugged node accumulates any error
>   condition occurring below it and can return the first error when
>   asked.  Also, all interface functions accepth ERR_PTR() value as
>   sysfs_dirent parameter.  This means that constructs like the
>   following can be used to replace the current group interface.
> 
>   <<-- code -->>
>   group = sysfs_add_dir(parent, "group_name", 0777 | SYSFS_PLUGGED, NULL);
>   sysfs_add_file(group, "file0", 0777, file0_ops, file0_data);
>   sysfs_add_file(group, "file1", 0777, file1_ops, file1_data);
>   ...
>   sysfs_add_file(group, "fileN", 0777, fileN_ops, fileN_data);
>   rc = sysfs_check_batch_error(group);
>   if (rc) {
> 	sysfs_remove(group);
> 	return rc;
>   }
>   sysfs_unplug(group);
>   return 0;
>   <<-- end of code -->>
> 
>   The above will create a subdirectory "group_name" which contains N
>   files and show them atomically to userland or remove them without
>   letting userland notice if any failure happens.  This will simplify
>   sysfs users quite a bit (not only for groups, other stuff too).

I'm still not sold on why this is needed.  It looks like a lot of extra
work for something that we are already handling.

thanks,

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