linux-kernel - Re: [RFC PATCH] genirq: don't disable BH for PREEMPT

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: Tue, 30 Apr 2024 16:26:41 -0500
From: Crystal Wood <crwood@...hat.com>
To: Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>, Thomas Gleixner
	 <tglx@...utronix.de>
Cc: Vincent Whitchurch <vincent.whitchurch@...s.com>, "Rafael J . Wysocki"
	 <rafael.j.wysocki@...el.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Andreas Gruenbacher
	 <agruenba@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] genirq: don't disable BH for PREEMPT_RT

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