Re: [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic()

[Paul E. McKenney]

On Sat, Jul 11, 2026 at 05:09:22AM +0000, Zqiang wrote:
> > 
> > On Fri, Jul 10, 2026 at 11:18:05PM +0000, Zqiang wrote:
> > 
> > > 
> > > On Fri, Jul 10, 2026 at 07:50:28PM +0800, Zqiang wrote:
> > >  
> > >  > 
> > >  > The smp_mb() after kthread_run() in rcu_spawn_tasks_kthread_generic()
> > >  > from early 'commit eacd6f04a133 ("rcu-tasks: Move Tasks RCU to its
> > >  > own file")' and 'commit 84a8f446ffd7 ("rcu: Defer rcu_tasks_kthread()
> > >  > creation till first call_rcu_tasks()")', the pairing as follows:
> > >  > 
> > >  > rcu_spawn_tasks_kthread()
> > >  > ->t = kthread_run(rcu_tasks_kthread, ...);
> > >  > ->smp_mb(); /* Ensure others see full kthread. */
> > >  > ->WRITE_ONCE(rcu_tasks_kthread_ptr, t);
> > >  > 
> > >  > call_rcu_tasks()
> > >  > ->if (READ_ONCE(rcu_tasks_kthread_ptr))
> > >  > ->wake_up(&rcu_tasks_cbs_wq)
> > >  > ->try_to_wake_up()
> > >  > lock pi_lock
> > >  > ->smp_mb__after_spinlock()
> > >  > //see full kthread
> > >  > 
> > >  > Currently, the 'commit d119357d0743 ("rcu-tasks: Treat only synchronous
> > >  > grace periods urgently")' moved kthread_ptr assignment into the
> > >  > rcu_tasks_kthread(), the following pairings are sufficient:
> > >  > 
> > >  > The rq->lock/unlock from wake_up_process() in kthread_run() and
> > >  > __schedule() provides a memory barrier when the kthread is first
> > >  > scheduled, this ensures the kthread itself observes all of the
> > >  > kthread's initialization.
> > >  > 
> > >  > The kthread's smp_store_release(&rtp->kthread_ptr, ...) in
> > >  > rcu_tasks_kthread() and smp_load_acquire(&rtp->kthread_ptr) in
> > >  > call_rcu_tasks_generic() compose release/acquire pairing, the
> > >  > cumulativity of smp_store_release() propagates visibility of the
> > >  > kthread's initialization through the scheduler chain.
> > >  > 
> > >  > This commit therefore remove smp_mb() in rcu_spawn_tasks_kthread_generic().
> > >  > 
> > >  > Signed-off-by: Zqiang <qiang.zhang@xxxxxxxxx>
> > >  > 
> > >  Very good, a removal of an smp_mb() that also simplifies analysis!
> > >  
> > >  One question...
> > >  
> > >  > 
> > >  > ---
> > >  > kernel/rcu/tasks.h | 2 --
> > >  > 1 file changed, 2 deletions(-)
> > >  > 
> > >  > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > >  > index 5a8db89135eb..18ab335665e3 100644
> > >  > --- a/kernel/rcu/tasks.h
> > >  > +++ b/kernel/rcu/tasks.h
> > >  > @@ -395,7 +395,6 @@ static void call_rcu_tasks_generic(struct rcu_head *rhp, rcu_callback_t func,
> > >  > raw_spin_unlock_irqrestore(&rtp->cbs_gbl_lock, flags);
> > >  > }
> > >  > rcu_read_unlock();
> > >  > - /* We can't create the thread unless interrupts are enabled. */
> > >  > 
> > >  Why remove this comment? People reading this code might wonder why we
> > >  are not just invoking rcu_spawn_tasks_kthread_generic() to create the
> > >  task here.
> > >  
> > >  Now, if you say that the current comment is confusing because if that
> > >  kthread already exists, we also defer the wakeup, fair enough. But in
> > >  that case, could you please propose a replacement comment, perhaps as
> > >  a separate patch?
> > >  
> > >  How about the following comment:
> > >  
> > >  /* Use irq_work do deferred wakeup to avoid potential lockdep splat */
> > >  
> > >  If directly use rcuwait_wake_up() to wakup, the lockdep will possible spalt，
> > >  (for example we hook trace_sched_wakeup() tracepoint, and in our hook
> > >  func inovke call_rcu_tasks())
> > > 
> > Fair enough. But the reason for the READ_ONCE(rtp->kthread_ptr) part of
> > the condition of that "if" statement is that we cannot create a kthread
> > with interrupts disabled, either because it is too early in boot or
> > because one of the scheduler locks might be held.
> > 
> > Maybe that is obvious? But we did add that comment at some point.
> > 
> > Thoughts?
> 
> Ok, I find that:
> 
> c63eb17ff06 ("rcu: Create call_rcu_tasks() kthread at boot time")
> 4929c913bda ("rcu: Make call_rcu_tasks() tolerate first call with irqs disabled")
> 
> As a separate patch send,  the update comments are as follows:
> 
> "We can't create the kthread with interrupts disabled, 
> or a sinlock might be held, create the kthread moved
> at core_initcall() time, at the same time, use the irq_work
> does deferred wakeup to avoid potential lockdep splat"

Very good, looking forward to the updates!

							Thanx, Paul