From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933759AbdGKQPZ (ORCPT ); Tue, 11 Jul 2017 12:15:25 -0400 Received: from muru.com ([72.249.23.125]:54154 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933344AbdGKQPY (ORCPT ); Tue, 11 Jul 2017 12:15:24 -0400 Date: Tue, 11 Jul 2017 09:15:20 -0700 From: Tony Lindgren To: Linus Torvalds Cc: Thomas Gleixner , Sebastian Reichel , LKML , Andrew Morton , Ingo Molnar , "H. Peter Anvin" , Pavel Machek , Linus Walleij , Grygorii Strashko Subject: Re: [GIT pull] irq updates for 4.13 Message-ID: <20170711161519.GY3730@atomide.com> References: <20170710133505.eo6w73kq2327n34p@earth> <20170711135131.GW3730@atomide.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Linus Torvalds [170711 08:40]: > On Tue, Jul 11, 2017 at 7:41 AM, Thomas Gleixner 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 > 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: