linux-kernel - Re: LED devices & poll() for brightness attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201702222216.32131@pali>
Date:   Wed, 22 Feb 2017 22:16:32 +0100
From:   Pali Rohár <pali.rohar@...il.com>
To:     Pavel Machek <pavel@....cz>
Cc:     Richard Purdie <rpurdie@...ys.net>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org,
        Darren Hart <dvhart@...radead.org>,
        platform-driver-x86@...r.kernel.org
Subject: Re: LED devices & poll() for brightness attribute

On Wednesday 22 February 2017 21:54:08 Pavel Machek wrote:
> On Wed 2017-02-22 13:39:29, Pali Rohár wrote:
> > On Wednesday 22 February 2017 13:25:57 Pavel Machek wrote:
> > > Hi!
> > > 
> > > > But this support is useful just for one single central
> > > > userspace application which will control all leds in system
> > > > other application which will change state by /sys/class/leds/
> > > > will de-synchronize that one central application.
> 
> Actually this is wrong.
> 
> > > Yes. Does it matter for some real use case?
> > 
> > Yes. E.g. systemd has already some support for changing leds. And I
> > do not want to be forced to use systemd (or any other specific
> > application) just because it already controls leds.
> 
> So... you don't want to use systemd, so don't use systemd. What is
> the problem? :-)
> 
> > And my own application for controlling leds should know current
> > state of them if it is possible.
> 
> If you have at most one application controlling each LED, you are
> fine. That's quite common situation, serial port can also be opened
> at most once. Do you actually have two applications accessing one
> led? If so, what applications?

First application which controls leds is integrated into desktop 
environment. In basic/default settings it do nothing automatically, just 
show GUI bar to user and allow user to adjust LED level.

Second one is basic shell script executed at specific situation (e.g. 
power adapter was unplugged) which changes LED level.

Now you have two independent applications which both could change LED 
level and at least one of them needs to know if another one changed 
level so can adjust GUI bar correctly.

I think this starts to be real situation for lot of users as more 
desktops (gnome, kde, ...) have already GUI bars for setting keyboard 
backlight and more users writes own scripts which at specific 
time/condition call echo > brightness.

Yes, if somebody uses GUI bar for manual keyboard backlight probably 
does not use led trigger or blinking feature. Probably that GUI desktop 
application could turn off them (do not know if kde/gnome application 
really turn off them).

> > > > So I think new ABI is not sufficient and I would propose to add
> > > > poll() support also for changes done by userspace, write() to
> > > > attribute /sys/class/leds/.../brightness.
> > > 
> > > Not easily possible, as we have triggers, and this was discussed
> > > in great great lengths before. Please go through that
> > > discussion.
> > 
> > Without triggers and blinking it should be possible.
> > 
> > Just notify only when some application do echo > brightness.
> 
> Yes, we _could_ do that. Would be slightly confusing
> w.r.t. triggers. But is it good idea?

Above situation for setting manual level of led is normal. And in this 
situation setting trigger or blink support does not make sense. 
Specially when user want to manually set led level.

I think it is good idea to provide some poll-able sysfs for this change. 
And if brightness is used for writing, then I would expect that 
brightness also provide poll() state.

Yes, it could be little confusing due to triggers/blinking support, but 
I think it is better as providing new sysfs file and it does not make 
this interface worse...

> Userspace could easily keep /var/run/led/XXX/brightness, and inotify
> works over regular files..

That would need to force all application to decide on this specific 
path. And looks bad as kernel already provides brightness attribute, why 
it should not also notify for changes on it?

> And yes, with three color LEDs, userspace daemon managing LED state
> will be pretty much mandatory...

Why it should be running daemon? Script which you start when you want 
change is also enough... no need to have permanently running process.

-- 
Pali Rohár
pali.rohar@...il.com

Download attachment "signature.asc " of type "application/pgp-signature" (199 bytes)