On Wed, Nov 25, 2020 at 03:15:18PM +0100, Andrea Righi wrote:
...
> > I'd hate to see this in stable 3 days after Linus merges it...
> >
> > Do these need _irqsave, too?
> >
> > drivers/leds/led-triggers.c: read_lock(&trig->leddev_list_lock);
> > drivers/leds/led-triggers.c: read_unlock(&trig->leddev_list_lock);
> > drivers/leds/led-triggers.c: read_lock(&trig->leddev_list_lock);
> > drivers/leds/led-triggers.c: read_unlock(&trig->leddev_list_lock);
> >
> > Best regards,
>
> I think also led_trigger_blink_setup() needs to use irqsave/irqrestore,
> in fact:
>
> $ git grep "led_trigger_blink("
> drivers/leds/led-triggers.c:void led_trigger_blink(struct led_trigger *trig,
> drivers/power/supply/power_supply_leds.c:
> led_trigger_blink(psy->charging_blink_full_solid_trig,
> include/linux/leds.h:void led_trigger_blink(struct led_trigger *trigger,
> unsigned long *delay_on,
> include/linux/leds.h:static inline void led_trigger_blink(struct led_trigger
> *trigger,
>
> power_supply_leds.c is using led_trigger_blink() from a workqueue
> context, so potentially the same deadlock condition can also happen.
>
> Let me know if you want me to send a new patch to include also this
> case.
Just sent (and tested) a v2 of this patch that changes also
led_trigger_blink_setup().
-Andrea
