Re: [PATCH 0/6] genirq/test: Platform/architecture fixes

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi David,

On Thu, 21 Aug 2025 at 05:45, David Gow <davidgow@google.com> wrote:
> On Thu, 21 Aug 2025 at 01:22, Brian Norris <briannorris@chromium.org> wrote:
> > On Wed, Aug 20, 2025 at 03:00:34PM +0800, David Gow wrote:
> > > Looks like __irq_alloc_descs() is returning -ENOMEM (as
> > > irq_find_free_area() is returning 200 w/ nr_irqs == 200, and
> > > CONFIG_SPARSE_IRQ=n).
> >
> > Thanks for the insight. I bothered compiling my own qemu just so I can
> > run m68k this time, and I can reproduce.
> >
> > I wonder if I should make everything (CONFIG_IRQ_KUNIT_TEST) depend on
> > CONFIG_SPARSE_IRQ, since it seems like arches like m68k can't enable
> > SPARSE_IRQ, and they can't allocate new (fake) IRQs without it. That'd
> > be a tweak to patch 4.
> >
> > Or maybe just 'depends on !M68K', since architectures with higher
> > NR_IRQS headroom may still work even without SPARSE_IRQ.
>
> I'm not an m68k expert (so I've CCed Geert), but I think different
> m68k configs do have different NR_IRQS, so it's possible there are
> working m68k setups, too. (It also seems slightly suspicious to me
> that exactly 200 IRQs are allocated here, though, so a lack of extra
> headroom may be deliberate and/or triggered by something trying to
> allocate all IRQs.)
>
> Personally, I don't have any m68k machines lying around, so disabling
> the test so my qemu scripts don't report errors is fine by me. Ideally
> the dependency would be as narrow as possible, but that may well be
> !M68K.

M68k indeed has different values of NR_IRQS, based on the system(s)
support is enabled for.  These values are based on the IRQ hierarchy
of the system(s), which is rather fixed.  Hence this does not take
into account any additional irqchips that are being registered by
e.g. tests...

"git grep -w NR_IRQS -- arch/*/include/" shows m68k is not the only
architecture having that limitation...

> The other option would be to try to skip the test if there aren't free
> IRQs, but maybe that'd hide real issues?
>
> Regardless, I'll defer to the IRQ and m68k experts here: as long as
> I'm not seeing errors, I'm happy. :-)

kernel/irq/irqdesc.c:

    static bool irq_expand_nr_irqs(unsigned int nr)
    {
            if (nr > MAX_SPARSE_IRQS)
                    return false;
            nr_irqs = nr;
            return true;
    }

kernel/irq/internals.h:

    #ifdef CONFIG_SPARSE_IRQ
    # define MAX_SPARSE_IRQS        INT_MAX
    #else
    # define MAX_SPARSE_IRQS        NR_IRQS
    #endif

So probably the test should depend on SPARSE_IRQ?  Increasing NR_IRQS
everywhere when IRQ_KUNIT_TEST is enabled sounds rather invasive to me.

BTW, given the test calls irq_domain_alloc_descs(), I think it should
also depend on IRQ_DOMAIN.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds