Re: [PATCH v3 3/3] genirq: Use the maple tree for IRQ descriptors management

[Shanker Donthineni]

Hi Thomas & Marc,


Apologies for my lack of familiarity with the maple tree data structure and
not testing all functions. I received advice from the review comments below
regarding the iterator. I am looking for your guidance to address the issue
with the iterator if not possible can we increase NR_IRQS for ARM64 arch.


https://lore.kernel.org/linux-arm-kernel/875ydej9ur.ffs@tglx/

static unsigned int irq_find_next_irq(unsigned int offset)
{
        MA_STATE(mas, &sparse_irqs, offset, nr_irqs);
        struct irq_desc *desc = mas_next(&mas, nr_irqs);

        return desc ? irq_desc_get_irq(desc) : nr_irqs;
}

-Shanker

On 5/10/23 09:49, Thomas Gleixner wrote:
> External email: Use caution opening links or attachments
>
>
> Shanker!
>
> On Wed, May 10 2023 at 16:41, Thomas Gleixner wrote:
> > On Wed, May 10 2023 at 15:24, Yujie Liu wrote:
> > I decoded it by now and that maple_tree conversion is the culprit. It
> > broke irq_get_next_irq() which is used during hotplug. It misses every
> > other interrupt, so affinities are not fixed up.
>
> I'm seriously grumpy. You throw that untested stuff over the fence,
> pester me about merging it and then ignore the fallout.
>
> This breaks cpuhotplug, debugfs, /proc/stat, x86/IOAPIC and some more.
>
> It's not asked too much that if you change an iterator implementation to
> validate that the outcome is still the same on the usage sites.
>
> That change has never seen CPU hotplug testing. It reproduces
> instantaneously in a VM even without running blktest.
>
> I grant you that the documentation of mt_next() is incorrect, but that's
> absolutely no excuse for not testing such a fundamental change at
> all. It's neither an excuse for ignoring the fallout and wasting other
> peoples time.
>
> I'm dropping this from my to-merge list.
>
> Yours grumpy
>
>        Thomas