Re: [PATCH v1 06/11] rcu: Enable RCU callbacks to benefit from expedited grace periods

On Fri, Jul 10, 2026 at 03:58:59PM +0200, Frederic Weisbecker wrote:
> Le Wed, Jun 24, 2026 at 06:23:48AM -0700, Puranjay Mohan a écrit :
> > Currently, RCU callbacks only track normal grace-period sequence
> > numbers.  This means callbacks must wait for normal grace periods to
> > complete even when expedited grace periods have already elapsed.
> > 
> > Use the full struct rcu_gp_seq (which tracks both the normal and
> > expedited grace-period sequences) throughout the callback
> > infrastructure.
> > 
> > rcu_segcblist_advance() now checks both normal and expedited GP
> > completion via poll_state_synchronize_rcu_full(), and becomes
> > parameterless since it reads the grace-period state internally.
> > rcu_segcblist_accelerate() stores the full state (both sequences)
> > instead of just the normal one.  rcu_accelerate_cbs() and
> > rcu_accelerate_cbs_unlocked() use get_state_synchronize_rcu_full() to
> > capture both sequences, and the NOCB advance checks use
> > poll_state_synchronize_rcu_full() instead of comparing only the normal
> > sequence.
> > 
> > srcu_segcblist_advance() becomes a standalone implementation because it
> > compares SRCU sequences directly and cannot use
> > poll_state_synchronize_rcu_full(), which reads RCU-specific globals.
> > srcu_segcblist_accelerate() sets the ->exp field to
> > RCU_GET_STATE_NOT_TRACKED so that poll_state_synchronize_rcu_full()
> > compares only ->norm and ignores ->exp.
> > 
> > Reviewed-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: Puranjay Mohan <[email protected]>
> > ---
> >  kernel/rcu/rcu_segcblist.c | 30 +++++++++++++++++++++++-------
> >  kernel/rcu/rcu_segcblist.h |  2 +-
> >  kernel/rcu/tree.c          |  9 +++------
> >  kernel/rcu/tree_nocb.h     | 33 +++++++++++++++++++++++----------
> >  4 files changed, 50 insertions(+), 24 deletions(-)
> > 
> > diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
> > index 4e3dfe42bc097..cf8951d33e767 100644
> > --- a/kernel/rcu/rcu_segcblist.c
> > +++ b/kernel/rcu/rcu_segcblist.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/types.h>
> >  
> > +#include "rcu.h"
> >  #include "rcu_segcblist.h"
> >  
> >  /* Initialize simple callback list. */
> > @@ -494,9 +495,9 @@ static void rcu_segcblist_advance_compact(struct rcu_segcblist *rsclp, int i)
> >  
> >  /*
> >   * Advance the callbacks in the specified rcu_segcblist structure based
> > - * on the current value passed in for the grace-period counter.
> > + * on the current value of the grace-period counter.
> >   */
> > -void rcu_segcblist_advance(struct rcu_segcblist *rsclp, struct rcu_gp_seq *gsp)
> > +void rcu_segcblist_advance(struct rcu_segcblist *rsclp)
> >  {
> >  	int i;
> >  
> > @@ -509,7 +510,7 @@ void rcu_segcblist_advance(struct rcu_segcblist *rsclp, struct rcu_gp_seq *gsp)
> >  	 * are ready to invoke, and put them into the RCU_DONE_TAIL segment.
> >  	 */
> >  	for (i = RCU_WAIT_TAIL; i < RCU_NEXT_TAIL; i++) {
> > -		if (ULONG_CMP_LT(gsp->norm, rsclp->gp_seq[i].norm))
> > +		if (!poll_state_synchronize_rcu_full(&rsclp->gp_seq[i]))
> 
> So after more careful review, the smp_mb() at the end of a successful
> poll_state_synchronize_rcu_full() is necessary here because the current locking
> is not enough to make sure we synchronize against the end of the grace period.
> 
> But what about the smp_mb() at the beginning? Paul what is the point of this one
> already? It advertizes to pair with the smp_mb() on root cleanup but what
> exactly is to be ordered here? Why does gp cleanup need to synchronize with
> failing poll_state_synchronize_rcu_full() ? The smp_mb() before rcu_seq_snap()
> in get_state_synchronize_rcu_full() should already synchronize the accesses
> before that call against the beginning of the grace period.
> 
> If we keep all these barriers around and both RCU_WAIT_TAIL and RCU_NEXT_READY
> need to be advanced, that makes 4 smp_mb() calls.

Apologies for the delay, I missed this one.

You are right that if poll_state_synchronize_rcu_full() fails we don't
need ordering.  Especially given that poll_state_synchronize_rcu()
doesn't have this first memory barrier.

This fits in with get_state_synchronize_full() emulating a call to
synchronize_rcu() and poll_state_synchronize_rcu_full() emulating the
return from synchronize_rcu().  So get_state_synchronize_full() has
its smp_mb() at the beginning and poll_state_synchronize_rcu_full()
at the end.

The only rationale I can give for that initial smp_mb() is that in
the comment, but it makes no sense because the code prior to the call
to poll_state_synchronize_rcu_full() cannot know whether or not this
function will return false.

Puranjay, are you going to remove this smp_mb(), or would you prefer
that I do so?

> > @@ -637,14 +638,29 @@ void rcu_segcblist_merge(struct rcu_segcblist *dst_rsclp,
> >  
> >  void srcu_segcblist_advance(struct rcu_segcblist *rsclp, unsigned long seq)
> >  {
> > -	struct rcu_gp_seq gs = { .norm = seq };
> > +	int i;
> > +
> > +	WARN_ON_ONCE(!rcu_segcblist_is_enabled(rsclp));
> > +	if (rcu_segcblist_restempty(rsclp, RCU_DONE_TAIL))
> > +		return;
> > +
> > +	for (i = RCU_WAIT_TAIL; i < RCU_NEXT_TAIL; i++) {
> > +		if (ULONG_CMP_LT(seq, rsclp->gp_seq[i].norm))
> > +			break;
> 
> Why not use the same API here and consolidate the code? ->exp is RCU_GET_STATE_NOT_TRACKED so it's
> harmless?
> 
> > @@ -1164,7 +1164,7 @@ static bool rcu_accelerate_cbs(struct rcu_node *rnp, struct rcu_data *rdp)
> >  	 * accelerating callback invocation to an earlier grace-period
> >  	 * number.
> >  	 */
> > -	gs.norm = rcu_seq_snap(&rcu_state.gp_seq);
> > +	get_state_synchronize_rcu_full(&gs);
> 
> I have similar concerns about the three smp_mb() in
> get_state_synchronize_rcu_full(). It could be just two (rcu_seq_snap()
> has a barrier that could be just one). Not sure if that matters but,
> just wanted to point that.

We need the one at the beginning of get_state_synchronize_rcu_full(),
but from what I can see, not the ones in the calls to rcu_seq_snap().
I blame laziness.  We could make an rcu_seq_snap_no_ordering() that
didn't have the smp_mb(), but I didn't believe that the overhead would
be visible at the system level.

							Thanx, Paul

> Thanks.
> 
> -- 
> Frederic Weisbecker
> SUSE Labs