linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Samuel Thibault <samuel.thibault@ens-lyon.org>
To: Hugh Dickins <hughd@google.com>,
	akpm@linux-foundation.org, Sabrina Dubroca <sd@queasysnail.net>,
	Valdis.Kletnieks@vt.edu, Vincent Donnefort <vdonnefort@gmail.com>,
	Bryan Wu <cooloney@gmail.com>,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-wireless@vger.kernel.org
Subject: [PATCHv2][input-led] Defer input led work to workqueue
Date: Tue, 26 Aug 2014 11:17:25 +0200	[thread overview]
Message-ID: <20140826091725.GA23293@type.youpi.perso.aquilenet.fr> (raw)
In-Reply-To: <20140826015453.GA5235@type.youpi.perso.aquilenet.fr>

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 <samuel.thibault@ens-lyon.org>

---

Difference with v1: uses schedule_work instead of its own workqueue.

--- 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 <samuel.thibault@ens-lyon.org>");
+module_init(input_led_init);
+module_exit(input_led_exit);

  parent reply	other threads:[~2014-08-26  9:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-26  1:54 [PATCH][input-led] Defer input led work to workqueue Samuel Thibault
2014-08-26  8:01 ` Johannes Berg
2014-08-26  9:13   ` Samuel Thibault
     [not found]   ` <1409040095.2489.3.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>
2014-08-26 18:51     ` Valdis.Kletnieks-PjAqaU27lzQ
2014-08-26  9:17 ` Samuel Thibault [this message]
2014-08-26 13:22   ` [PATCHv2][input-led] " Johannes Berg
2014-08-26 13:24     ` Samuel Thibault
     [not found]       ` <20140826132410.GD3186-hVaJJPvgRkSHR6PIDP45TBlftxrVp6Uy@public.gmane.org>
2014-08-26 13:32         ` Samuel Thibault
     [not found]   ` <20140826091725.GA23293-ImhJuBFkDxsZRTKc5xlRkz/bP2T7CorvwZZFX4Cs8Hg@public.gmane.org>
2014-08-26 20:41     ` Valdis.Kletnieks-PjAqaU27lzQ
2014-08-26 18:49 ` [PATCH][input-led] " Valdis.Kletnieks
2014-08-26 20:02   ` Sabrina Dubroca
2014-08-26 21:55     ` Sabrina Dubroca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140826091725.GA23293@type.youpi.perso.aquilenet.fr \
    --to=samuel.thibault@ens-lyon.org \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=akpm@linux-foundation.org \
    --cc=cooloney@gmail.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=sd@queasysnail.net \
    --cc=vdonnefort@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).