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

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 10 May 2023 10:19:58 -0500
From:   Shanker Donthineni <sdonthineni@...dia.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Yujie Liu <yujie.liu@...el.com>, oe-lkp@...ts.linux.dev,
        lkp@...el.com, linux-kernel@...r.kernel.org,
        Marc Zyngier <maz@...nel.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Michael Walle <michael@...le.cc>,
        Vikram Sethi <vsethi@...dia.com>
Subject: Re: [PATCH v3 3/3] genirq: Use the maple tree for IRQ descriptors
 management


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