From patchwork Mon Dec 16 07:53:36 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 3352171 Return-Path: X-Original-To: patchwork-linux-fbdev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 181449F32E for ; Mon, 16 Dec 2013 07:52:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0DC262035E for ; Mon, 16 Dec 2013 07:52:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 10A0B2031C for ; Mon, 16 Dec 2013 07:52:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751359Ab3LPHwE (ORCPT ); Mon, 16 Dec 2013 02:52:04 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:39409 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751329Ab3LPHwC (ORCPT ); Mon, 16 Dec 2013 02:52:02 -0500 Received: from localhost (c-24-7-24-226.hsd1.ca.comcast.net [24.7.24.226]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id 16DDD711; Mon, 16 Dec 2013 07:52:01 +0000 (UTC) Date: Sun, 15 Dec 2013 23:53:36 -0800 From: Andrew Morton To: Henrique de Moraes Holschuh Cc: Matthew Garrett , linux-fbdev@vger.kernel.org, "'Kyungmin Park'" , kay@vrfy.org, Jingoo Han , "'Henrique de Moraes Holschuh'" , linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org, ibm-acpi-devel@lists.sourceforge.net, "'Richard Purdie'" Subject: Re: [ibm-acpi-devel] [PATCH] video: backlight: Remove backlight sysfs uevent Message-Id: <20131215235336.49d49a25.akpm@linux-foundation.org> In-Reply-To: <20131124035311.GC19499@khazad-dum.debian.net> References: <20131111235700.GA29987@july> <002901cedf3c$a7e77a00$f7b66e00$%han@samsung.com> <20131112005628.GA2914@khazad-dum.debian.net> <1384990859.20536.4.camel@x230> <20131121114332.GA23710@khazad-dum.debian.net> <20131121143326.GA19773@srcf.ucam.org> <20131122113601.GB27196@khazad-dum.debian.net> <20131122171556.GA15680@srcf.ucam.org> <20131124004015.GA19499@khazad-dum.debian.net> <20131124010257.GA6376@srcf.ucam.org> <20131124035311.GC19499@khazad-dum.debian.net> X-Mailer: Sylpheed 2.7.1 (GTK+ 2.18.9; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sun, 24 Nov 2013 01:53:11 -0200 Henrique de Moraes Holschuh 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: 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. --- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- 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);