Messages in this thread | | | Subject | Re: [irqchip: irq/irqchip-next] irqdomain: Protect the linear revmap with RCU | From | Guenter Roeck <> | Date | Mon, 5 Jul 2021 13:36:36 -0700 |
| |
On 7/5/21 11:43 AM, Marc Zyngier wrote: > On Mon, 05 Jul 2021 19:23:27 +0100, > Guenter Roeck <linux@roeck-us.net> wrote: >> >> Hi Marc, >> >> On 7/5/21 11:01 AM, Marc Zyngier wrote: >>> Hi Guenter, >>> >>> On Mon, 05 Jul 2021 18:23:52 +0100, >>> Guenter Roeck <linux@roeck-us.net> wrote: >>>> >>>> Hi, >>>> >>>> On Fri, Jun 11, 2021 at 01:54:36PM -0000, irqchip-bot for Marc Zyngier wrote: >>>>> The following commit has been merged into the irq/irqchip-next branch of irqchip: >>>>> >>>>> Commit-ID: d4a45c68dc81f9117ceaff9f058d5fae674181b9 >>>>> Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9 >>>>> Author: Marc Zyngier <maz@kernel.org> >>>>> AuthorDate: Mon, 05 Apr 2021 12:57:27 +01:00 >>>>> Committer: Marc Zyngier <maz@kernel.org> >>>>> CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00 >>>>> >>>>> irqdomain: Protect the linear revmap with RCU >>>>> >>>>> It is pretty odd that the radix tree uses RCU while the linear >>>>> portion doesn't, leading to potential surprises for the users, >>>>> depending on how the irqdomain has been created. >>>>> >>>>> Fix this by moving the update of the linear revmap under >>>>> the mutex, and the lookup under the RCU read-side lock. >>>>> >>>>> The mutex name is updated to reflect that it doesn't only >>>>> cover the radix-tree anymore. >>>>> >>>>> Signed-off-by: Marc Zyngier <maz@kernel.org> >>>> >>>> This patch results in various RCU warnings when booting mipsel images >>>> in qemu. I can not revert the patch due to subsequent changes, so I >>>> don't know if a simple revert fixes the problem. Log messages and >>>> bisect log see below. >>> >>> Thanks for the heads up. Do you have a config file I can use to >>> reproduce this? The QEMU invocation runes would certainly help too. >>> >>> It strikes me that in drivers/irqchip/irq-mips-cpu.c, >>> plat_irq_dispatch() now uses the irqdomain resolution before >>> irq_enter() took place. That's certainly a latent bug. I'll fix that >>> regardless, but I'd like to make sure this is what you are seeing too. >>> >> >> See http://server.roeck-us.net/qemu/mipsel/ >> >> config Complete configuration file >> defconfig Shortened configuration file >> rootfs.cpio root file system (initrd) >> run.sh qemu run script (tested with qemu 4.2.1 and 6.0.0) >> vmlinux Kernel image experiencing the problem (v5.13-9883-gaf9efb8b661) >> >> Hope this helps, > > It definitely helps, and confirms my hunch. With the patch below, I'm > not getting the warnings anymore. I'm pretty sure a number of other > MIPS systems suffer from similar issues, which I'll address similarly. > > Please let me know if that addresses the issue on your end. >
Yes, it does. Feel free to add
Tested-by: Guenter Roeck <linux@roeck-us.net>
to the real patch.
Now the big question: Why does this only affect 32-bit little endian mips images, but not the matching big endian images, nor 64-bit images ?
Thanks, Guenter
> Thanks, > > M. > > diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h > index d1477ecb1af9..57561e0e6e8d 100644 > --- a/arch/mips/include/asm/irq.h > +++ b/arch/mips/include/asm/irq.h > @@ -57,6 +57,9 @@ asmlinkage void plat_irq_dispatch(void); > > extern void do_IRQ(unsigned int irq); > > +struct irq_domain; > +extern void do_domain_IRQ(struct irq_domain *domain, unsigned int irq); > + > extern void arch_init_irq(void); > extern void spurious_interrupt(void); > > diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c > index 85b6c60f285d..c76005cd3b79 100644 > --- a/arch/mips/kernel/irq.c > +++ b/arch/mips/kernel/irq.c > @@ -21,6 +21,7 @@ > #include <linux/kallsyms.h> > #include <linux/kgdb.h> > #include <linux/ftrace.h> > +#include <linux/irqdomain.h> > > #include <linux/atomic.h> > #include <linux/uaccess.h> > @@ -107,3 +108,16 @@ void __irq_entry do_IRQ(unsigned int irq) > irq_exit(); > } > > +void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq) > +{ > + struct irq_desc *desc; > + > + irq_enter(); > + check_stack_overflow(); > + > + desc = irq_resolve_mapping(domain, hwirq); > + if (likely(desc)) > + handle_irq_desc(desc); > + > + irq_exit(); > +} > diff --git a/drivers/irqchip/irq-mips-cpu.c b/drivers/irqchip/irq-mips-cpu.c > index 0bbb0b2d0dd5..0c7ae71a0af0 100644 > --- a/drivers/irqchip/irq-mips-cpu.c > +++ b/drivers/irqchip/irq-mips-cpu.c > @@ -127,7 +127,6 @@ static struct irq_chip mips_mt_cpu_irq_controller = { > asmlinkage void __weak plat_irq_dispatch(void) > { > unsigned long pending = read_c0_cause() & read_c0_status() & ST0_IM; > - unsigned int virq; > int irq; > > if (!pending) { > @@ -137,12 +136,15 @@ asmlinkage void __weak plat_irq_dispatch(void) > > pending >>= CAUSEB_IP; > while (pending) { > + struct irq_domain *d; > + > irq = fls(pending) - 1; > if (IS_ENABLED(CONFIG_GENERIC_IRQ_IPI) && irq < 2) > - virq = irq_linear_revmap(ipi_domain, irq); > + d = ipi_domain; > else > - virq = irq_linear_revmap(irq_domain, irq); > - do_IRQ(virq); > + d = irq_domain; > + > + do_domain_IRQ(d, irq); > pending &= ~BIT(irq); > } > } >
| |