It turns out that various triggers use led_blink_setup() from atomic
context, so we can't do a flush_work there. Flush is still needed for
slow LEDs, but we can move it to sysfs code where it is safe.
WARNING: inconsistent lock state
5.2.0-rc1 #1 Tainted: G W
--------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/1/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
000000006e30541b
((work_completion)(&led_cdev->set_brightness_work)){+.?.}, at:
+__flush_work+0x3b/0x38a
{SOFTIRQ-ON-W} state was registered at:
lock_acquire+0x146/0x1a1
__flush_work+0x5b/0x38a
flush_work+0xb/0xd
led_blink_setup+0x1e/0xd3
led_blink_set+0x3f/0x44
tpt_trig_timer+0xdb/0x106
ieee80211_mod_tpt_led_trig+0xed/0x112
Fixes: 0db37915d912
Signed-off-by: Pavel Machek <[email protected]>
Tested-by: Hugh Dickins <[email protected]>
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index aefac4d..9f8da39 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -166,11 +166,6 @@ static void led_blink_setup(struct led_classdev *led_cdev,
unsigned long *delay_on,
unsigned long *delay_off)
{
- /*
- * If "set brightness to 0" is pending in workqueue, we don't
- * want that to be reordered after blink_set()
- */
- flush_work(&led_cdev->set_brightness_work);
if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
led_cdev->blink_set &&
!led_cdev->blink_set(led_cdev, delay_on, delay_off))
diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c
index ca898c1..427fc3c 100644
--- a/drivers/leds/trigger/ledtrig-timer.c
+++ b/drivers/leds/trigger/ledtrig-timer.c
@@ -113,6 +113,11 @@ static int timer_trig_activate(struct led_classdev *led_cdev)
led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
}
+ /*
+ * If "set brightness to 0" is pending in workqueue, we don't
+ * want that to be reordered after blink_set()
+ */
+ flush_work(&led_cdev->set_brightness_work);
led_blink_set(led_cdev, &led_cdev->blink_delay_on,
&led_cdev->blink_delay_off);
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
Thank you for the patch.
I've applied it however I'm not sure if it is an official
submission, since it doesn't look like (no [PATCH] tag
in the subject).
Beside that 'Fixes' tag is somewhat incomplete - it should be
generated using following git command:
git log -1 0db37915d912 --format='Fixes: %h ("%s")'
Fixed that and applied to the for-next branch and will push
it upstream after a bit of testing for rc3 or rc4.
Best regards,
Jacek Anaszewski
On 5/26/19 9:38 AM, Pavel Machek wrote:
>
> It turns out that various triggers use led_blink_setup() from atomic
> context, so we can't do a flush_work there. Flush is still needed for
> slow LEDs, but we can move it to sysfs code where it is safe.
>
> WARNING: inconsistent lock state
> 5.2.0-rc1 #1 Tainted: G W
> --------------------------------
> inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> swapper/1/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
> 000000006e30541b
> ((work_completion)(&led_cdev->set_brightness_work)){+.?.}, at:
> +__flush_work+0x3b/0x38a
> {SOFTIRQ-ON-W} state was registered at:
> lock_acquire+0x146/0x1a1
> __flush_work+0x5b/0x38a
> flush_work+0xb/0xd
> led_blink_setup+0x1e/0xd3
> led_blink_set+0x3f/0x44
> tpt_trig_timer+0xdb/0x106
> ieee80211_mod_tpt_led_trig+0xed/0x112
>
> Fixes: 0db37915d912
> Signed-off-by: Pavel Machek <[email protected]>
> Tested-by: Hugh Dickins <[email protected]>
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index aefac4d..9f8da39 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -166,11 +166,6 @@ static void led_blink_setup(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off)
> {
> - /*
> - * If "set brightness to 0" is pending in workqueue, we don't
> - * want that to be reordered after blink_set()
> - */
> - flush_work(&led_cdev->set_brightness_work);
> if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
> led_cdev->blink_set &&
> !led_cdev->blink_set(led_cdev, delay_on, delay_off))
> diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c
> index ca898c1..427fc3c 100644
> --- a/drivers/leds/trigger/ledtrig-timer.c
> +++ b/drivers/leds/trigger/ledtrig-timer.c
> @@ -113,6 +113,11 @@ static int timer_trig_activate(struct led_classdev *led_cdev)
> led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
> }
>
> + /*
> + * If "set brightness to 0" is pending in workqueue, we don't
> + * want that to be reordered after blink_set()
> + */
> + flush_work(&led_cdev->set_brightness_work);
> led_blink_set(led_cdev, &led_cdev->blink_delay_on,
> &led_cdev->blink_delay_off);
>
>
Hi!
> Thank you for the patch.
>
> I've applied it however I'm not sure if it is an official
> submission, since it doesn't look like (no [PATCH] tag
> in the subject).
It was official submission :-).
> Beside that 'Fixes' tag is somewhat incomplete - it should be
> generated using following git command:
>
> git log -1 0db37915d912 --format='Fixes: %h ("%s")'
>
> Fixed that and applied to the for-next branch and will push
> it upstream after a bit of testing for rc3 or rc4.
Ok. Note that this did not crash Hugh's machine but it may crash
someone else's, and probably crashed mine already.
So... it makes sense to push it to Linus "soon".
Thanks,
Pavel
On Sun, 26 May 2019, Pavel Machek wrote:
> Hi!
>
> > Thank you for the patch.
> >
> > I've applied it however I'm not sure if it is an official
> > submission, since it doesn't look like (no [PATCH] tag
> > in the subject).
>
> It was official submission :-).
>
> > Beside that 'Fixes' tag is somewhat incomplete - it should be
> > generated using following git command:
> >
> > git log -1 0db37915d912 --format='Fixes: %h ("%s")'
> >
> > Fixed that and applied to the for-next branch and will push
> > it upstream after a bit of testing for rc3 or rc4.
>
> Ok. Note that this did not crash Hugh's machine but it may crash
> someone else's, and probably crashed mine already.
Where "this" is commit 0db37915d912, I hope: right, it did not
actually crash my machine, but the splurge of warning messages
forced me to reboot quickly, and go look for the culprit.
Your fix certainly did not crash my machine either,
but I hope we don't expect your fix to crash someone else's!
>
> So... it makes sense to push it to Linus "soon".
Agreed.
Hugh