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: <20250822094812.L4hiquhY@linutronix.de>
Date: Fri, 22 Aug 2025 11:48:12 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Tejun Heo <tj@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Lai Jiangshan <jiangshanlai@...il.com>,
	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-21 07:10:42 [-1000], Tejun Heo wrote:
> Hello, Sebastian.
Hello Tejun,

> Oh, I'm not insisting, don't know enough to do so. Just trying to understand
> the situation.

ah okay.

> > > I'm most likely missing something about RT but wouldn't the above still lead
> > > to deadlocks if the caller is non-hardirq but higher priority thread?
> > 
> > Not sure what you refer to. Right now there is this lock in
> > local_bh_disable() which forces PI.
> > Removing the whole section for RT as in this snippet gets us to the
> > wait_for_completion() below. It lets the task with higher priority
> > schedule out allowing task with lower priority to run. Eventually the
> > barrier item completes and with the wakeup the high priority task will
> > continue.
> > So the high-priority task will lose runtime (allowing task with lower
> > priority to run). I don't think it will be a problem because it is
> > mostly used in "quit" scenarios (not during normal workload) and matches
> > tasklet_disable().
> 
> Okay, so, on !RT, that busy loop section is there to allow
> cancel_work_sync() to be called from BH-disabled contexts and the caller is
> responsible for ensuring there's no recursion. It's not great but matches
> the existing behavior. 

hold on for for a sec: existing behaviour for tasklet_unlock_spin_wait()
which has three users (a fourth one if we count i915 which has its own
tasklet layer). Not something that I would call common or wide spread
behaviour in the kernel (and task workqueue does not have it either).

tasklet_disable() and tasklet_kill() both sleep while waiting for
completion and don't spin.

>                        Obviously, in !RT, we can't go to
> wait_for_completion() there because we can be in a non-sleepable context.

Again, only a small amount of users require to do so.

> Are you saying that, in RT, it'd be fine to call wait_for_completion()
> inside local_bh_disable() and won't trip any of the context warnings? If so,
> yeah, we don't need any of the looping.

No, that won't work. local_bh_disable() will start a RCU read section
and then RCU will complain during schedule().
So if the requirement is to cancel a BH workitem from within a BH
disabled section then we need the first patch in this thread.

But if we get rid of this requirement…

> Thanks.

Sebastian