linux-kernel - Re: [PATCH 2/2] irqchip/mips-gic: Use raw spinlock for gic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <B793C674-2122-4464-A23D-1FDC450E9631@flygoat.com>
Date:   Sun, 14 May 2023 21:45:06 +0100
From:   Jiaxun Yang <jiaxun.yang@...goat.com>
To:     Serge Semin <fancer.lancer@...il.com>
Cc:     Marc Zyngier <maz@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        linux-kernel@...r.kernel.org,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH 2/2] irqchip/mips-gic: Use raw spinlock for gic_lock



> 2023年5月14日 14:56，Serge Semin <fancer.lancer@...il.com> 写道：
> 
> On Mon, Apr 24, 2023 at 11:31:56AM +0100, Jiaxun Yang wrote:
>> Since we may hold gic_lock in hardirq context, use raw spinlock
>> makes more sense given that it is for low-level interrupt handling
>> routine and the critical section is small.
>> 
>> Fixes BUG:
>> 
>> [    0.426106] =============================
>> [    0.426257] [ BUG: Invalid wait context ]
>> [    0.426422] 6.3.0-rc7-next-20230421-dirty #54 Not tainted
>> [    0.426638] -----------------------------
>> [    0.426766] swapper/0/1 is trying to lock:
>> [    0.426954] ffffffff8104e7b8 (gic_lock){....}-{3:3}, at: gic_set_type+0x30/08
>> 
>> Fixes: 95150ae8b330 ("irqchip: mips-gic: Implement irq_set_type callback")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@...goat.com>
> 
> LGTM especially in the RT-patch context. Feel free to add:
> Reviewed-by: Serge Semin <fancer.lancer@...il.com>
> 
> Please see a tiny nitpick below.
> 
>> ---
>> drivers/irqchip/irq-mips-gic.c | 30 +++++++++++++++---------------
>> 1 file changed, 15 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
>> index b568d55ef7c5..6d5ecc10a870 100644
>> --- a/drivers/irqchip/irq-mips-gic.c
>> +++ b/drivers/irqchip/irq-mips-gic.c
>> @@ -50,7 +50,7 @@ void __iomem *mips_gic_base;
>> 
>> static DEFINE_PER_CPU_READ_MOSTLY(unsigned long[GIC_MAX_LONGS], pcpu_masks);
>> 
>> -static DEFINE_SPINLOCK(gic_lock);
>> +static DEFINE_RAW_SPINLOCK(gic_lock);
>> static struct irq_domain *gic_irq_domain;
>> static int gic_shared_intrs;
>> static unsigned int gic_cpu_pin;
>> @@ -210,7 +210,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
>> 
>> irq = GIC_HWIRQ_TO_SHARED(d->hwirq);
>> 
> 
>> - spin_lock_irqsave(&gic_lock, flags);
>> + raw_spin_lock_irqsave(&gic_lock, flags);
> 
> AFAICS this call can be moved way down to be after the switch-case
> block.

Thanks for the suggestion :-)

Since it actually reduced critical section I think it should not be included in this patch which
Cced stable.

I’ll fix that in a new patch.

Thanks
- Jiaxun

> 
> -Serge(y)
>