On Wed, Jul 22, 2026 at 10:50 PM Frederic Weisbecker <[email protected]> wrote: > > Le Tue, Jul 21, 2026 at 04:06:24PM +0100, Puranjay Mohan a écrit : > > On Tue, Jul 21, 2026 at 3:35 PM Frederic Weisbecker <[email protected]> > > wrote: > > > > > > Le Wed, Jun 24, 2026 at 06:23:52AM -0700, Puranjay Mohan a écrit : > > > > Even when rcu_pending() triggers rcu_core(), the normal callback > > > > advancement path through note_gp_changes() -> __note_gp_changes() bails > > > > out when rdp->gp_seq == rnp->gp_seq (no normal GP change). Since > > > > expedited GPs do not update rnp->gp_seq, rcu_advance_cbs() is never > > > > called and callbacks remain stuck in RCU_WAIT_TAIL. > > > > > > > > Add a direct callback advancement block in rcu_core() that checks for GP > > > > completion via rcu_segcblist_nextgp() combined with > > > > poll_state_synchronize_rcu_full(). When detected, trylock rnp and call > > > > rcu_advance_cbs() to move completed callbacks to RCU_DONE_TAIL. Wake the > > > > GP kthread if rcu_advance_cbs() requests a new grace period. > > > > > > > > Uses trylock to avoid adding contention on rnp->lock. If the lock is > > > > contended, callbacks will be advanced on the next tick. > > > > > > > > Reviewed-by: Paul E. McKenney <[email protected]> > > > > Signed-off-by: Puranjay Mohan <[email protected]> > > > > --- > > > > kernel/rcu/tree.c | 17 +++++++++++++++++ > > > > 1 file changed, 17 insertions(+) > > > > > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > > > index b01d7bf6b57b1..f42e01ef479c4 100644 > > > > --- a/kernel/rcu/tree.c > > > > +++ b/kernel/rcu/tree.c > > > > @@ -2891,6 +2891,23 @@ static __latent_entropy void rcu_core(void) > > > > /* Update RCU state based on any recent quiescent states. */ > > > > rcu_check_quiescent_state(rdp); > > > > > > > > + /* Advance callbacks if an expedited GP has completed. */ > > > > + if (!rcu_rdp_is_offloaded(rdp) && > > > > rcu_segcblist_is_enabled(&rdp->cblist)) { > > > > + struct rcu_gp_seq gp_state; > > > > + > > > > + if (rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && > > > > + poll_state_synchronize_rcu_full(&gp_state)) { > > > > + guard(irqsave)(); > > > > + if (raw_spin_trylock_rcu_node(rnp)) { > > > > + bool needwake = rcu_advance_cbs(rnp, rdp); > > > > + > > > > + raw_spin_unlock_rcu_node(rnp); > > > > + if (needwake) > > > > + rcu_gp_kthread_wake(); > > > > + } > > > > + } > > > > + } > > > > > > Should that go as an improvement to note_gp_changes() instead? > > > > note_gp_changes() only reconciles rdp->gp_seq against rnp->gp_seq, and > > the expedited path never advances rnp->gp_seq. So the gap this closes > > is exactly rdp->gp_seq == rnp->gp_seq, where note_gp_changes() and > > __note_gp_changes() both short-circuit, the expedited completion isn't > > visible there at all. It's detected from the cblist's stored gp_seq > > (rcu_segcblist_nextgp()) confirmed with > > poll_state_synchronize_rcu_full(), so hosting it in note_gp_changes() > > would mean running that in the lockless preamble for every caller, > > including the off-tick call_rcu_core() path. In rcu_core() it's > > already gated by rcu_pending(), which does the barrier-free detection. > > Let's take a step back. note_gp_changes() is for the CPU to ackowledge > a grace period change, either start or completion, and react upon with: > > _ Making the callback progress through the state machine if a grace period > has changed. > > _ Starting to chase quiescent states. > > And now callback advancing/acceleration don't even refer anymore to the > leaf node state but to the global one. So why not proceed with that > logic? > > Also other callers of note_gp_changes() may want to benefit from expedited > grace periods as well. > > Would the following (untested) work? > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index ff6601411a89..96bf7fe03be8 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -1271,27 +1271,29 @@ static bool __note_gp_changes(struct rcu_node *rnp, > struct rcu_data *rdp) > { > bool ret = false; > bool need_qs; > + struct rcu_gp_seq gp_state; > const bool offloaded = rcu_rdp_is_offloaded(rdp); > > raw_lockdep_assert_held_rcu_node(rnp); > > - if (rdp->gp_seq == rnp->gp_seq) > - return false; /* Nothing to do. */ > - > /* Handle the ends of any preceding grace periods first. */ > - if (rcu_seq_completed_gp(rdp->gp_seq, rnp->gp_seq) || > + if ((rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && > + poll_state_synchronize_rcu_full_unordered(&gp_state)) || > unlikely(rdp->gpwrap)) { > if (!offloaded) > ret = rcu_advance_cbs(rnp, rdp); /* Advance CBs. */ > rdp->core_needs_qs = false; > trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, > TPS("cpuend")); > - } else { > + } else if (rdp->gp_seq != rnp->gp_seq) { > if (!offloaded) > ret = rcu_accelerate_cbs(rnp, rdp); /* Recent CBs. */ > if (rdp->core_needs_qs) > rdp->core_needs_qs = !!(rnp->qsmask & rdp->grpmask); > } > > + if (rdp->gp_seq == rnp->gp_seq) > + return ret; /* Nothing else to do. */ > + > /* Now handle the beginnings of any new-to-this-CPU grace periods. */ > if (rcu_seq_new_gp(rdp->gp_seq, rnp->gp_seq) || > unlikely(rdp->gpwrap)) { > @@ -1316,6 +1318,27 @@ static bool __note_gp_changes(struct rcu_node *rnp, > struct rcu_data *rdp) > return ret; > } > > +static bool need_note_gp_changes(struct rcu_data *rdp) > +{ > + struct rcu_gp_seq gp_state; > + struct rcu_node *rnp = rdp->mynode; > + > + /* Need to chase QS or accelerate? */ > + if (rdp->gp_seq != rcu_seq_current(&rnp->gp_seq)) > + return true; > + > + /* Waited upon GP has ended, need to advance CBs ? */ > + if (rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && > + poll_state_synchronize_rcu_full_unordered(&gp_state)) > + return true; > + > + /* Wrapped? */ > + if (unlikely(READ_ONCE(rdp->gpwrap))) > + return true; > + > + return false; > +} > + > static void note_gp_changes(struct rcu_data *rdp) > { > unsigned long flags; > @@ -1324,8 +1347,7 @@ static void note_gp_changes(struct rcu_data *rdp) > > local_irq_save(flags); > rnp = rdp->mynode; > - if ((rdp->gp_seq == rcu_seq_current(&rnp->gp_seq) && > - !unlikely(READ_ONCE(rdp->gpwrap))) || /* w/out lock. */ > + if (!need_note_gp_changes(rdp) || /* w/out lock. */ > !raw_spin_trylock_rcu_node(rnp)) { /* irqs already off, so later. > */ > local_irq_restore(flags); > return; > @@ -2888,23 +2910,6 @@ static __latent_entropy void rcu_core(void) > /* Update RCU state based on any recent quiescent states. */ > rcu_check_quiescent_state(rdp); > > - /* Advance callbacks if an expedited GP has completed. */ > - if (!rcu_rdp_is_offloaded(rdp) && > rcu_segcblist_is_enabled(&rdp->cblist)) { > - struct rcu_gp_seq gp_state; > - > - if (rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && > - poll_state_synchronize_rcu_full(&gp_state)) { > - guard(irqsave)(); > - if (raw_spin_trylock_rcu_node(rnp)) { > - bool needwake = rcu_advance_cbs(rnp, rdp); > - > - raw_spin_unlock_rcu_node(rnp); > - if (needwake) > - rcu_gp_kthread_wake(); > - } > - } > - } > - > /* No grace period and unregistered callbacks? */ > if (!rcu_gp_in_progress() && > rcu_segcblist_is_enabled(&rdp->cblist) && > !rcu_rdp_is_offloaded(rdp)) { > diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h > index 01a1b2985abd..6b9b058d138e 100644 > --- a/kernel/rcu/tree.h > +++ b/kernel/rcu/tree.h > @@ -517,6 +517,7 @@ static void rcu_nocb_unlock(struct rcu_data *rdp); > static void rcu_nocb_unlock_irqrestore(struct rcu_data *rdp, > unsigned long flags); > static void rcu_lockdep_assert_cblist_protected(struct rcu_data *rdp); > +static bool poll_state_synchronize_rcu_full_unordered(struct rcu_gp_seq > *gsp); > #ifdef CONFIG_RCU_NOCB_CPU > static void __init rcu_organize_nocb_kthreads(void); > >
Hi Frederic. I took your approach and created this commit with minor changes to your diff: A few points I'd like a second opinion on. I kept the unordered poll_state_synchronize_rcu_full_unordered() for the advance check (the old rcu_core() block used the ordered variant): this looks safe because rcu_segcblist_advance() re-checks each segment with the ordered poll_state_synchronize_rcu_full() before moving callbacks to RCU_DONE_TAIL, so the check here is only a gate and the barriers still apply where callbacks are actually advanced, please confirm that reasoning. need_note_gp_changes() runs the callback-list poll on the lockless preamble for both callers, including offloaded rdps where __note_gp_changes() won't advance anything; I left it ungated since it only leads to a trylock, but it could take a !rcu_rdp_is_offloaded(rdp) guard. I also dropped the rcu_segcblist_is_enabled() guard the rcu_core() block had, relying on __note_gp_changes() already operating on the cblist unconditionally for non-offloaded rdps. -- >8 -- >From c38c5f599d699e2c40d3459fdf3ef383a99c0097 Mon Sep 17 00:00:00 2001 From: Puranjay Mohan <[email protected]> Date: Fri, 24 Jul 2026 07:26:25 -0700 Subject: [PATCH] rcu: Advance callbacks for expedited GP completion in note_gp_changes() When rcu_pending() triggers rcu_core(), the callback advancement path through note_gp_changes() -> __note_gp_changes() bails out when rdp->gp_seq == rnp->gp_seq (no normal GP change). Since expedited GPs do not update rnp->gp_seq, rcu_advance_cbs() is never reached from there and callbacks satisfied by an expedited GP would otherwise remain stuck in RCU_WAIT_TAIL until the next normal GP. This is currently handled by a dedicated advancement block in rcu_core() that polls the callback list and advances under a trylock. But callback advancement no longer depends on the leaf-node grace-period delta; it is driven by the grace-period state stored in the callback list, which tracks both normal and expedited GPs. __note_gp_changes() is the natural home for it, and hosting it there lets every note_gp_changes() caller benefit from expedited completions rather than just rcu_core(). Move the advancement into __note_gp_changes(): trigger rcu_advance_cbs() whenever rcu_segcblist_nextgp() confirmed with poll_state_synchronize_rcu_full_unordered() reports a completed grace period, and add need_note_gp_changes() so the lockless preamble takes the lock for an expedited-only completion instead of short-circuiting on rdp->gp_seq == rnp->gp_seq. Remove the now-redundant rcu_core() block. The quiescent-state bookkeeping stays keyed to an actual rnp->gp_seq change, so an expedited completion never clears a still-pending core_needs_qs and stalls the normal grace period. Signed-off-by: Puranjay Mohan <[email protected]> --- kernel/rcu/tree.c | 56 ++++++++++++++++++++++++----------------------- kernel/rcu/tree.h | 1 + 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 21b6ce1dffb63..0e700d0ecf27e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -1270,27 +1270,33 @@ static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp) { bool ret = false; bool need_qs; + struct rcu_gp_seq gp_state; const bool offloaded = rcu_rdp_is_offloaded(rdp); + bool completed = rcu_seq_completed_gp(rdp->gp_seq, rnp->gp_seq) || + unlikely(rdp->gpwrap); raw_lockdep_assert_held_rcu_node(rnp); - if (rdp->gp_seq == rnp->gp_seq) - return false; /* Nothing to do. */ - /* Handle the ends of any preceding grace periods first. */ - if (rcu_seq_completed_gp(rdp->gp_seq, rnp->gp_seq) || - unlikely(rdp->gpwrap)) { + if (completed || + (rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && + poll_state_synchronize_rcu_full_unordered(&gp_state))) { if (!offloaded) ret = rcu_advance_cbs(rnp, rdp); /* Advance CBs. */ - rdp->core_needs_qs = false; - trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("cpuend")); - } else { + if (completed) { + rdp->core_needs_qs = false; + trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("cpuend")); + } + } else if (rdp->gp_seq != rnp->gp_seq) { if (!offloaded) ret = rcu_accelerate_cbs(rnp, rdp); /* Recent CBs. */ if (rdp->core_needs_qs) rdp->core_needs_qs = !!(rnp->qsmask & rdp->grpmask); } + if (rdp->gp_seq == rnp->gp_seq) + return ret; /* Nothing else to do. */ + /* Now handle the beginnings of any new-to-this-CPU grace periods. */ if (rcu_seq_new_gp(rdp->gp_seq, rnp->gp_seq) || unlikely(rdp->gpwrap)) { @@ -1315,6 +1321,20 @@ static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp) return ret; } +static bool need_note_gp_changes(struct rcu_data *rdp) +{ + struct rcu_gp_seq gp_state; + struct rcu_node *rnp = rdp->mynode; + + if (rdp->gp_seq != rcu_seq_current(&rnp->gp_seq) || + unlikely(READ_ONCE(rdp->gpwrap))) + return true; + + /* Has a grace period a callback is waiting on completed? */ + return rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && + poll_state_synchronize_rcu_full_unordered(&gp_state); +} + static void note_gp_changes(struct rcu_data *rdp) { unsigned long flags; @@ -1323,8 +1343,7 @@ static void note_gp_changes(struct rcu_data *rdp) local_irq_save(flags); rnp = rdp->mynode; - if ((rdp->gp_seq == rcu_seq_current(&rnp->gp_seq) && - !unlikely(READ_ONCE(rdp->gpwrap))) || /* w/out lock. */ + if (!need_note_gp_changes(rdp) || /* w/out lock. */ !raw_spin_trylock_rcu_node(rnp)) { /* irqs already off, so later. */ local_irq_restore(flags); return; @@ -2886,23 +2905,6 @@ static __latent_entropy void rcu_core(void) /* Update RCU state based on any recent quiescent states. */ rcu_check_quiescent_state(rdp); - /* Advance callbacks if an expedited GP has completed. */ - if (!rcu_rdp_is_offloaded(rdp) && rcu_segcblist_is_enabled(&rdp->cblist)) { - struct rcu_gp_seq gp_state; - - if (rcu_segcblist_nextgp(&rdp->cblist, &gp_state) && - poll_state_synchronize_rcu_full(&gp_state)) { - guard(irqsave)(); - if (raw_spin_trylock_rcu_node(rnp)) { - bool needwake = rcu_advance_cbs(rnp, rdp); - - raw_spin_unlock_rcu_node(rnp); - if (needwake) - rcu_gp_kthread_wake(); - } - } - } - /* No grace period and unregistered callbacks? */ if (!rcu_gp_in_progress() && rcu_segcblist_is_enabled(&rdp->cblist) && !rcu_rdp_is_offloaded(rdp)) { diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h index eedfa43059e80..962f86afc3c9a 100644 --- a/kernel/rcu/tree.h +++ b/kernel/rcu/tree.h @@ -521,6 +521,7 @@ static void rcu_nocb_unlock(struct rcu_data *rdp); static void rcu_nocb_unlock_irqrestore(struct rcu_data *rdp, unsigned long flags); static void rcu_lockdep_assert_cblist_protected(struct rcu_data *rdp); +static bool poll_state_synchronize_rcu_full_unordered(struct rcu_gp_seq *gsp); #ifdef CONFIG_RCU_NOCB_CPU static void __init rcu_organize_nocb_kthreads(void); base-commit: dc597bcabf31a58f8c1aca01677a6af0b3307398 -- 2.53.0-Meta -- 8< -- Thanks, Puranjay
