lkml.org 
[lkml]   [2016]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] leds: Add mutex protection in brightness_show()
Hi!

> > Thanks for the analysis. Either way, this patch, with the modification
> > I mentioned in my previous message is required to assure proper
> > LED sysfs locking.
> >
> > Regarding the races between user and atomic context, I think that
> > it should be system root's responsibility to define LED access
> > policy. If a LED is registered for any trigger events then setting
> > brightness from user space should be made impossible. Such a hint
> > could be even added to the Documentation/leds/leds-class.txt.
>
> No, kernel locking may not depend on "root did not do anything
> stupid". Sorry.
>
> Is there problem with led_access becoming a spinlock, and
> brightness_set_blocking taking it internally while it reads the
> brightness (but not while sleeping)?

led_timer_function() does not seem to have any locking. IMO it needs
some, as it does not use atomic accesses.

Do we need a spinlock protecting led_classdev.flags and
delayed_set_value?

Would it be good idea to create new "blink_cancel" so brightness_set
is used just for .. brightness and not for timer manipulations?

Should we do something like this for consistency?

BTW how is locking expected to work with userland LED drivers? What if
userland LED driver accesses /sys files for its own LED? I'd really
prefer that patch not to be merged until we get locking right.

Thanks,
Pavel

diff --git a/include/linux/leds.h b/include/linux/leds.h
index ddfcb2d..60e436d 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -88,11 +88,11 @@ struct led_classdev {

unsigned long blink_delay_on, blink_delay_off;
struct timer_list blink_timer;
- int blink_brightness;
+ enum led_brightness blink_brightness;
void (*flash_resume)(struct led_classdev *led_cdev);

struct work_struct set_brightness_work;
- int delayed_set_value;
+ enum led_brightness delayed_set_value;

#ifdef CONFIG_LEDS_TRIGGERS
/* Protects the trigger data below */
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2016-11-09 21:26    [W:1.728 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site