* [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic()
@ 2026-07-10 11:50 Zqiang
2026-07-10 18:25 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Zqiang @ 2026-07-10 11:50 UTC (permalink / raw)
To: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun
Cc: qiang.zhang, rcu, linux-kernel
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@linux.dev>
---
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. */
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() 2026-07-10 11:50 [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() Zqiang @ 2026-07-10 18:25 ` Paul E. McKenney 2026-07-10 23:18 ` Zqiang 0 siblings, 1 reply; 6+ messages in thread From: Paul E. McKenney @ 2026-07-10 18:25 UTC (permalink / raw) To: Zqiang Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu, linux-kernel 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@linux.dev> 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? 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() 2026-07-10 18:25 ` Paul E. McKenney @ 2026-07-10 23:18 ` Zqiang 2026-07-11 2:48 ` Paul E. McKenney 0 siblings, 1 reply; 6+ messages in thread From: Zqiang @ 2026-07-10 23:18 UTC (permalink / raw) To: paulmck Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu, linux-kernel > > 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@linux.dev> > > > 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()) 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 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() 2026-07-10 23:18 ` Zqiang @ 2026-07-11 2:48 ` Paul E. McKenney 2026-07-11 5:09 ` Zqiang 0 siblings, 1 reply; 6+ messages in thread From: Paul E. McKenney @ 2026-07-11 2:48 UTC (permalink / raw) To: Zqiang Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu, linux-kernel 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@linux.dev> > > > > > 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 > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() 2026-07-11 2:48 ` Paul E. McKenney @ 2026-07-11 5:09 ` Zqiang 2026-07-11 23:56 ` Paul E. McKenney 0 siblings, 1 reply; 6+ messages in thread From: Zqiang @ 2026-07-11 5:09 UTC (permalink / raw) To: paulmck Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu, linux-kernel > > 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@linux.dev> > > > > > 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" Thanks Zqiang > > 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 > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() 2026-07-11 5:09 ` Zqiang @ 2026-07-11 23:56 ` Paul E. McKenney 0 siblings, 0 replies; 6+ messages in thread From: Paul E. McKenney @ 2026-07-11 23:56 UTC (permalink / raw) To: Zqiang Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu, linux-kernel 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@linux.dev> > > > > > > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-11 23:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-10 11:50 [PATCH] rcu-tasks: Remove smp_mb() in rcu_spawn_tasks_kthread_generic() Zqiang 2026-07-10 18:25 ` Paul E. McKenney 2026-07-10 23:18 ` Zqiang 2026-07-11 2:48 ` Paul E. McKenney 2026-07-11 5:09 ` Zqiang 2026-07-11 23:56 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox