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 <[email protected]>
> > >
> > 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?
Thanx, Paul
> Thanks
> Zqiang
>
>
> >
> > Thanx, Paul
> >
> > >
> > > if (needwake && READ_ONCE(rtp->kthread_ptr))
> > > irq_work_queue(&rtpcp->rtp_irq_work);
> > > }
> > > @@ -681,7 +680,6 @@ static void __init
> > > rcu_spawn_tasks_kthread_generic(struct rcu_tasks *rtp)
> > > t = kthread_run(rcu_tasks_kthread, rtp, "%s_kthread", rtp->kname);
> > > if (WARN_ONCE(IS_ERR(t), "%s: Could not start %s grace-period kthread,
> > > OOM is now expected behavior\n", __func__, rtp->name))
> > > return;
> > > - smp_mb(); /* Ensure others see full kthread. */
> > > }
> > >
> > > #ifndef CONFIG_TINY_RCU
> > > --
> > > 2.17.1
> > >
> >
