Re: [PATCH] video: backlight: Remove backlight sysfs uevent

[Kyungmin Park]

On Tue, Nov 12, 2013 at 10:19 AM, Kay Sievers <kay@xxxxxxxx> wrote:
> On Tue, Nov 12, 2013 at 1:56 AM, Henrique de Moraes Holschuh
> <hmh@xxxxxxxxxx> wrote:
>> On Tue, 12 Nov 2013, Jingoo Han wrote:
>>> On Tuesday, November 12, 2013 8:57 AM, Kyungmin Park wrote:
>>> > From: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>>> >
>>> > The most mobile phones have Ambient Light Sensors and it changes brightness according lux.
>>> > It means it changes backlight brightness frequently by just writing sysfs node, so it generates uevent.
>>> >
>>> > Usually there's no user to use this backlight changes. But it forks udev worker threads and it takes
>>> > about 5ms. The main problem is that it hurts other process activities. so remove it.
>>> >
>>> > Kay said
>>> > "Uevents are for the major, low-frequent, global device state-changes,
>>> >  not for carrying-out any sort of measurement data. Subsystems which
>>> >  need that should use other facilities like poll()-able sysfs file or
>>> >  any other subscription-based, client-tracking interface which does not
>>> >  cause overhead if it isn't used. Uevents are not the right thing to
>>> >  use here, and upstream udev should not paper-over broken kernel
>>> >  subsystems."
>>
>> True.
>>
>> Now, let's take a look at reality: should you poll()/select() on a sysfs
>> node that doesn't suport it, it will wait until the poll/select timeout
>> happens (or EINTR happens), and userspace has absolutely NO way to detect
>> whether a sysfs node has poll/select support.
>>
>> What happens if the sysfs interface did not provide poll/select support
>> since day one, but rather added it later?  Nobody will use it for a *long*
>> time, if ever... unless you actually took pains to version the sysfs
>> interface, and people actually care.
>
> If that's an issue, we can add a new "event" file, just for that.
>
>>> 'thinkpad_acpi.c' uses the 'BACKLIGHT_UPDATE_SYSFS'.
>>> Henrique, can we remove it?
>>
>> Can't you fix this by rate-limiting, or otherwise adding an attribute that
>> backlight devices should set when they need to supress change events?
>
> Yeah, great idea, fix a bad hack with another bad one on top. :)
> Passing measurement data through uevents is just an utterly broken
> idea which cannot be fixed.
>
>> Is there a proper on-screen-display support path for the backlight class
>> nowadays?  Otherwise, you'd be removing the only way userspace ever had to
>> do proper OSD of backlight changes...
>
> OSD drawing and event sounds usually happen as a fedback for
> keypresses of brightness control, it would be weird to show up when
> something else, like a light-sensor, adjusts the brightness in the
> background.
>
> Anyway, there might be the need for coordination and a new interface,
> but uevents for measurement data need to die entirely; they make no
> sense, never made any; and the sooner they are gone the better.

Now power_supply, especially battery uses this scheme. it passes
battery data using uevent.
do you have any idea to kill it?

Thank you,
Kyungmin Park
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/