* Linus Torvalds <torva...@linux-foundation.org> [170711 08:40]:
> On Tue, Jul 11, 2017 at 7:41 AM, Thomas Gleixner <t...@linutronix.de> wrote:
> >
> > Ah. Now that makes sense.
> >
> > Unpatched the ordering is:
> >
> > chip_bus_lock(desc);
> > irq_request_resources(desc);
>
> I *looked* at that ordering and then went "Naah, that makes no sense".
>
> But if that's the only issue, how about we just re-order those things
> - we still don't need to move the irq_request_resources() into the
> spinlock, we just move it to below the chip_bus_lock().
>
> IOW, something like the (COMPLETELY UNTEESTED!) attached patch.
Yeah that fixes the issue:
Tested-by: Tony Lindgren <t...@atomide.com>
> This assumes that the chip_bus_lock() thing is still ok for the RT
> case, but it looks like it might be: the only other one I looked at
> (apart from the gpio-omap one) used a mutex.
Yeah and the ordering below makes more sense to me at least. That is
assuming we want to call chip_bus_lock() before we start calling the
chip functions :)
Regards,
Tony
> kernel/irq/manage.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 5624b2dd6b58..ea1b9404c041 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1168,17 +1168,17 @@ __setup_irq(unsigned int irq, struct irq_desc *desc,
> struct irqaction *new)
> new->flags &= ~IRQF_ONESHOT;
>
> mutex_lock(&desc->request_mutex);
> + chip_bus_lock(desc);
> +
> if (!desc->action) {
> ret = irq_request_resources(desc);
> if (ret) {
> pr_err("Failed to request resources for %s (irq %d) on
> irqchip %s\n",
> new->name, irq, desc->irq_data.chip->name);
> - goto out_mutex;
> + goto out_unlock_chip_bus;
> }
> }
>
> - chip_bus_lock(desc);
> -
> /*
> * The following block of code has to be executed atomically
> */
> @@ -1385,12 +1385,11 @@ __setup_irq(unsigned int irq, struct irq_desc *desc,
> struct irqaction *new)
> out_unlock:
> raw_spin_unlock_irqrestore(&desc->lock, flags);
>
> - chip_bus_sync_unlock(desc);
> -
> if (!desc->action)
> irq_release_resources(desc);
>
> -out_mutex:
> +out_unlock_chip_bus:
> + chip_bus_sync_unlock(desc);
> mutex_unlock(&desc->request_mutex);
>
> out_thread:
