Re: [PATCH RFC v3 03/13] rcu-tasks: Add a Tasks RCU implementation for reader-marked trampolines

[Paul E. McKenney]

On Wed, Sep 16, 2026 at 05:23:31PM +0200, Frederic Weisbecker wrote:
> Le Wed, Sep 16, 2026 at 07:55:02AM -0700, Paul E. McKenney a écrit :
> > On Wed, Sep 16, 2026 at 04:47:29PM +0200, Frederic Weisbecker wrote:
> > > Le Wed, Sep 16, 2026 at 04:35:50PM +0200, Frederic Weisbecker a écrit :
> > > > Le Wed, Sep 16, 2026 at 07:26:35AM -0700, Paul E. McKenney a écrit :
> > > > > On Wed, Sep 16, 2026 at 02:40:20PM +0200, Frederic Weisbecker wrote:
> > > > > > Le Tue, Sep 15, 2026 at 04:56:41PM -0700, Paul E. McKenney a écrit :
> > > > > > > > Alternatively the approach could be generalized to vanilla RCU, it could be
> > > > > > > > possible to define a .text.rcu_no_qs section within which code running is
> > > > > > > > considered as an RCU reader (with a pause while on the explicit RCU tasks
> > > > > > > > section). It would be forbidden to voluntary sleep inside
> > > > > > > > and to put explicit preemption points (CONFIG_PROVE_RCU could report misuses).
> > > > > > > > 
> > > > > > > > Based on IP, RCU could consider those interrupted section as readers. This would
> > > > > > > > require PREEMPT_RCU though.
> > > > > > > > 
> > > > > > > > And then synchronize_rcu() would do the 1, 2, 4 jobs.
> > > > > > > 
> > > > > > > If I am following correctly (ha!), sleepable BPF programs rule out use
> > > > > > > of RCU in this manner.
> > > > > > > 
> > > > > > > But your point is nevertheless valid, in that SRCU could be used.
> > > > > > > And because rcu_read_lock_trace() is a thin wrapper around SRCU-fast, we
> > > > > > > *might* be able to instead use rcu_read_lock_tasks_trace(), which would
> > > > > > > skip the task-struct increment and decrement, saving a few instructions.
> > > > > > > Then, instead of waiting for each task's counter to go to zero, instead
> > > > > > > just invoke synchronize_rcu_tasks_trace().
> > > > > > > 
> > > > > > > Which is pretty close to what Josef is proposing, just with the new RCU
> > > > > > > Tasks Trace read-side primitives.  I think.  ;-)
> > > > > > > 
> > > > > > > This assumes that we do not need to flatten partially overlapping RCU
> > > > > > > Tasks Trace readers into one big reader.
> > > > > > > 
> > > > > > > Or am I missing something here?
> > > > > > 
> > > > > > Yes I think that's what Josef does in this patchset. The problem is about
> > > > > > handling the few instructions:
> > > > > > 
> > > > > > 1) between the begining of the trampoline and the call to rcu_read_lock_trace()
> > > > > > 
> > > > > > 2) between the call to rcu_read_unlock_trace() and the end of the trampoline
> > > > > > 
> > > > > > So what I'm proposing is to make those two parts implicit RCU read lock sections.
> > > > > > 
> > > > > > So the whole trampoline would be .text.rcu_no_qs:
> > > > > > 
> > > > > > .text.rcu_no_qs trampoline:
> > > > > >   __________________________________________________________________________________________
> > > > > >  |Few instructions 1 | rcu_read_lock_trace() .... rcu_read_unlock_trace | Few instructions 2|
> > > > > >  ___________________________________________________________________________________________
> > > > > > 
> > > > > > Then when a tick fires, rcu_flavor_sched_clock_irq() discards the interrupted
> > > > > > code as QS if the IP was within .text.rcu_no_qs _unless_ it is in the
> > > > > > rcu_read_lock_trace. Both are easy and quick to verify.
> > > > > > 
> > > > > > Also preempt_schedule_irq() would make sure to verify the same condition and
> > > > > > enqueue the task as a GP blocker if preempting inside "Few instructions 1"
> > > > > > or "Few instructions 2".
> > > > > 
> > > > > Ah, OK, I might be following now.  ;-)
> > > > > 
> > > > > We also need both versions of rcu_exp_handler() to check the IP as well,
> > > > > given that sooner or later someone is going to want trampoline removal
> > > > > to go faster.  Or am I still missing a turn in here somewhere?
> > 
> > I should add that the thing that I really like about Frederic's approach
> > is that avoids the task-list scan.  Or at least has the potential to
> > do so.  Such scans have proven problematic in the past.
> > 
> > > > Yes indeed, missed the exp part!
> > > 
> > > What remains to handle also is non-preemptible RCU because if the task is
> > > preempted by an IRQ while in the .text.rcu_no_qs, we may still need to keep
> > > track of that somewhere.
> > 
> > Perhaps in rcu_core() in kernels booted with use_softirq?  I am thinking
> > specifically of the checks for deferred quiescent states.  I don't (yet)
> > see a need to modify rcu_check_quiescent_state().
> > 
> > Maybe other places as well.  ;-)
> 
> Hmm this tracking would have to happen on preempt_schedule() just like we
> do for PREEMPT_RCU. Or am I missing something? And then we would need a
> list scan of those tasks.

I am thinking of the case where a trampoline is interrupted before entering
(or after leaving) its RCU Tasks Trace read-side critical section.  Then
there is a softirq handler on the back of that interrupt handler, and
RCU_SOFTIRQ is invoked, calling rcu_core().  Specifically:

	/* Report any deferred quiescent states if preemption enabled. */
	if (IS_ENABLED(CONFIG_PREEMPT_COUNT) && (!(preempt_count() & PREEMPT_MASK))) {
		rcu_preempt_deferred_qs(current);
	} else if (rcu_preempt_need_deferred_qs(current)) {
		guard(irqsave)();
		set_need_resched_current();
	}

Preemption is enabled, but we should not report a quiescent state because
we have interrupted a trampoline.  Correct?

> Or we can build the blocked task list handling, that we already have for PREEMPT_RCU,
> when CONFIG_RCU_TASKS && !CONFIG_PREEMPT_RCU. We would just only add tasks when
> preempted in .text.rcu_no_qs since rcu_read_lock() would still disable
> preemption on normal explicit readers. So I wouldn't expect more overhead due to
> that blocked list tracking built since it would rarely track tasks.

Yes, we could avoid the list of tasks by treating the preemption within
the trampoline the same as preemption within an RCU read-side critical
section, but there might not be an rcu_read_unlock() to clean up.
Which could be a problem.

Trampolines that transfer control to tracing code could supply the needed
cleanup call.  But last I checked, there were trampolines that transferred
directly back to the original code, with no opportunity for cleaning up.

Or am I still missing a trick here?

							Thanx, Paul