Re: [GIT pull] irq updates for 4.13

[Tony Lindgren]

* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> [170711 08:40]:
> On Tue, Jul 11, 2017 at 7:41 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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 <tony@xxxxxxxxxxx>

> 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: