From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751684Ab2GUQNn (ORCPT ); Sat, 21 Jul 2012 12:13:43 -0400 Received: from mail-pb0-f46.google.com ([209.85.160.46]:61287 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751396Ab2GUQNm (ORCPT ); Sat, 21 Jul 2012 12:13:42 -0400 Date: Sat, 21 Jul 2012 09:13:37 -0700 From: Greg KH To: Colin Cross Cc: Richard Purdie , lkml , Bryan Wu Subject: Re: sysfs permissions on dynamic attributes (led delay_on and delay_off) Message-ID: <20120721161337.GA22896@kroah.com> References: <20120721040816.GA7313@kroah.com> <1342856010.21788.47.camel@ted> <1342869707.21788.50.camel@ted> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jul 21, 2012 at 08:42:12AM -0700, Colin Cross wrote: > On Sat, Jul 21, 2012 at 4:21 AM, Richard Purdie > wrote: > > On Sat, 2012-07-21 at 01:26 -0700, Colin Cross wrote: > >> On Sat, Jul 21, 2012 at 12:33 AM, Richard Purdie > >> 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/ 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