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] [day] [month] [year] [list]

Message-ID: <87lehwxapp.ffs@tglx>
Date:   Wed, 10 May 2023 21:12:02 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Shanker Donthineni <sdonthineni@...dia.com>
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

Shanker!

On Wed, May 10 2023 at 10:19, Shanker Donthineni wrote:
> 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.
>
> 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;
> }

You gracefully omitted what I wrote below this:

    "or something like that."

which means that it's a suggestion and not claiming that it is in any
way correct.

It does not mean that you should copy it verbatim and assume that it is
correct. It means that if you pick that suggestion up it's your
responsibility to thoroughly understand and test the code you submit,
no?

That has nothing to do with familiarity vs. the maple tree. I'm neither
familiar with the maple tree and all I did was reading through the
documentation, which is pretty well done, except for that tiny
documentation error vs. the return value of mt_next().

It's a simple question of engineering principles:

     "correctness first" vs. "works for me".

> I am looking for your guidance to address the issue with the iterator

You must be kidding.

  1) You got 90% of the solution on a silber tablet

  2) You failed to understand and test the stuff you submitted

  3) You ignored the bug report. Someone else debugged the fallout and
     explained where and what the problem is.

  4) Now you ask that someone else goes and reads the maple tree
     documentation for you to find the proper function to use.  So that
     you can go and blindly apply the change and then go back to #2.

I'm really happy to help and guide people, but what you are asking for
is that I do your homework completely. Seriously?

> if not possible can we increase NR_IRQS for ARM64 arch.

Definitely not. The maple tree conversion is the right thing to do and
fixing the issue at hand is definitely not rocket science.

I'm pretty sure that it is about 3 lines of change to make it work. Plus
some polishing afterwards to make it palatable.

Thanks,

        tglx