Route keyboard LEDs through the generic LEDs layer.

Commit Message

Samuel Thibault Oct. 1, 2014, 9:29 p.m. UTC

Andrew Morton, le Wed 01 Oct 2014 11:42:57 -0700, a écrit :
> On Mon, 31 Mar 2014 14:23:23 +0200 Samuel Thibault <[e-mail]> wrote:
> > This permits to reassign keyboard LEDs to something else than keyboard "leds"
> > state, by adding keyboard led and modifier triggers connected to a series
> > of VT input LEDs, themselves connected to VT input triggers, which
> > per-input device LEDs use by default.  Userland can thus easily change the LED
> > behavior of (a priori) all input devices, or of particular input devices.
> > 
> > This also permits to fix #7063 from userland by using a modifier to implement
> > proper CapsLock behavior and have the keyboard caps lock led show that modifier
> > state.
> 
> When this patch is combined with current linux-next I'm getting the
> below lockdep splat.  Config: http://ozlabs.org/~akpm/config-akpm2.txt

Yes, this was reported and I sent a fix some time ago (Tue, 26 Aug 2014
11:17:25 +0200), here is the patch again:

Subject: Defer input led work to workqueue

When the kbd changes its led state (e.g. caps lock), this triggers
(led_trigger_event) the kbd-capsl trigger, which is by default
used by the vt::capsl LED, which triggers (led_trigger_event) the
vt-capsl trigger. These two nested led_trigger_event calls take a
trig->leddev_list_lock lock and thus lockdep complains.

Actually the user can make the vt::capsl LED use its own vt-capsl
trigger and thus build a loop.  This produces an immediate oops.

This changeset defers the second led_trigger_event call into a
workqueue, which avoids the nested locking altogether.  This does
not prevent the user from shooting himself in the foot by creating a
vt::capsl <-> vt-capsl loop, but the only consequence is the workqueue
threads eating some CPU until the user breaks the loop, which is not too
bad.

Signed-off-by: Samuel Thibault <[e-mail]>

Comments

Pali Rohár Oct. 12, 2014, 2:52 p.m. UTC | #1

Hello,

I would like to ask if something was changed and if this patch 
(in any way) is going to mainline kernel.

Samuel Thibault Oct. 15, 2014, 11:15 p.m. UTC | #2

Pali Rohár, le Sun 12 Oct 2014 16:52:08 +0200, a écrit :
> I would like to ask if something was changed and if this patch 
> (in any way) is going to mainline kernel.

Well, the latest news I got from Dmity was on 12 Apr 2014, to which I
answered, and never got more news since then.  I'm indeed starting to
wonder whether it'll ever get applied at all, after the several years it
has been waiting.  It's no use trying to contribute to free software if
review/discussion etc. doesn't happen.

Samuel

Pali Rohár Oct. 15, 2014, 11:29 p.m. UTC | #3

On Thursday 16 October 2014 01:15:45 Samuel Thibault wrote:
> Pali Rohár, le Sun 12 Oct 2014 16:52:08 +0200, a écrit :
> > I would like to ask if something was changed and if this
> > patch (in any way) is going to mainline kernel.
> 
> Well, the latest news I got from Dmity was on 12 Apr 2014, to
> which I answered, and never got more news since then.  I'm
> indeed starting to wonder whether it'll ever get applied at
> all, after the several years it has been waiting.  It's no
> use trying to contribute to free software if
> review/discussion etc. doesn't happen.
> 
> Samuel

Dmitry, ping!
https://lkml.org/lkml/2014/4/16/725

There are more people who would like to see this patch in 
mainline kernel and we are waiting for you.

Pali Rohár Dec. 9, 2014, 5:12 p.m. UTC | #4

On Thursday 16 October 2014 01:29:21 Pali Rohár wrote:
> On Thursday 16 October 2014 01:15:45 Samuel Thibault wrote:
> > Pali Rohár, le Sun 12 Oct 2014 16:52:08 +0200, a écrit :
> > > I would like to ask if something was changed and if this
> > > patch (in any way) is going to mainline kernel.
> > 
> > Well, the latest news I got from Dmity was on 12 Apr 2014,
> > to which I answered, and never got more news since then. 
> > I'm indeed starting to wonder whether it'll ever get
> > applied at all, after the several years it has been
> > waiting.  It's no use trying to contribute to free software
> > if
> > review/discussion etc. doesn't happen.
> > 
> > Samuel
> 
> Dmitry, ping!
> https://lkml.org/lkml/2014/4/16/725
> 
> There are more people who would like to see this patch in
> mainline kernel and we are waiting for you.

Dmitry: ping!

I would like you to remind you this patch which is waiting for 
your review since April 2014!

Peter Korsgaard Dec. 9, 2014, 8:22 p.m. UTC | #5

>>>>> "Pali" == Pali Rohár <[e-mail]> writes:

Hi,

>> There are more people who would like to see this patch in
 >> mainline kernel and we are waiting for you.

 > Dmitry: ping!

 > I would like you to remind you this patch which is waiting for 
 > your review since April 2014!

Hah, I'm pretty sure it dates back to 2010 atleast.

Samuel Thibault Dec. 10, 2014, 1:03 a.m. UTC | #6

Peter Korsgaard, le Tue 09 Dec 2014 21:22:03 +0100, a écrit :
>  > I would like you to remind you this patch which is waiting for 
>  > your review since April 2014!
> 
> Hah, I'm pretty sure it dates back to 2010 atleast.

Yes, the first version was posted on February 2010.  There has been some
reviews, and thus a few versions, I've just posted a 4th version which
has been sitting in -mm for some time now.

Samuel

Patch

--- a/drivers/input/leds.c
+++ b/drivers/input/leds.c
@@ -100,13 +100,24 @@  static unsigned long vt_led_registered[B
 /* Number of input devices having each LED */
 static int vt_led_references[LED_CNT];
 
+static int vt_led_state[LED_CNT];
+static struct work_struct vt_led_work[LED_CNT];
+
+static void vt_led_cb(struct work_struct *work)
+{
+	int led = work - vt_led_work;
+
+	led_trigger_event(&vt_led_triggers[led], vt_led_state[led]);
+}
+
 /* VT LED state change, tell the VT trigger.  */
 static void vt_led_set(struct led_classdev *cdev,
 			  enum led_brightness brightness)
 {
 	int led = cdev - vt_leds;
 
-	led_trigger_event(&vt_led_triggers[led], !!brightness);
+	vt_led_state[led] = !!brightness;
+	schedule_work(&vt_led_work[led]);
 }
 
 /* LED state change for some keyboard, notify that keyboard.  */
@@ -244,6 +255,22 @@  void input_led_disconnect(struct input_d
 	mutex_unlock(&vt_led_registered_lock);
 }
 
+static int __init input_led_init(void)
+{
+	unsigned i;
+
+	for (i = 0; i < LED_CNT; i++)
+		INIT_WORK(&vt_led_work[i], vt_led_cb);
+
+	return 0;
+}
+
+static void __exit input_led_exit(void)
+{
+}
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("User LED support for input layer");
 MODULE_AUTHOR("Samuel Thibault <[e-mail]>");
+module_init(input_led_init);
+module_exit(input_led_exit);