Re: [PATCH] genirq/manage: Reduce priority of forced secondary IRQ handler

[Sebastian Andrzej Siewior]

On 2025-09-08 18:08:31 [+0200], Lukas Wunner wrote:
> Crystal reports the PCIe Advanced Error Reporting driver getting stuck
> in an infinite loop on PREEMPT_RT:  Both the primary IRQ handler
> aer_irq() as well as the secondary handler aer_isr() are forced into
> threads with identical priority.
> 
> Crystal writes that on the ARM system in question, the primary IRQ
> handler has to clear an error in the Root Error Status register...
> 
>    "before the next error happens, or else the hardware will set the
>     Multiple ERR_COR Received bit.  If that bit is set, then aer_isr()
>     can't rely on the Error Source Identification register, so it scans
>     through all devices looking for errors -- and for some reason, on
>     this system, accessing the AER registers (or any Config Space above
>     0x400, even though there are capabilities located there) generates
>     an Unsupported Request Error (but returns valid data).  Since this
>     happens more than once, without aer_irq() preempting, it causes
>     another multi error and we get stuck in a loop."
> 
> The issue does not show on non-PREEMPT_RT since the primary IRQ handler
> runs in hardirq context and thus clears the Root Error Status register
> before waking the secondary IRQ handler's thread.
> 
> Simulate the same behavior on PREEMPT_RT by assigning a lower default
> priority to forced secondary IRQ handlers.
> 
> Reported-by: Crystal Wood <crwood@xxxxxxxxxx>
> Tested-by: Crystal Wood <crwood@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/20250902224441.368483-1-crwood@xxxxxxxxxx/
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

> Crystal provided a Tested-by off-list.  I've used Crystal's suggestion
> for the code comment, but if anyone prefers Sebastian's or some other
> wording, I don't mind either respinning myself or a maintainer amending
> this or any other aspect of the patch when applying.  Thanks!

Crystal's wording ist fine.

Sebastian