Re: [PATCH] rcu: Move expedited-GP callback advancement into __note_gp_changes()

[Frederic Weisbecker <frederic@kernel.org>]

Le Fri, Oct 16, 2026 at 03:28:42AM -0700, Puranjay Mohan a écrit :
> Commit ec5ee5417b002 ("rcu: Advance callbacks for expedited GP
> completion in rcu_core()") advances callbacks whose grace period was an
> expedited one from a dedicated block in rcu_core().  That block is
> needed because __note_gp_changes() returns early when
> rdp->gp_seq == rnp->gp_seq, and expedited grace periods do not change
> rnp->gp_seq.
> 
> The advancement depends only on the grace-period state kept in the
> callback list, which covers both normal and expedited grace periods, so
> it can be done in __note_gp_changes() instead.  Move it there.  This is
> mostly a relocation, with the following differences:
> 
> - rcu_cbs_gp_done() combines rcu_segcblist_nextgp() with
>   poll_state_synchronize_rcu_full_unordered(), and returns false for
>   offloaded CPUs, as the removed block did.  need_note_gp_changes() uses
>   it so that the lockless check in note_gp_changes() takes the leaf
>   rcu_node lock for such a completion, and rcu_pending() now calls
>   need_note_gp_changes() instead of open-coding the same tests.
> 
> - The removed block gated on poll_state_synchronize_rcu_full(), which
>   executes smp_mb() on every rcu_core() with pending callbacks.  The
>   gate is now the unordered variant from fde2b18817e4f.  Which callbacks
>   become ready is still decided by rcu_segcblist_advance(), which uses
>   the ordered poll.
> 
> - note_gp_changes() has one other caller, the qhimark path of
>   call_rcu_core(), which now also advances callbacks on an expedited
>   completion.  __note_gp_changes() is also called by rcu_gp_init(),
>   where an expedited completion now takes rcu_advance_cbs() instead of
>   rcu_accelerate_cbs(), which it ends with anyway.  In rcu_gp_cleanup(),
>   the leaf rcu_node's ->gp_seq has just advanced, so the existing
>   completed-grace-period path is taken as before.
> 
> - An expedited-only completion (rdp->gp_seq == rnp->gp_seq and ->gpwrap
>   clear) returns before touching core_needs_qs, cpu_no_qs, rdp->gp_seq
>   or gp_seq_needed.
> 
> One behavior change is not related to expedited grace periods.  With
> ->gpwrap set and rdp->gp_seq == rnp->gp_seq, the early return skipped
> the code that clears ->gpwrap and recomputes core_needs_qs from
> rnp->qsmask.  That case is now processed.  It exists at boot:
> rcu_data is statically initialized with .gpwrap = true, and
> rcutree_prepare_cpu() sets rdp->gp_seq from rnp->gp_seq without
> clearing it.  Until the first grace period starts, ->gpwrap therefore
> stays set and the ->gpwrap test in rcu_pending() reports work pending.
> 
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
>  kernel/rcu/tree.c | 105 ++++++++++++++++++++++++++++------------------
>  1 file changed, 64 insertions(+), 41 deletions(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index aea3aef7397ec..c224a8bc8aa6b 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -163,6 +163,7 @@ static void rcu_report_exp_rdp(struct rcu_data *rdp);
>  static void rcu_report_qs_rdp(struct rcu_data *rdp);
>  static void check_cb_ovld_locked(struct rcu_data *rdp, struct rcu_node *rnp);
>  static bool rcu_rdp_is_offloaded(struct rcu_data *rdp);
> +static bool poll_state_synchronize_rcu_full_unordered(struct rcu_gp_seq *gsp);
>  static bool rcu_rdp_cpu_online(struct rcu_data *rdp);
>  static bool rcu_init_invoked(void);
>  static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf);
> @@ -1260,30 +1261,63 @@ static void rcu_strict_gp_check_qs(void)
>  	}
>  }
>  
> +/*
> + * Has a grace period (normal or expedited) that this CPU's pending callbacks
> + * are waiting on completed?  Offloaded CPUs are handled by their rcuog
> + * kthreads, so always say "no" for them.  This is a racy, memory-ordering-free
> + * hint: the ordered check is done by rcu_segcblist_advance() when the
> + * callbacks are actually advanced.  The hint can also be true for callbacks
> + * that have not yet been assigned a grace period, because the stale
> + * ->gp_seq[RCU_WAIT_TAIL] then looks completed.  That is harmless, as the
> + * resulting rcu_advance_cbs() assigns them one.
> + */
> +static bool rcu_cbs_gp_done(struct rcu_data *rdp)
> +{
> +	struct rcu_gp_seq gp_state;
> +
> +	return !rcu_rdp_is_offloaded(rdp) &&
> +	       rcu_segcblist_nextgp(&rdp->cblist, &gp_state) &&
> +	       poll_state_synchronize_rcu_full_unordered(&gp_state);
> +}
> +
>  /*
>   * Update CPU-local rcu_data state to record the beginnings and ends of
>   * grace periods.  The caller must hold the ->lock of the leaf rcu_node
>   * structure corresponding to the current CPU, and must have irqs disabled.
> - * Returns true if the grace-period kthread needs to be awakened.
> + * Also advances callbacks whose grace period was an expedited one, which
> + * does not change ->gp_seq.  Returns true if the grace-period kthread
> + * needs to be awakened.
>   */
>  static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp)
>  {
>  	bool ret = false;
>  	bool need_qs;
> +	bool cbs_done;
>  	const bool offloaded = rcu_rdp_is_offloaded(rdp);
> +	// ->gpwrap means ->gp_seq cannot be trusted, even if it now equals ours.
> +	const bool gp_changed = rdp->gp_seq != rnp->gp_seq || unlikely(rdp->gpwrap);
> +	const 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)
> +	/*
> +	 * Expedited grace periods do not change ->gp_seq, but they can still
> +	 * make callbacks ready to invoke.  The callback list tracks both
> +	 * normal and expedited grace periods, so use it to detect that case.
> +	 */
> +	cbs_done = !completed && rcu_cbs_gp_done(rdp);

Why the "!completed" here? rcu_cbs_gp_done() should be enough.

Other than that:

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

Thanks a lot!

-- 
Frederic Weisbecker
SUSE Labs