diff mbox series

[3/6,v2] rcu/exp: Remove superfluous full memory barrier upon first EQS snapshot

Message ID Zn1O2qKaIVOXniYH@pavilion.home (mailing list archive)
State Accepted
Commit 33c0860bf7e7ace39eba96f51cc6d898ab253202
Headers show
Series None | expand

Commit Message

Frederic Weisbecker June 27, 2024, 11:36 a.m. UTC 
When the grace period kthread checks the extended quiescent state
counter of a CPU, full ordering is necessary to ensure that either:

* If the GP kthread observes the remote target in an extended quiescent
  state, then that target must observe all accesses prior to the current
  grace period, including the current grace period sequence number, once
  it exits that extended quiescent state.

or:

* If the GP kthread observes the remote target NOT in an extended
  quiescent state, then the target further entering in an extended
  quiescent state must observe all accesses prior to the current
  grace period, including the current grace period sequence number, once
  it enters that extended quiescent state.

This ordering is enforced through a full memory barrier placed right
before taking the first EQS snapshot. However this is superfluous
because the snapshot is taken while holding the target's rnp lock which
provides the necessary ordering through its chain of
smp_mb__after_unlock_lock().

Remove the needless explicit barrier before the snapshot and put a
comment about the implicit barrier newly relied upon here.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Neeraj Upadhyay <neeraj.upadhyay@kernel.org>
---
 kernel/rcu/tree_exp.h | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Paul E. McKenney June 27, 2024, 6:53 p.m. UTC | #1 
On Thu, Jun 27, 2024 at 01:36:58PM +0200, Frederic Weisbecker wrote:
> When the grace period kthread checks the extended quiescent state
> counter of a CPU, full ordering is necessary to ensure that either:

Just to make sure that I understand...

I need to replace these commits in -rcu:

da979d0162fc6 rcu: Remove full ordering on second EQS snapshot
6411f4185f657 rcu: Remove superfluous full memory barrier upon first EQS snapshot
dec56ca5f1c34 rcu/exp: Remove superfluous full memory barrier upon first EQS snapshot

With these three patches, and keep these three commits as they are?

d43a302fc08a5 rcu: Remove full memory barrier on boot time eqs sanity check
b1c36aa90cbf1 rcu: Remove full memory barrier on RCU stall printout
64d68f1d53f77 rcu/exp: Remove redundant full memory barrier at the end of GP

							Thanx, Paul

> * If the GP kthread observes the remote target in an extended quiescent
>   state, then that target must observe all accesses prior to the current
>   grace period, including the current grace period sequence number, once
>   it exits that extended quiescent state.
> 
> or:
> 
> * If the GP kthread observes the remote target NOT in an extended
>   quiescent state, then the target further entering in an extended
>   quiescent state must observe all accesses prior to the current
>   grace period, including the current grace period sequence number, once
>   it enters that extended quiescent state.
> 
> This ordering is enforced through a full memory barrier placed right
> before taking the first EQS snapshot. However this is superfluous
> because the snapshot is taken while holding the target's rnp lock which
> provides the necessary ordering through its chain of
> smp_mb__after_unlock_lock().
> 
> Remove the needless explicit barrier before the snapshot and put a
> comment about the implicit barrier newly relied upon here.
> 
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Neeraj Upadhyay <neeraj.upadhyay@kernel.org>
> ---
>  kernel/rcu/tree_exp.h | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
> index 8a1d9c8bd9f7..1dbad2442e8d 100644
> --- a/kernel/rcu/tree_exp.h
> +++ b/kernel/rcu/tree_exp.h
> @@ -357,7 +357,21 @@ static void __sync_rcu_exp_select_node_cpus(struct rcu_exp_work *rewp)
>  		    !(rnp->qsmaskinitnext & mask)) {
>  			mask_ofl_test |= mask;
>  		} else {
> -			snap = rcu_dynticks_snap(cpu);
> +			/*
> +			 * Full ordering between remote CPU's post idle accesses
> +			 * and updater's accesses prior to current GP (and also
> +			 * the started GP sequence number) is enforced by
> +			 * rcu_seq_start() implicit barrier, relayed by kworkers
> +			 * locking and even further by smp_mb__after_unlock_lock()
> +			 * barriers chained all the way throughout the rnp locking
> +			 * tree since sync_exp_reset_tree() and up to the current
> +			 * leaf rnp locking.
> +			 *
> +			 * Ordering between remote CPU's pre idle accesses and
> +			 * post grace period updater's accesses is enforced by the
> +			 * below acquire semantic.
> +			 */
> +			snap = ct_dynticks_cpu_acquire(cpu);
>  			if (rcu_dynticks_in_eqs(snap))
>  				mask_ofl_test |= mask;
>  			else
> -- 
> 2.45.2
>
Frederic Weisbecker June 28, 2024, 11:20 a.m. UTC | #2 
Le Thu, Jun 27, 2024 at 11:53:29AM -0700, Paul E. McKenney a écrit :
> On Thu, Jun 27, 2024 at 01:36:58PM +0200, Frederic Weisbecker wrote:
> > When the grace period kthread checks the extended quiescent state
> > counter of a CPU, full ordering is necessary to ensure that either:
> 
> Just to make sure that I understand...
> 
> I need to replace these commits in -rcu:
> 
> da979d0162fc6 rcu: Remove full ordering on second EQS snapshot
> 6411f4185f657 rcu: Remove superfluous full memory barrier upon first EQS snapshot
> dec56ca5f1c34 rcu/exp: Remove superfluous full memory barrier upon first EQS snapshot
> 
> With these three patches, and keep these three commits as they are?
> 
> d43a302fc08a5 rcu: Remove full memory barrier on boot time eqs sanity check
> b1c36aa90cbf1 rcu: Remove full memory barrier on RCU stall printout
> 64d68f1d53f77 rcu/exp: Remove redundant full memory barrier at the end of GP

Exactly! Those were the precisions I forgot to mention.

Thanks.
Paul E. McKenney June 28, 2024, 8:03 p.m. UTC | #3 
On Fri, Jun 28, 2024 at 01:20:55PM +0200, Frederic Weisbecker wrote:
> Le Thu, Jun 27, 2024 at 11:53:29AM -0700, Paul E. McKenney a écrit :
> > On Thu, Jun 27, 2024 at 01:36:58PM +0200, Frederic Weisbecker wrote:
> > > When the grace period kthread checks the extended quiescent state
> > > counter of a CPU, full ordering is necessary to ensure that either:
> > 
> > Just to make sure that I understand...
> > 
> > I need to replace these commits in -rcu:
> > 
> > da979d0162fc6 rcu: Remove full ordering on second EQS snapshot
> > 6411f4185f657 rcu: Remove superfluous full memory barrier upon first EQS snapshot
> > dec56ca5f1c34 rcu/exp: Remove superfluous full memory barrier upon first EQS snapshot
> > 
> > With these three patches, and keep these three commits as they are?
> > 
> > d43a302fc08a5 rcu: Remove full memory barrier on boot time eqs sanity check
> > b1c36aa90cbf1 rcu: Remove full memory barrier on RCU stall printout
> > 64d68f1d53f77 rcu/exp: Remove redundant full memory barrier at the end of GP
> 
> Exactly! Those were the precisions I forgot to mention.

Done, and started testing.  If all goes well, this might make tomorrow's
-next.

							Thanx, Paul
diff mbox series

Patch

diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index 8a1d9c8bd9f7..1dbad2442e8d 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -357,7 +357,21 @@  static void __sync_rcu_exp_select_node_cpus(struct rcu_exp_work *rewp)
 		    !(rnp->qsmaskinitnext & mask)) {
 			mask_ofl_test |= mask;
 		} else {
-			snap = rcu_dynticks_snap(cpu);
+			/*
+			 * Full ordering between remote CPU's post idle accesses
+			 * and updater's accesses prior to current GP (and also
+			 * the started GP sequence number) is enforced by
+			 * rcu_seq_start() implicit barrier, relayed by kworkers
+			 * locking and even further by smp_mb__after_unlock_lock()
+			 * barriers chained all the way throughout the rnp locking
+			 * tree since sync_exp_reset_tree() and up to the current
+			 * leaf rnp locking.
+			 *
+			 * Ordering between remote CPU's pre idle accesses and
+			 * post grace period updater's accesses is enforced by the
+			 * below acquire semantic.
+			 */
+			snap = ct_dynticks_cpu_acquire(cpu);
 			if (rcu_dynticks_in_eqs(snap))
 				mask_ofl_test |= mask;
 			else