Re: [PATCHSET 4/4] sysfs: implement new features
On Sep 25, 2007, at 18:50:05, Greg KH wrote: On Thu, Sep 20, 2007 at 05:31:37PM +0900, Tejun Heo wrote: * 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. IMHO this would be nicer if it could reuse existing sprintf code to handle all the nice shiny sprintf format specifiers. The only challenge would be how to dynamically build a varargs list from an array of component names although perhaps there could be an internal __csprintf function which took a callback for retrieving arguments. Also since all of the path components are strings I don't know that numeric specifiers could be made useful, so perhaps it's not the greatest idea. I think the primary importance for this functionality is: * 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. Cheers, Kyle Moffett - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET 4/4] sysfs: implement new features
Hello, Greg.
Please read the other reply first. We need some consensus there first.
Greg KH wrote:
>> * Notify pollers on file deactivation.
>
> This looks nice.
Cool.
>> * 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.
This is a package deal with the autorenaming. More on this later.
>> * 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.
To rename symlink when its target or one of its ancestors is renamed or
moved, sysfs should be able to determine what should be the new name of
the symlink when it points to the new target, right? So, we need
symlink name formatting to do this.
I think the implemented formatting scheme covers all symlinks. Symlink
name formatting will also simplify callers. No need to allocate name,
format and free it. It can simply use constant string.
>> * 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?
Yes, we can. This is the way to do it with the new interface. More on
this below.
>> * 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.
Plugged creation + batch error handling is a way to group operations and
commit them atomically under the new interface. The problem of visible
partial creation / removal is mostly solved using
device->uevent_suppress but I think things can be improved for both
userland and kernel.
After uevent is moved into sysfs, uevent generation can be coupled with
grouped commit. sysfs hierarchy and accompanying uevents will appear
atomically to userland and in-kernel users end up with simpler and less
buggy code - single error check at the end of all sysfs operations
instead of piece-by-piece construction coupled with the same amount of
unroll code which is usually buggy. Maybe in (long) time, sysfs access
can be simplified for embedded systems or whatnot.
I don't really see much downside other than the added complexity and as
the added complexity will reduce complexity in the users, I think it's a
good trade off.
Thanks.
--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET 4/4] sysfs: implement new features
Hello, Greg.
Please read the other reply first. We need some consensus there first.
Greg KH wrote:
* Notify pollers on file deactivation.
This looks nice.
Cool.
* 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.
This is a package deal with the autorenaming. More on this later.
* 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.
To rename symlink when its target or one of its ancestors is renamed or
moved, sysfs should be able to determine what should be the new name of
the symlink when it points to the new target, right? So, we need
symlink name formatting to do this.
I think the implemented formatting scheme covers all symlinks. Symlink
name formatting will also simplify callers. No need to allocate name,
format and free it. It can simply use constant string.
* 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?
Yes, we can. This is the way to do it with the new interface. More on
this below.
* 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.
Plugged creation + batch error handling is a way to group operations and
commit them atomically under the new interface. The problem of visible
partial creation / removal is mostly solved using
device-uevent_suppress but I think things can be improved for both
userland and kernel.
After uevent is moved into sysfs, uevent generation can be coupled with
grouped commit. sysfs hierarchy and accompanying uevents will appear
atomically to userland and in-kernel users end up with simpler and less
buggy code - single error check at the end of all sysfs operations
instead of piece-by-piece construction coupled with the same amount of
unroll code which is usually buggy. Maybe in (long) time, sysfs access
can be simplified for embedded systems or whatnot.
I don't really see much downside other than the added complexity and as
the added complexity will reduce complexity in the users, I think it's a
good trade off.
Thanks.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET 4/4] sysfs: implement new features
On Sep 25, 2007, at 18:50:05, Greg KH wrote: On Thu, Sep 20, 2007 at 05:31:37PM +0900, Tejun Heo wrote: * 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. IMHO this would be nicer if it could reuse existing sprintf code to handle all the nice shiny sprintf format specifiers. The only challenge would be how to dynamically build a varargs list from an array of component names although perhaps there could be an internal __csprintf function which took a callback for retrieving arguments. Also since all of the path components are strings I don't know that numeric specifiers could be made useful, so perhaps it's not the greatest idea. I think the primary importance for this functionality is: * 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. Cheers, Kyle Moffett - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET 4/4] sysfs: implement new features
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET 4/4] sysfs: implement new features
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHSET 4/4] sysfs: implement new features
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.
* 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.
* Autoremoval of symlinks when target is removed. This only applies
when new interface is used.
* 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.
* 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.
* 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).
This patchset contains the following 8 patches.
0001-sysfs-notify-file-on-deactivation.patch
0002-sysfs-add-name-formatting-support-for-symlinks.patch
0003-sysfs-chain-symlinks-to-their-targets.patch
0004-sysfs-implement-symlink-auto-removal.patch
0005-sysfs-implement-symlink-auto-rename.patch
0006-sysfs-implement-plugged-creation-of-sysfs-nodes.patch
0007-sysfs-implement-batch-error-handling.patch
0008-sysfs-add-copyrights.patch
0001 implements notification on file deactivation. 0002-0005
implement symlink name formatting, autoremoving and autorenaming.
0006 implements plugged creation. 0007 batch error handling, and 0008
adds copyright notices on top of all sysfs header and source files.
Thanks.
--
tejun
[1] http://thread.gmane.org/gmane.linux.kernel/582105
[2] http://thread.gmane.org/gmane.linux.kernel/582147
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHSET 4/4] sysfs: implement new features
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.
* 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.
* Autoremoval of symlinks when target is removed. This only applies
when new interface is used.
* 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.
* 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.
* 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).
This patchset contains the following 8 patches.
0001-sysfs-notify-file-on-deactivation.patch
0002-sysfs-add-name-formatting-support-for-symlinks.patch
0003-sysfs-chain-symlinks-to-their-targets.patch
0004-sysfs-implement-symlink-auto-removal.patch
0005-sysfs-implement-symlink-auto-rename.patch
0006-sysfs-implement-plugged-creation-of-sysfs-nodes.patch
0007-sysfs-implement-batch-error-handling.patch
0008-sysfs-add-copyrights.patch
0001 implements notification on file deactivation. 0002-0005
implement symlink name formatting, autoremoving and autorenaming.
0006 implements plugged creation. 0007 batch error handling, and 0008
adds copyright notices on top of all sysfs header and source files.
Thanks.
--
tejun
[1] http://thread.gmane.org/gmane.linux.kernel/582105
[2] http://thread.gmane.org/gmane.linux.kernel/582147
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
