Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 25 Nov 2020 16:20:42 +0100
From:   Andrea Righi <andrea.righi@canonical.com>
To:     Pavel Machek <pavel@ucw.cz>
Cc:     Boqun Feng <boqun.feng@gmail.com>, Dan Murphy <dmurphy@ti.com>,
        linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] leds: trigger: fix potential deadlock with libata
Message-ID: <20201125152042.GC73489@xps-13-7390>
References: <20201102104152.GG9930@xps-13-7390>
 <20201125124648.GJ29328@amd>
 <20201125141517.GA73489@xps-13-7390>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201125141517.GA73489@xps-13-7390>
Precedence: bulk

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