linux-kernel - Re: [PATCH] softirq: Provide a handshake for canceling tasklets via polling on 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250812145359.QMcaYh9g@linutronix.de>
Date: Tue, 12 Aug 2025 16:53:59 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Tejun Heo <tj@...nel.org>, Lai Jiangshan <jiangshanlai@...il.com>
Cc: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] softirq: Provide a handshake for canceling tasklets via
 polling on PREEMPT_RT

On 2025-08-12 16:39:32 [+0200], To linux-kernel@...r.kernel.org wrote:
> The tasklet_unlock_spin_wait() via tasklet_disable_in_atomic() is
> provided for a few legacy tasklet users. The interface is used from
> atomic context (which is either softirq or disabled preemption) on
> non-PREEMPT_RT an relies on spinning until the tasklet callback
> completes.
> On PREEMPT_RT the context is never atomic but the busy polling logic
> remains. It possible that the thread invoking tasklet_unlock_spin_wait()
> has higher priority than the tasklet. If both run on the same CPU the
> the tasklet makes no progress and the thread trying to cancel the
> tasklet will live-lock the system.
> To avoid the lockup tasklet_unlock_spin_wait() uses local_bh_disable()/
> enable() which utilizes the local_lock_t for synchronisation. This lock
> is a central per-CPU BKL and about to be removed.
> 
> Acquire a lock in tasklet_action_common() which is held while the
> tasklet's callback is invoked. This lock will be acquired from
> tasklet_unlock_spin_wait() via tasklet_callback_cancel_wait_running().
> After the tasklet completed tasklet_callback_sync_wait_running() drops
> the lock and acquires it again. In order to avoid unlocking the lock
> even if there is no cancel request, there is a cb_waiters counter which
> is incremented during a cancel request.
> Blocking on the lock will PI-boost the tasklet if needed, ensuring
> progress is made.
> 

Tejun, Lai, I noticed that the BH part of workqueue also relies on this
mechanism (__flush_work(), the PREEMPT_RT ifdef).
This is a fairly recent API so there should be no "legacy" users as we
have it the tasklet interface. The majority of users use tasklet_kill()
(or seldom tasklet_unlock_wait()) and not tasklet_unlock_spin_wait().
The plan was to get rid of the spinning API but I didn't manage to get
rid of all users especially since some of the code could not be sanely/
safely converted (+tested).

Does the workqueue-BH code require the canceling from atomic context or
was this just added because the API for BH and non-BH work items is the
same and __cancel_work_sync() allows it?
Could we avoid the busy-waiting for BH work items and rely on the
wait_for_completion() below or do we need something similar to what I
added here for the tasklet API?

Sebastian