linux-kernel - Re: [PATCH] irq: Request and release resources for chained IRQs

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]

Date:   Fri, 7 Dec 2018 14:46:20 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Linus Walleij <linus.walleij@...aro.org>
cc:     Marc Zyngier <marc.zyngier@....com>,
        Jason Cooper <jason@...edaemon.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Hans Verkuil <hverkuil@...all.nl>
Subject: Re: [PATCH] irq: Request and release resources for chained IRQs

Linus,

On Fri, 7 Dec 2018, Linus Walleij wrote:
> On Fri, Dec 7, 2018 at 12:52 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> > Needs more thought. Btw, the uninstall path does not invoke irq_deactive()
> > either.... I so hate that chained handler mess....
> 
> I think it is just extremely uncommon to remove a chained handler
> (I don't know if anything exercises that path even) that is why we
> don't see any fallout from it. It's probably just untested code.

git grep irq_set_chained.*NULL

tells that there are users. Whether any of this is ever invoked is a
different questions.

> Do you see that chained handlers have any merit at all or should
> they all be moved to nested? The question needs asking, but IIUC
> there are performance benefits with chaining as opposed to
> nesting. :/

The nested case in gpio land is about nesting in irq thread context, which
is obviously slower. So instead of nesting you can just use a regular
interrupt for demultiplexing.

The thing about chained is:

  1) It's hidden, i.e. not exposed in /proc/interrupt

  2) It's not protected against runaway interrupts, which has happened in
     x86 land. We've converted those over to use regular interrupts to
     catch that case and shutdown the offending interrupt.
     e.g. ba714a9c1dea

  3) All the chained handlers have their own flow control inside the
     handler.

The chained performance benefit compared to a regular handler is
minimal. The main difference are a few conditionals and the desc->action
indirection. You probably can measure it with a high interrupt load, but
I'm still not convinced that it outweighs the downsides for a lot of the
places which use chained interrupts. I can see the benefit for very low
level interrupts, but even there we had hard to debug issues with some ARM
SoCs which ran into interrupt storms.

But hey, we surely can fix that chained issue. It's not going to be pretty
though :)

Thanks,

	tglx