On Sun, 24 Nov 2013 01:53:11 -0200 Henrique de Moraes Holschuh <hmh@xxxxxxxxxx> wrote: > On Sun, 24 Nov 2013, Matthew Garrett wrote: > > On Sat, Nov 23, 2013 at 10:40:15PM -0200, Henrique de Moraes Holschuh wrote: > > > On Fri, 22 Nov 2013, Matthew Garrett wrote: > > > > We have userspace that relies on uevents of type > > > > BACKLIGHT_UPDATE_HOTKEY. I don't know that we have userspace that relies > > > > on uevents of type BACKLIGHT_UPDATE_SYSFS. > > > > > > Any OSD application would have to rely on both uevent types, or it is broken > > > (and to test that, just write a level to sysfs and watch the OSD app fail to > > > tell you about the backlight level change...) > > > > Right, OSDs are supposed to respond to keypresses, not arbitrary changes > > of backlight. If the user's just echoed 8 into brightness, they know > > they set the brightness to 8 - they don't need an OSD to tell them that. > > It is not just the user that sets the brightness. > > Still, if you're sure that all userspace users react only to the hotkey type > of event, removing the sysfs one won't break anything any further. > > But it will be *really* annoying the day we revisit this because someone > started abusing the hotkey uevent and we have to deploy a proper fix (rate > limiting or switching to a proper event report interface that doesn't use > uevents). > > > BACKLIGHT_UPDATE_HOTKEY is when the firmware itself has changed the > > brightness in response to a keypress, and so reporting the keypress > > would result in additional backlight changes. > > Yeah, I know that bug quite well, thinkpads were the first victims of > idiotic feedback event loops caused by braindead userspace. I'm not seeing a lot of consensus here and afaict the v2 patch: --- a/drivers/video/backlight/backlight.c~drivers-video-backlight-backlightc-remove-backlight-sysfs-uevent +++ a/drivers/video/backlight/backlight.c @@ -175,8 +175,6 @@ static ssize_t brightness_store(struct d } mutex_unlock(&bd->ops_lock); - backlight_generate_event(bd, BACKLIGHT_UPDATE_SYSFS); - return rc; } static DEVICE_ATTR_RW(brightness); will still break userspace which relies on BACKLIGHT_UPDATE_SYSFS uevents. I see no way we can guarantee that there is no such userspace so the patch is worrying. Should we instead be looking for a way of avoiding this risk? Say, add a new knob which people can set if they don't want to generate this event? Ugly, but that's the price we pay for mucking it up originally. ------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk _______________________________________________ ibm-acpi-devel mailing list ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
