Re: [RFC PATCH] genirq: don't disable BH for PREEMPT_RT

[Crystal Wood]

On Mon, 2024-04-15 at 14:28 +0300, Vladimir Kondratiev wrote:
> With PREEMPT_RT, both BH and irq handled in threads.
> BH served by the ksoftirqd that is SCHED_OTHER, IRQ threads are
> SCHED_FIFO with some priority (default is 50).
> 
> On the test platform the following observed:
> 
> - ksoftirqd thread running, servicing softirqs
> - hard IRQ received by the CPU. Default handler wakes up IRQ thread
>   and exits
> - in the IRQ thread, force_irqthreads is defined as (true) for PREEMPT_RT
>   thus handler is irq_forced_thread_fn
> - before calling IRQ handler, irq_forced_thread_fn calls
>   local_bh_disable(); it in turn acquires local_lock(&softirq_ctrl.lock);
> - softirq_ctrl.lock still owned by the ksoftirqd thread, so
>   slow path taken (rt_spin_lock_slowlock), causing context switch to the
>   ksoftirqd (with properly adjusted priority) and waiting for it to
>   release the lock. Then, context switched back to the IRQ thread
> - as result, IRQ handler invoked with observed delay for several
>   hundreds micro-seconds on 2GHz platform.
> 
> This causes failure to deliver on real-time latency requirements.
> 
> What is the reason for disabling BH in the PREEMPT_RT? Looks like
> it is more reasonable to let scheduler to run threads according to
> priorities; IRQ thread will preempt BHs, run to its completion.
> 
> Experiment conducted with this approach, smooth execution observed with
> no spikes in the IRQ latency.
> 
> I am likely missing corner cases with this approach, so this is request
> for comments. Input welcome

I don't know if anything is actually depending on the mutual exclusion, but
disabling BH there causes any softirq raised by the irq to run in
local_bh_enable() rather than waking ksoftirqd.  It also appears to be the
only thing causing in_interrupt() to return true inside the threaded
handler, which may or may not actually matter.

-Crystal