On Tue, Mar 18, 2025 at 02:56:18PM +0100, Frederic Weisbecker wrote:
> The numbers used in rcu_seq_done_exact() lack some explanation behind
> their magic. Especially after the commit:
>
> 85aad7cc4178 ("rcu: Fix get_state_synchronize_rcu_full() GP-start
> detection")
>
> which reported a subtle issue where a new GP sequence snapshot was taken
> on the root node state while a grace period had already been started and
> reflected on the global state sequence but not yet on the root node
> sequence, making a polling user waiting on a wrong already started grace
> period that would ignore freshly online CPUs.
>
> The fix involved taking the snaphot on the global state sequence and
> waiting on the root node sequence. And since a grace period is first
> started on the global state and only afterward reflected on the root
> node, a snapshot taken on the global state sequence might be two full
> grace periods ahead of the root node as in the following example:
>
> rnp->gp_seq = rcu_state.gp_seq = 0
>
> CPU 0 CPU 1
> ----- -----
> // rcu_state.gp_seq = 1
> rcu_seq_start(&rcu_state.gp_seq)
> // snap = 8
> snap =
> rcu_seq_snap(&rcu_state.gp_seq)
> // Two full GP differences
>
> rcu_seq_done_exact(&rnp->gp_seq, snap)
> // rnp->gp_seq = 1
> WRITE_ONCE(rnp->gp_seq, rcu_state.gp_seq);
>
> Add a comment about those expectations and to clarify the magic within
> the relevant function.
>
> Signed-off-by: Frederic Weisbecker <frede...@kernel.org>
Reviewed-by: Paul E. McKenney <paul...@kernel.org>
But it would of course be good to get reviews from the others.
> ---
> kernel/rcu/rcu.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index eed2951a4962..7acf1f36dd6c 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -157,6 +157,13 @@ static inline bool rcu_seq_done(unsigned long *sp,
> unsigned long s)
> * Given a snapshot from rcu_seq_snap(), determine whether or not a
> * full update-side operation has occurred, but do not allow the
> * (ULONG_MAX / 2) safety-factor/guard-band.
> + *
> + * The token returned by get_state_synchronize_rcu_full() is based on
> + * rcu_state.gp_seq but it is tested in poll_state_synchronize_rcu_full()
> + * against the root rnp->gp_seq. Since rcu_seq_start() is first called
> + * on rcu_state.gp_seq and only later reflected on the root rnp->gp_seq,
> + * it is possible that rcu_seq_snap(rcu_state.gp_seq) returns 2 full grace
> + * periods ahead of the root rnp->gp_seq.
> */
> static inline bool rcu_seq_done_exact(unsigned long *sp, unsigned long s)
> {
> --
> 2.48.1
>
