linux-kernel - Re: [PATCH] irq: fasteoi handler re-runs on concurrent invoke

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: <f9cb8c3f546cf9aa6422e478ec56422b92264c8c.camel@amazon.com>
Date:   Tue, 30 May 2023 21:47:57 +0000
From:   "Gowans, James" <jgowans@...zon.com>
To:     "maz@...nel.org" <maz@...nel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "liaochang1@...wei.com" <liaochang1@...wei.com>
CC:     "zouyipeng@...wei.com" <zouyipeng@...wei.com>,
        "Raslan, KarimAllah" <karahmed@...zon.com>,
        "Woodhouse, David" <dwmw@...zon.co.uk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Sironi, Filippo" <sironi@...zon.de>,
        "chris.zjh@...wei.com" <chris.zjh@...wei.com>
Subject: Re: [PATCH] irq: fasteoi handler re-runs on concurrent invoke

On Mon, 2023-05-29 at 10:47 +0800, Liao, Chang wrote:
> 
> > 
> > Wording may need a slight tweak, especially pointing out why PENDING is
> > set.
> 
> Sure, see the comment below:
> 
> "Ack another interrupt from the same source can occurs on new CPU even before
> the first one is handled on original CPU, IRQS_PENDING bit can be reused to
> indicate this situation, which will defer the execution of the interrupt handler
> function associated with the irq_desc until the first interrupt handler returns."
> 
> In summary, the IRQ_PENDINGS ensures that only one interrupt handler is ever
> running for a particular source at a time, and the major usages of IRQS_PENDING
> in kernel as follows:
> 
> 1. Used in irq flow handler to indicate that an acknowledged interrupt cannot be
> handled immediately due to three different reasons:
> - Case1: the interrupt handler function has been unregistered via free_irq().
> - Case2: the interrupt has been disabled via irq_disable().
> - Case3: the interrupt is an edge-triggered interrupt and its handler is already
>          running on the CPU.
> 
> In any of these cases, the kernel will defer the execution of the interrupt handler
> until the interrupt is enabled and new handler is established again via check_irq_resend(),
> or via the inside loop in handle_edge_irq() upon the previous handler returns.
> 
> 2. Used in the spurious interrupt detector, a few systems with misdescribed IRQ
> routing can cause an interrupt to be handled on the wrong CPU. In this situation,
> the spurious interrupt detector searches for a recovery handler for the interrupt.
> If the found handler is running on another CPU, the spurious interrupt detector
> also defers the execution of the recovery handler, similar to case 3 in #1.
> 
> I hope this is helpful.

Stunning! I tried to pillage some of this for the commit message, but I
think it was too much detail for this commit message - it probably belongs
in a doc or code comment somewhere though. Useful!

> > If you want I'm happy to make these tweaks and post as V2?
> > There are also some other comments I'm keen to add to the flag enums to
> > make it a bit clearer what some of the flags mean.
> 
> Don't see why not :)

Done, V2: https://lore.kernel.org/lkml/20230530213848.3273006-2-jgowans@amazon.com/

JG