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

[Sebastian Andrzej Siewior]

On 2025-10-27 13:59:31 [+0100], Lukas Wunner wrote:
> Crystal reports that the PCIe Advanced Error Reporting driver gets stuck
> in an infinite loop on PREEMPT_RT:
> 
> Both the primary interrupt 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 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 because the primary handler
> runs in hardirq context and thus can preempt the threaded secondary
> handler, clear the Root Error Status register and prevent the secondary
> handler from getting stuck.

Not sure if I mentioned it before but this is due to forced threaded
IRQs which can also be enabled on non-PREEMPT_RT systems via `threadirqs`.

> Emulate the same behavior on PREEMPT_RT by assigning a lower default
> priority to the secondary handler if the primary handler is forced into
> a thread.
> 
> 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>
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -856,6 +856,19 @@ void sched_set_fifo_low(struct task_struct *p)
>  }
>  EXPORT_SYMBOL_GPL(sched_set_fifo_low);
>  
> +/*
> + * For when the primary interrupt handler is forced into a thread, in addition
> + * to the (always threaded) secondary handler.  The secondary handler gets a
> + * slightly lower priority so that the primary handler can preempt it, thereby
> + * emulating the behavior of a non-PREEMPT_RT system where the primary handler
> + * runs in hardirq context.

s/non-PREEMPT_RT/non-forced threaded/ ?

Other than that,
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

Sebastian