Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: ifd894703:Fastmail
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\))
Subject: Re: [PATCH 2/2] irqchip/mips-gic: Use raw spinlock for gic_lock
From:   Jiaxun Yang <jiaxun.yang@flygoat.com>
In-Reply-To: <20230514135601.5irhslf6tdv4tk5z@mobilestation>
Date:   Sun, 14 May 2023 21:45:06 +0100
Cc:     Marc Zyngier <maz@kernel.org>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
        linux-kernel@vger.kernel.org,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <B793C674-2122-4464-A23D-1FDC450E9631@flygoat.com>
References: <20230424103156.66753-1-jiaxun.yang@flygoat.com>
 <20230424103156.66753-3-jiaxun.yang@flygoat.com>
 <20230514135601.5irhslf6tdv4tk5z@mobilestation>
To:     Serge Semin <fancer.lancer@gmail.com>
Precedence: bulk


> 2023=E5=B9=B45=E6=9C=8814=E6=97=A5 14:56=EF=BC=8CSerge Semin =
<fancer.lancer@gmail.com> =E5=86=99=E9=81=93=EF=BC=9A
>=20
> 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.
>>=20
>> Fixes BUG:
>>=20
>> [    0.426106] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> [    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
>>=20
>> Fixes: 95150ae8b330 ("irqchip: mips-gic: Implement irq_set_type =
callback")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>=20
> LGTM especially in the RT-patch context. Feel free to add:
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
>=20
> Please see a tiny nitpick below.
>=20
>> ---
>> drivers/irqchip/irq-mips-gic.c | 30 +++++++++++++++---------------
>> 1 file changed, 15 insertions(+), 15 deletions(-)
>>=20
>> 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;
>>=20
>> static DEFINE_PER_CPU_READ_MOSTLY(unsigned long[GIC_MAX_LONGS], =
pcpu_masks);
>>=20
>> -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)
>>=20
>> irq =3D GIC_HWIRQ_TO_SHARED(d->hwirq);
>>=20
>=20
>> - spin_lock_irqsave(&gic_lock, flags);
>> + raw_spin_lock_irqsave(&gic_lock, flags);
>=20
> 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=E2=80=99ll fix that in a new patch.

Thanks
- Jiaxun

>=20
> -Serge(y)
>=20