Messages in this thread | | | Date | Sat, 21 Jul 2012 09:13:37 -0700 | From | Greg KH <> | Subject | Re: sysfs permissions on dynamic attributes (led delay_on and delay_off) |
| |
On Sat, Jul 21, 2012 at 08:42:12AM -0700, Colin Cross wrote: > On Sat, Jul 21, 2012 at 4:21 AM, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > On Sat, 2012-07-21 at 01:26 -0700, Colin Cross wrote: > >> On Sat, Jul 21, 2012 at 12:33 AM, Richard Purdie > >> <richard.purdie@linuxfoundation.org> wrote: > >> > On Fri, 2012-07-20 at 21:08 -0700, Greg KH wrote: > >> >> On Fri, Jul 20, 2012 at 05:46:14PM -0700, Colin Cross wrote: > >> >> > I'm trying to use the standard ledtrig-timer.c code to handle led > >> >> > blinking for notifications on an Android device, and I'm hitting some > >> >> > issues with setting permissions on the dynamically created delay_on > >> >> > and delay_off attributes. For most sysfs files, we have userspace > >> >> > uevent parser that watches for device add notifications and > >> >> > chowns/chmods attributes. This doesn't work for delay_on and > >> >> > delay_off, because they are created later, when "timer" is written to > >> >> > the trigger attribute. There is no uevent when the new files are > >> >> > created, and sysfs doesn't support inotify, so I don't see any way to > >> >> > receive an event to set the permissions. This issue exists any time > >> >> > that device_create_file is called after device_add. > >> >> > > >> >> > What is the appropriate way to get an event to set the permissions? > >> >> > Add inotify support for sysfs file creation? Send a KOBJ_CHANGE > >> >> > uevent in device_create_file? > >> >> > >> >> No. > >> >> > >> >> > Send a KOBJ_CHANGE uevent from the driver after calling > >> >> > device_create_file? > >> >> > >> >> Yes. > >> >> > >> >> > Dynamically create a timer device under /sys/class/leds/<led> so a new > >> >> > add uevent gets sent? > >> >> > >> >> Ick. > >> >> > >> >> > Promote blinking to be a core led feature instead of a trigger, so the > >> >> > files are always present? > >> >> > >> >> That's the best thing, why not just do that? > >> > > >> > This implies we should make every trigger a core led feature and > >> > effectively do away with triggers. I'm not sure that makes sense. > >> > >> Blinking is already effectively a core feature. It is implemented in > >> led-core.c so it can be used by other triggers besides timer, it's > >> state is stored in the led_classdev structure, not in the trigger > >> data, and the only thing left in ledtrig-timer.c is the sysfs files. > > > > Having the attributes present all the time leads to some nasty questions > > like how the on/off delays interact with things like say a network > > activity trigger. Not all triggers are going to respect these delay > > values and I can imagine a whole new set of nasty bug reports with no > > easy solutions if this change is made... > > The delay_on and delay_off files could easily override the values from > the trigger. > > Sending a KOBJ_CHANGE uevent is not a great solution, it's still > horribly racy in userspace. This script would never work reliably: > echo timer > trigger
When this returned, the sysfs files would then be there, right?
> echo 1000 > delay_on > echo 1000 > delay_off > echo 255 > brightness
So this would work.
What is racy here?
greg k-h
| |