Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1558878 > unrolled thread

[PATCH tip/core/rcu 0/6] Dynticks updates for 4.11

Back to article view | Back to linux.kernel

Contents

  [PATCH tip/core/rcu 0/6] Dynticks updates for 4.11 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-14 10:00 +0100
    [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended quiescent state enter/exit operations "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-14 10:00 +0100
      Re: [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended  quiescent state enter/exit operations Josh Triplett <josh@joshtriplett.org> - 2017-01-16 08:50 +0100
        Re: [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended  quiescent state enter/exit operations "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-16 12:40 +0100
      Re: [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended  quiescent state enter/exit operations Peter Zijlstra <peterz@infradead.org> - 2017-01-16 17:50 +0100
    [PATCH tip/core/rcu 4/6] rcu: Abstract extended quiescent state determination "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-14 10:00 +0100
    [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks momentary-idle operation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-14 10:00 +0100
      Re: [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks  momentary-idle operation Josh Triplett <josh@joshtriplett.org> - 2017-01-16 08:50 +0100
        Re: [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks  momentary-idle operation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-16 12:30 +0100
          Re: [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks  momentary-idle operation Josh Triplett <josh@joshtriplett.org> - 2017-01-16 20:00 +0100
    Re: [PATCH tip/core/rcu 0/6] Dynticks updates for 4.11 Josh Triplett <josh@joshtriplett.org> - 2017-01-16 09:00 +0100

#1558878 — [PATCH tip/core/rcu 0/6] Dynticks updates for 4.11

Hello!

This series provides dynticks updates:

1-4.	Abstract access to the dyntick counter, replacing the current
	open-coding of atomic operations.

5.	Check cond_resched_rcu_qs() state less often to reduce GP overhead.

6.	Adjust FQS offline checks for exact online-CPU detection. 

							Thanx, Paul

------------------------------------------------------------------------

 include/linux/rcutiny.h    |    6 +
 include/trace/events/rcu.h |   10 -
 kernel/rcu/tree.c          |  245 +++++++++++++++++++++++++++++++--------------
 kernel/rcu/tree.h          |    2 
 kernel/rcu/tree_exp.h      |   12 --
 kernel/rcu/tree_plugin.h   |    2 
 kernel/rcu/tree_trace.c    |    2 
 7 files changed, 190 insertions(+), 89 deletions(-)

[toc] | [next] | [standalone]

#1558879 — [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended quiescent state enter/exit operations

This commit is the third step towards full abstraction of all accesses
to the ->dynticks counter, implementing the previously open-coded atomic
add of 1 and entry checks in a new rcu_dynticks_eqs_enter() function, and
the same but with exit checks in a new rcu_dynticks_eqs_exit() function.
This abstraction will ease changes to the ->dynticks counter operation.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcu/tree.c | 92 +++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 26 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 805d55ee0b2a..fc49e008963a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -282,6 +282,65 @@ static DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
 };
 
 /*
+ * Record entry into an extended quiescent state.  This is only to be
+ * called when not already in an extended quiescent state.
+ */
+static void rcu_dynticks_eqs_enter(void)
+{
+	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+
+	/*
+	 * CPUs seeing atomic_inc() must see prior RCU read-side critical
+	 * sections, and we also must force ordering with the next idle
+	 * sojourn.
+	 */
+	smp_mb__before_atomic(); /* See above. */
+	atomic_inc(&rdtp->dynticks);
+	smp_mb__after_atomic(); /* See above. */
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
+		     atomic_read(&rdtp->dynticks) & 0x1);
+}
+
+/*
+ * Record exit from an extended quiescent state.  This is only to be
+ * called from an extended quiescent state.
+ */
+static void rcu_dynticks_eqs_exit(void)
+{
+	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+
+	/*
+	 * CPUs seeing atomic_inc() must see prior idle sojourns,
+	 * and we also must force ordering with the next RCU read-side
+	 * critical section.
+	 */
+	smp_mb__before_atomic(); /* See above. */
+	atomic_inc(&rdtp->dynticks);
+	smp_mb__after_atomic(); /* See above. */
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
+		     !(atomic_read(&rdtp->dynticks) & 0x1));
+}
+
+/*
+ * Reset the current CPU's ->dynticks counter to indicate that the
+ * newly onlined CPU is no longer in an extended quiescent state.
+ * This will either leave the counter unchanged, or increment it
+ * to the next non-quiescent value.
+ *
+ * The non-atomic test/increment sequence works because the upper bits
+ * of the ->dynticks counter are manipulated only by the corresponding CPU,
+ * or when the corresponding CPU is offline.
+ */
+static void rcu_dynticks_eqs_online(void)
+{
+	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+
+	if (atomic_read(&rdtp->dynticks) & 0x1)
+		return;
+	atomic_add(0x1, &rdtp->dynticks);
+}
+
+/*
  * Snapshot the ->dynticks counter with full ordering so as to allow
  * stable comparison of this counter with past and future snapshots.
  */
@@ -693,7 +752,7 @@ static void rcu_eqs_enter_common(long long oldval, bool user)
 {
 	struct rcu_state *rsp;
 	struct rcu_data *rdp;
-	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+	struct rcu_dynticks __maybe_unused *rdtp = this_cpu_ptr(&rcu_dynticks);
 
 	trace_rcu_dyntick(TPS("Start"), oldval, rdtp->dynticks_nesting);
 	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
@@ -712,12 +771,7 @@ static void rcu_eqs_enter_common(long long oldval, bool user)
 		do_nocb_deferred_wakeup(rdp);
 	}
 	rcu_prepare_for_idle();
-	/* CPUs seeing atomic_inc() must see prior RCU read-side crit sects */
-	smp_mb__before_atomic();  /* See above. */
-	atomic_inc(&rdtp->dynticks);
-	smp_mb__after_atomic();  /* Force ordering with next sojourn. */
-	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
-		     atomic_read(&rdtp->dynticks) & 0x1);
+	rcu_dynticks_eqs_enter();
 	rcu_dynticks_task_enter();
 
 	/*
@@ -846,15 +900,10 @@ void rcu_irq_exit_irqson(void)
  */
 static void rcu_eqs_exit_common(long long oldval, int user)
 {
-	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+	struct rcu_dynticks __maybe_unused *rdtp = this_cpu_ptr(&rcu_dynticks);
 
 	rcu_dynticks_task_exit();
-	smp_mb__before_atomic();  /* Force ordering w/previous sojourn. */
-	atomic_inc(&rdtp->dynticks);
-	/* CPUs seeing atomic_inc() must see later RCU read-side crit sects */
-	smp_mb__after_atomic();  /* See above. */
-	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
-		     !(atomic_read(&rdtp->dynticks) & 0x1));
+	rcu_dynticks_eqs_exit();
 	rcu_cleanup_after_idle();
 	trace_rcu_dyntick(TPS("End"), oldval, rdtp->dynticks_nesting);
 	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
@@ -1001,11 +1050,7 @@ void rcu_nmi_enter(void)
 	 * period (observation due to Andy Lutomirski).
 	 */
 	if (!(atomic_read(&rdtp->dynticks) & 0x1)) {
-		smp_mb__before_atomic();  /* Force delay from prior write. */
-		atomic_inc(&rdtp->dynticks);
-		/* atomic_inc() before later RCU read-side crit sects */
-		smp_mb__after_atomic();  /* See above. */
-		WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks) & 0x1));
+		rcu_dynticks_eqs_exit();
 		incby = 1;
 	}
 	rdtp->dynticks_nmi_nesting += incby;
@@ -1043,11 +1088,7 @@ void rcu_nmi_exit(void)
 
 	/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
 	rdtp->dynticks_nmi_nesting = 0;
-	/* CPUs seeing atomic_inc() must see prior RCU read-side crit sects */
-	smp_mb__before_atomic();  /* See above. */
-	atomic_inc(&rdtp->dynticks);
-	smp_mb__after_atomic();  /* Force delay to next write. */
-	WARN_ON_ONCE(atomic_read(&rdtp->dynticks) & 0x1);
+	rcu_dynticks_eqs_enter();
 }
 
 /**
@@ -3800,8 +3841,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)
 		init_callback_list(rdp);  /* Re-enable callbacks on this CPU. */
 	rdp->dynticks->dynticks_nesting = DYNTICK_TASK_EXIT_IDLE;
 	rcu_sysidle_init_percpu_data(rdp->dynticks);
-	atomic_set(&rdp->dynticks->dynticks,
-		   (atomic_read(&rdp->dynticks->dynticks) & ~0x1) + 1);
+	rcu_dynticks_eqs_online();
 	raw_spin_unlock_rcu_node(rnp);		/* irqs remain disabled. */
 
 	/*
-- 
2.5.2

[toc] | [prev] | [next] | [standalone]

#1559499 — Re: [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended quiescent state enter/exit operations

On Sat, Jan 14, 2017 at 12:54:42AM -0800, Paul E. McKenney wrote:
> This commit is the third step towards full abstraction of all accesses
> to the ->dynticks counter, implementing the previously open-coded atomic
> add of 1 and entry checks in a new rcu_dynticks_eqs_enter() function, and
> the same but with exit checks in a new rcu_dynticks_eqs_exit() function.
> This abstraction will ease changes to the ->dynticks counter operation.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

A couple of comments below.  With those addressed:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  kernel/rcu/tree.c | 92 +++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 66 insertions(+), 26 deletions(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 805d55ee0b2a..fc49e008963a 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -282,6 +282,65 @@ static DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
>  };
>  
>  /*
> + * Record entry into an extended quiescent state.  This is only to be
> + * called when not already in an extended quiescent state.
> + */
> +static void rcu_dynticks_eqs_enter(void)
> +{
> +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +
> +	/*
> +	 * CPUs seeing atomic_inc() must see prior RCU read-side critical
> +	 * sections, and we also must force ordering with the next idle
> +	 * sojourn.
> +	 */
> +	smp_mb__before_atomic(); /* See above. */
> +	atomic_inc(&rdtp->dynticks);
> +	smp_mb__after_atomic(); /* See above. */
> +	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> +		     atomic_read(&rdtp->dynticks) & 0x1);
> +}
> +
> +/*
> + * Record exit from an extended quiescent state.  This is only to be
> + * called from an extended quiescent state.
> + */
> +static void rcu_dynticks_eqs_exit(void)
> +{
> +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +
> +	/*
> +	 * CPUs seeing atomic_inc() must see prior idle sojourns,
> +	 * and we also must force ordering with the next RCU read-side
> +	 * critical section.
> +	 */
> +	smp_mb__before_atomic(); /* See above. */
> +	atomic_inc(&rdtp->dynticks);
> +	smp_mb__after_atomic(); /* See above. */
> +	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> +		     !(atomic_read(&rdtp->dynticks) & 0x1));
> +}
> +
> +/*
> + * Reset the current CPU's ->dynticks counter to indicate that the
> + * newly onlined CPU is no longer in an extended quiescent state.
> + * This will either leave the counter unchanged, or increment it
> + * to the next non-quiescent value.
> + *
> + * The non-atomic test/increment sequence works because the upper bits
> + * of the ->dynticks counter are manipulated only by the corresponding CPU,
> + * or when the corresponding CPU is offline.
> + */
> +static void rcu_dynticks_eqs_online(void)
> +{
> +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +
> +	if (atomic_read(&rdtp->dynticks) & 0x1)
> +		return;
> +	atomic_add(0x1, &rdtp->dynticks);
> +}
> +
> +/*
>   * Snapshot the ->dynticks counter with full ordering so as to allow
>   * stable comparison of this counter with past and future snapshots.
>   */
> @@ -693,7 +752,7 @@ static void rcu_eqs_enter_common(long long oldval, bool user)
>  {
>  	struct rcu_state *rsp;
>  	struct rcu_data *rdp;
> -	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +	struct rcu_dynticks __maybe_unused *rdtp = this_cpu_ptr(&rcu_dynticks);

Rather than marking a local variable as __maybe_unused (such that the
compiler can no longer help detect it as unused), could you move it into
the portion of the function that uses it, so that if reached, it'll
always get used?

>  	trace_rcu_dyntick(TPS("Start"), oldval, rdtp->dynticks_nesting);
>  	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> @@ -712,12 +771,7 @@ static void rcu_eqs_enter_common(long long oldval, bool user)
>  		do_nocb_deferred_wakeup(rdp);
>  	}
>  	rcu_prepare_for_idle();
> -	/* CPUs seeing atomic_inc() must see prior RCU read-side crit sects */
> -	smp_mb__before_atomic();  /* See above. */
> -	atomic_inc(&rdtp->dynticks);
> -	smp_mb__after_atomic();  /* Force ordering with next sojourn. */
> -	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> -		     atomic_read(&rdtp->dynticks) & 0x1);
> +	rcu_dynticks_eqs_enter();
>  	rcu_dynticks_task_enter();
>  
>  	/*
> @@ -846,15 +900,10 @@ void rcu_irq_exit_irqson(void)
>   */
>  static void rcu_eqs_exit_common(long long oldval, int user)
>  {
> -	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +	struct rcu_dynticks __maybe_unused *rdtp = this_cpu_ptr(&rcu_dynticks);

Same comment as above.

>  	rcu_dynticks_task_exit();
> -	smp_mb__before_atomic();  /* Force ordering w/previous sojourn. */
> -	atomic_inc(&rdtp->dynticks);
> -	/* CPUs seeing atomic_inc() must see later RCU read-side crit sects */
> -	smp_mb__after_atomic();  /* See above. */
> -	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> -		     !(atomic_read(&rdtp->dynticks) & 0x1));
> +	rcu_dynticks_eqs_exit();
>  	rcu_cleanup_after_idle();
>  	trace_rcu_dyntick(TPS("End"), oldval, rdtp->dynticks_nesting);
>  	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> @@ -1001,11 +1050,7 @@ void rcu_nmi_enter(void)
>  	 * period (observation due to Andy Lutomirski).
>  	 */
>  	if (!(atomic_read(&rdtp->dynticks) & 0x1)) {
> -		smp_mb__before_atomic();  /* Force delay from prior write. */
> -		atomic_inc(&rdtp->dynticks);
> -		/* atomic_inc() before later RCU read-side crit sects */
> -		smp_mb__after_atomic();  /* See above. */
> -		WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks) & 0x1));
> +		rcu_dynticks_eqs_exit();
>  		incby = 1;
>  	}
>  	rdtp->dynticks_nmi_nesting += incby;
> @@ -1043,11 +1088,7 @@ void rcu_nmi_exit(void)
>  
>  	/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
>  	rdtp->dynticks_nmi_nesting = 0;
> -	/* CPUs seeing atomic_inc() must see prior RCU read-side crit sects */
> -	smp_mb__before_atomic();  /* See above. */
> -	atomic_inc(&rdtp->dynticks);
> -	smp_mb__after_atomic();  /* Force delay to next write. */
> -	WARN_ON_ONCE(atomic_read(&rdtp->dynticks) & 0x1);
> +	rcu_dynticks_eqs_enter();
>  }
>  
>  /**
> @@ -3800,8 +3841,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)
>  		init_callback_list(rdp);  /* Re-enable callbacks on this CPU. */
>  	rdp->dynticks->dynticks_nesting = DYNTICK_TASK_EXIT_IDLE;
>  	rcu_sysidle_init_percpu_data(rdp->dynticks);
> -	atomic_set(&rdp->dynticks->dynticks,
> -		   (atomic_read(&rdp->dynticks->dynticks) & ~0x1) + 1);
> +	rcu_dynticks_eqs_online();
>  	raw_spin_unlock_rcu_node(rnp);		/* irqs remain disabled. */
>  
>  	/*
> -- 
> 2.5.2
> 

[toc] | [prev] | [next] | [standalone]

#1559655 — Re: [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended quiescent state enter/exit operations

On Sun, Jan 15, 2017 at 11:47:35PM -0800, Josh Triplett wrote:
> On Sat, Jan 14, 2017 at 12:54:42AM -0800, Paul E. McKenney wrote:
> > This commit is the third step towards full abstraction of all accesses
> > to the ->dynticks counter, implementing the previously open-coded atomic
> > add of 1 and entry checks in a new rcu_dynticks_eqs_enter() function, and
> > the same but with exit checks in a new rcu_dynticks_eqs_exit() function.
> > This abstraction will ease changes to the ->dynticks counter operation.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> A couple of comments below.  With those addressed:
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> >  kernel/rcu/tree.c | 92 +++++++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 66 insertions(+), 26 deletions(-)
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 805d55ee0b2a..fc49e008963a 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -282,6 +282,65 @@ static DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
> >  };
> >  
> >  /*
> > + * Record entry into an extended quiescent state.  This is only to be
> > + * called when not already in an extended quiescent state.
> > + */
> > +static void rcu_dynticks_eqs_enter(void)
> > +{
> > +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> > +
> > +	/*
> > +	 * CPUs seeing atomic_inc() must see prior RCU read-side critical
> > +	 * sections, and we also must force ordering with the next idle
> > +	 * sojourn.
> > +	 */
> > +	smp_mb__before_atomic(); /* See above. */
> > +	atomic_inc(&rdtp->dynticks);
> > +	smp_mb__after_atomic(); /* See above. */
> > +	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > +		     atomic_read(&rdtp->dynticks) & 0x1);
> > +}
> > +
> > +/*
> > + * Record exit from an extended quiescent state.  This is only to be
> > + * called from an extended quiescent state.
> > + */
> > +static void rcu_dynticks_eqs_exit(void)
> > +{
> > +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> > +
> > +	/*
> > +	 * CPUs seeing atomic_inc() must see prior idle sojourns,
> > +	 * and we also must force ordering with the next RCU read-side
> > +	 * critical section.
> > +	 */
> > +	smp_mb__before_atomic(); /* See above. */
> > +	atomic_inc(&rdtp->dynticks);
> > +	smp_mb__after_atomic(); /* See above. */
> > +	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > +		     !(atomic_read(&rdtp->dynticks) & 0x1));
> > +}
> > +
> > +/*
> > + * Reset the current CPU's ->dynticks counter to indicate that the
> > + * newly onlined CPU is no longer in an extended quiescent state.
> > + * This will either leave the counter unchanged, or increment it
> > + * to the next non-quiescent value.
> > + *
> > + * The non-atomic test/increment sequence works because the upper bits
> > + * of the ->dynticks counter are manipulated only by the corresponding CPU,
> > + * or when the corresponding CPU is offline.
> > + */
> > +static void rcu_dynticks_eqs_online(void)
> > +{
> > +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> > +
> > +	if (atomic_read(&rdtp->dynticks) & 0x1)
> > +		return;
> > +	atomic_add(0x1, &rdtp->dynticks);
> > +}
> > +
> > +/*
> >   * Snapshot the ->dynticks counter with full ordering so as to allow
> >   * stable comparison of this counter with past and future snapshots.
> >   */
> > @@ -693,7 +752,7 @@ static void rcu_eqs_enter_common(long long oldval, bool user)
> >  {
> >  	struct rcu_state *rsp;
> >  	struct rcu_data *rdp;
> > -	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> > +	struct rcu_dynticks __maybe_unused *rdtp = this_cpu_ptr(&rcu_dynticks);
> 
> Rather than marking a local variable as __maybe_unused (such that the
> compiler can no longer help detect it as unused), could you move it into
> the portion of the function that uses it, so that if reached, it'll
> always get used?
> 
> >  	trace_rcu_dyntick(TPS("Start"), oldval, rdtp->dynticks_nesting);

Its only use is in the above event trace, which can be disabled via
CONFIG_RCU_TRACE=n.  I could put the definition of rdtp under #ifdef,
but this seems ugly.  I could eliminate the variable, substituting
the initialization for rdtp in the event trace, but that would make
for a very long line, or an odd line break.

Or am I missing something here?

> >  	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > @@ -712,12 +771,7 @@ static void rcu_eqs_enter_common(long long oldval, bool user)
> >  		do_nocb_deferred_wakeup(rdp);
> >  	}
> >  	rcu_prepare_for_idle();
> > -	/* CPUs seeing atomic_inc() must see prior RCU read-side crit sects */
> > -	smp_mb__before_atomic();  /* See above. */
> > -	atomic_inc(&rdtp->dynticks);
> > -	smp_mb__after_atomic();  /* Force ordering with next sojourn. */
> > -	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > -		     atomic_read(&rdtp->dynticks) & 0x1);
> > +	rcu_dynticks_eqs_enter();
> >  	rcu_dynticks_task_enter();
> >  
> >  	/*
> > @@ -846,15 +900,10 @@ void rcu_irq_exit_irqson(void)
> >   */
> >  static void rcu_eqs_exit_common(long long oldval, int user)
> >  {
> > -	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> > +	struct rcu_dynticks __maybe_unused *rdtp = this_cpu_ptr(&rcu_dynticks);
> 
> Same comment as above.
> 
> >  	rcu_dynticks_task_exit();
> > -	smp_mb__before_atomic();  /* Force ordering w/previous sojourn. */
> > -	atomic_inc(&rdtp->dynticks);
> > -	/* CPUs seeing atomic_inc() must see later RCU read-side crit sects */
> > -	smp_mb__after_atomic();  /* See above. */
> > -	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > -		     !(atomic_read(&rdtp->dynticks) & 0x1));
> > +	rcu_dynticks_eqs_exit();
> >  	rcu_cleanup_after_idle();
> >  	trace_rcu_dyntick(TPS("End"), oldval, rdtp->dynticks_nesting);

This one is used here, at the top level of the function, and a few
lines down.  I see the same options available.  Thoughts?

							Thanx, Paul

> >  	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > @@ -1001,11 +1050,7 @@ void rcu_nmi_enter(void)
> >  	 * period (observation due to Andy Lutomirski).
> >  	 */
> >  	if (!(atomic_read(&rdtp->dynticks) & 0x1)) {
> > -		smp_mb__before_atomic();  /* Force delay from prior write. */
> > -		atomic_inc(&rdtp->dynticks);
> > -		/* atomic_inc() before later RCU read-side crit sects */
> > -		smp_mb__after_atomic();  /* See above. */
> > -		WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks) & 0x1));
> > +		rcu_dynticks_eqs_exit();
> >  		incby = 1;
> >  	}
> >  	rdtp->dynticks_nmi_nesting += incby;
> > @@ -1043,11 +1088,7 @@ void rcu_nmi_exit(void)
> >  
> >  	/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
> >  	rdtp->dynticks_nmi_nesting = 0;
> > -	/* CPUs seeing atomic_inc() must see prior RCU read-side crit sects */
> > -	smp_mb__before_atomic();  /* See above. */
> > -	atomic_inc(&rdtp->dynticks);
> > -	smp_mb__after_atomic();  /* Force delay to next write. */
> > -	WARN_ON_ONCE(atomic_read(&rdtp->dynticks) & 0x1);
> > +	rcu_dynticks_eqs_enter();
> >  }
> >  
> >  /**
> > @@ -3800,8 +3841,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)
> >  		init_callback_list(rdp);  /* Re-enable callbacks on this CPU. */
> >  	rdp->dynticks->dynticks_nesting = DYNTICK_TASK_EXIT_IDLE;
> >  	rcu_sysidle_init_percpu_data(rdp->dynticks);
> > -	atomic_set(&rdp->dynticks->dynticks,
> > -		   (atomic_read(&rdp->dynticks->dynticks) & ~0x1) + 1);
> > +	rcu_dynticks_eqs_online();
> >  	raw_spin_unlock_rcu_node(rnp);		/* irqs remain disabled. */
> >  
> >  	/*
> > -- 
> > 2.5.2
> > 
> 

[toc] | [prev] | [next] | [standalone]

#1559880 — Re: [PATCH tip/core/rcu 3/6] rcu: Abstract dynticks extended quiescent state enter/exit operations

On Sat, Jan 14, 2017 at 12:54:42AM -0800, Paul E. McKenney wrote:
>  /*
> + * Record entry into an extended quiescent state.  This is only to be
> + * called when not already in an extended quiescent state.
> + */
> +static void rcu_dynticks_eqs_enter(void)
> +{
> +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +
> +	/*
> +	 * CPUs seeing atomic_inc() must see prior RCU read-side critical
> +	 * sections, and we also must force ordering with the next idle
> +	 * sojourn.
> +	 */
> +	smp_mb__before_atomic(); /* See above. */
> +	atomic_inc(&rdtp->dynticks);
> +	smp_mb__after_atomic(); /* See above. */
> +	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> +		     atomic_read(&rdtp->dynticks) & 0x1);
> +}

In an earlier patch you replaced things with atomic_add_return(), why
not use atomic_inc_return() here?

[toc] | [prev] | [next] | [standalone]

#1558882 — [PATCH tip/core/rcu 4/6] rcu: Abstract extended quiescent state determination

This commit is the fourth step towards full abstraction of all accesses
to the ->dynticks counter, implementing previously open-coded checks and
comparisons in new rcu_dynticks_in_eqs() and rcu_dynticks_in_eqs_since()
functions.  This abstraction will ease changes to the ->dynticks counter
operation.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcutiny.h  |  6 ++++++
 kernel/rcu/tree.c        | 52 +++++++++++++++++++++++++++++++++++-------------
 kernel/rcu/tree.h        |  2 ++
 kernel/rcu/tree_exp.h    |  6 +++---
 kernel/rcu/tree_plugin.h |  2 +-
 kernel/rcu/tree_trace.c  |  2 +-
 6 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index ac81e4063b40..4f9b2fa2173d 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -27,6 +27,12 @@
 
 #include <linux/cache.h>
 
+struct rcu_dynticks;
+static inline int rcu_dynticks_snap(struct rcu_dynticks *rdtp)
+{
+	return 0;
+}
+
 static inline unsigned long get_state_synchronize_rcu(void)
 {
 	return 0;
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index fc49e008963a..5a4e7427f372 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -341,10 +341,22 @@ static void rcu_dynticks_eqs_online(void)
 }
 
 /*
+ * Is the current CPU in an extended quiescent state?
+ *
+ * No ordering, as we are sampling CPU-local information.
+ */
+bool rcu_dynticks_curr_cpu_in_eqs(void)
+{
+	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+
+	return !(atomic_read(&rdtp->dynticks) & 0x1);
+}
+
+/*
  * Snapshot the ->dynticks counter with full ordering so as to allow
  * stable comparison of this counter with past and future snapshots.
  */
-static int rcu_dynticks_snap(struct rcu_dynticks *rdtp)
+int rcu_dynticks_snap(struct rcu_dynticks *rdtp)
 {
 	int snap = atomic_add_return(0, &rdtp->dynticks);
 
@@ -352,6 +364,25 @@ static int rcu_dynticks_snap(struct rcu_dynticks *rdtp)
 }
 
 /*
+ * Return true if the snapshot returned from rcu_dynticks_snap()
+ * indicates that RCU is in an extended quiescent state.
+ */
+static bool rcu_dynticks_in_eqs(int snap)
+{
+	return !(snap & 0x1);
+}
+
+/*
+ * Return true if the CPU corresponding to the specified rcu_dynticks
+ * structure has spent some time in an extended quiescent state since
+ * rcu_dynticks_snap() returned the specified snapshot.
+ */
+static bool rcu_dynticks_in_eqs_since(struct rcu_dynticks *rdtp, int snap)
+{
+	return snap != rcu_dynticks_snap(rdtp);
+}
+
+/*
  * Do a double-increment of the ->dynticks counter to emulate a
  * momentary idle-CPU quiescent state.
  */
@@ -1049,7 +1080,7 @@ void rcu_nmi_enter(void)
 	 * to be in the outermost NMI handler that interrupted an RCU-idle
 	 * period (observation due to Andy Lutomirski).
 	 */
-	if (!(atomic_read(&rdtp->dynticks) & 0x1)) {
+	if (rcu_dynticks_curr_cpu_in_eqs()) {
 		rcu_dynticks_eqs_exit();
 		incby = 1;
 	}
@@ -1075,7 +1106,7 @@ void rcu_nmi_exit(void)
 	 * to us!)
 	 */
 	WARN_ON_ONCE(rdtp->dynticks_nmi_nesting <= 0);
-	WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks) & 0x1));
+	WARN_ON_ONCE(rcu_dynticks_curr_cpu_in_eqs());
 
 	/*
 	 * If the nesting level is not 1, the CPU wasn't RCU-idle, so
@@ -1101,9 +1132,7 @@ void rcu_nmi_exit(void)
  */
 bool notrace __rcu_is_watching(void)
 {
-	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
-
-	return atomic_read(&rdtp->dynticks) & 0x1;
+	return !rcu_dynticks_curr_cpu_in_eqs();
 }
 
 /**
@@ -1188,7 +1217,7 @@ static int dyntick_save_progress_counter(struct rcu_data *rdp,
 {
 	rdp->dynticks_snap = rcu_dynticks_snap(rdp->dynticks);
 	rcu_sysidle_check_cpu(rdp, isidle, maxj);
-	if ((rdp->dynticks_snap & 0x1) == 0) {
+	if (rcu_dynticks_in_eqs(rdp->dynticks_snap)) {
 		trace_rcu_fqs(rdp->rsp->name, rdp->gpnum, rdp->cpu, TPS("dti"));
 		if (ULONG_CMP_LT(READ_ONCE(rdp->gpnum) + ULONG_MAX / 4,
 				 rdp->mynode->gpnum))
@@ -1207,12 +1236,7 @@ static int dyntick_save_progress_counter(struct rcu_data *rdp,
 static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 				    bool *isidle, unsigned long *maxj)
 {
-	unsigned int curr;
 	int *rcrmp;
-	unsigned int snap;
-
-	curr = (unsigned int)rcu_dynticks_snap(rdp->dynticks);
-	snap = (unsigned int)rdp->dynticks_snap;
 
 	/*
 	 * If the CPU passed through or entered a dynticks idle phase with
@@ -1222,7 +1246,7 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 	 * read-side critical section that started before the beginning
 	 * of the current RCU grace period.
 	 */
-	if ((curr & 0x1) == 0 || UINT_CMP_GE(curr, snap + 2)) {
+	if (rcu_dynticks_in_eqs_since(rdp->dynticks, rdp->dynticks_snap)) {
 		trace_rcu_fqs(rdp->rsp->name, rdp->gpnum, rdp->cpu, TPS("dti"));
 		rdp->dynticks_fqs++;
 		return 1;
@@ -3811,7 +3835,7 @@ rcu_boot_init_percpu_data(int cpu, struct rcu_state *rsp)
 	rdp->grpmask = leaf_node_cpu_bit(rdp->mynode, cpu);
 	rdp->dynticks = &per_cpu(rcu_dynticks, cpu);
 	WARN_ON_ONCE(rdp->dynticks->dynticks_nesting != DYNTICK_TASK_EXIT_IDLE);
-	WARN_ON_ONCE(atomic_read(&rdp->dynticks->dynticks) != 1);
+	WARN_ON_ONCE(rcu_dynticks_in_eqs(rcu_dynticks_snap(rdp->dynticks)));
 	rdp->cpu = cpu;
 	rdp->rsp = rsp;
 	rcu_boot_init_nocb_percpu_data(rdp);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index fe98dd24adf8..3b953dcf6afc 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -595,6 +595,8 @@ extern struct rcu_state rcu_bh_state;
 extern struct rcu_state rcu_preempt_state;
 #endif /* #ifdef CONFIG_PREEMPT_RCU */
 
+int rcu_dynticks_snap(struct rcu_dynticks *rdtp);
+
 #ifdef CONFIG_RCU_BOOST
 DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_status);
 DECLARE_PER_CPU(int, rcu_cpu_kthread_cpu);
diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index 011f626b2fd8..e155a465cf84 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -360,7 +360,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
 			rdp->exp_dynticks_snap =
 				rcu_dynticks_snap(rdp->dynticks);
 			if (raw_smp_processor_id() == cpu ||
-			    !(rdp->exp_dynticks_snap & 0x1) ||
+			    rcu_dynticks_in_eqs(rdp->exp_dynticks_snap) ||
 			    !(rnp->qsmaskinitnext & rdp->grpmask))
 				mask_ofl_test |= rdp->grpmask;
 		}
@@ -383,8 +383,8 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
 			if (!(mask_ofl_ipi & mask))
 				continue;
 retry_ipi:
-			if (rcu_dynticks_snap(rdp->dynticks) !=
-			    rdp->exp_dynticks_snap) {
+			if (rcu_dynticks_in_eqs_since(rdp->dynticks,
+						      rdp->exp_dynticks_snap)) {
 				mask_ofl_test |= mask;
 				continue;
 			}
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 56583e764ebf..652209589adf 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1643,7 +1643,7 @@ static void print_cpu_stall_info(struct rcu_state *rsp, int cpu)
 	       "o."[!!(rdp->grpmask & rdp->mynode->qsmaskinit)],
 	       "N."[!!(rdp->grpmask & rdp->mynode->qsmaskinitnext)],
 	       ticks_value, ticks_title,
-	       atomic_read(&rdtp->dynticks) & 0xfff,
+	       rcu_dynticks_snap(rdtp) & 0xfff,
 	       rdtp->dynticks_nesting, rdtp->dynticks_nmi_nesting,
 	       rdp->softirq_snap, kstat_softirqs_cpu(RCU_SOFTIRQ, cpu),
 	       READ_ONCE(rsp->n_force_qs) - rsp->n_force_qs_gpstart,
diff --git a/kernel/rcu/tree_trace.c b/kernel/rcu/tree_trace.c
index b1f28972872c..b833cd0a29e8 100644
--- a/kernel/rcu/tree_trace.c
+++ b/kernel/rcu/tree_trace.c
@@ -124,7 +124,7 @@ static void print_one_rcu_data(struct seq_file *m, struct rcu_data *rdp)
 		   rdp->rcu_qs_ctr_snap == per_cpu(rcu_qs_ctr, rdp->cpu),
 		   rdp->core_needs_qs);
 	seq_printf(m, " dt=%d/%llx/%d df=%lu",
-		   atomic_read(&rdp->dynticks->dynticks),
+		   rcu_dynticks_snap(rdp->dynticks),
 		   rdp->dynticks->dynticks_nesting,
 		   rdp->dynticks->dynticks_nmi_nesting,
 		   rdp->dynticks_fqs);
-- 
2.5.2

[toc] | [prev] | [next] | [standalone]

#1558883 — [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks momentary-idle operation

This commit is the first step towards full abstraction of all accesses to
the ->dynticks counter, implementing the previously open-coded atomic add
of two in a new rcu_dynticks_momentary_idle() function.  This abstraction
will ease changes to the ->dynticks counter operation.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcu/tree.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index cb4e2056ccf3..14e283c351f6 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -281,6 +281,19 @@ static DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
 #endif /* #ifdef CONFIG_NO_HZ_FULL_SYSIDLE */
 };
 
+/*
+ * Do a double-increment of the ->dynticks counter to emulate a
+ * momentary idle-CPU quiescent state.
+ */
+static void rcu_dynticks_momentary_idle(void)
+{
+	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
+	int special = atomic_add_return(2, &rdtp->dynticks);
+
+	/* It is illegal to call this from idle state. */
+	WARN_ON_ONCE(!(special & 0x1));
+}
+
 DEFINE_PER_CPU_SHARED_ALIGNED(unsigned long, rcu_qs_ctr);
 EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);
 
@@ -300,7 +313,6 @@ EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);
 static void rcu_momentary_dyntick_idle(void)
 {
 	struct rcu_data *rdp;
-	struct rcu_dynticks *rdtp;
 	int resched_mask;
 	struct rcu_state *rsp;
 
@@ -327,10 +339,7 @@ static void rcu_momentary_dyntick_idle(void)
 		 * quiescent state, with no need for this CPU to do anything
 		 * further.
 		 */
-		rdtp = this_cpu_ptr(&rcu_dynticks);
-		smp_mb__before_atomic(); /* Earlier stuff before QS. */
-		atomic_add(2, &rdtp->dynticks);  /* QS. */
-		smp_mb__after_atomic(); /* Later stuff after QS. */
+		rcu_dynticks_momentary_idle();
 		break;
 	}
 }
-- 
2.5.2

[toc] | [prev] | [next] | [standalone]

#1559498 — Re: [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks momentary-idle operation

On Sat, Jan 14, 2017 at 12:54:40AM -0800, Paul E. McKenney wrote:
> This commit is the first step towards full abstraction of all accesses to
> the ->dynticks counter, implementing the previously open-coded atomic add
> of two in a new rcu_dynticks_momentary_idle() function.  This abstraction
> will ease changes to the ->dynticks counter operation.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

This change has an additional effect not documented in the commit
message: it eliminates the smp_mb__before_atomic and
smp_mb__after_atomic calls.  Can you please document that in the commit
message, and explain why that doesn't cause a problem?

> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -281,6 +281,19 @@ static DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
>  #endif /* #ifdef CONFIG_NO_HZ_FULL_SYSIDLE */
>  };
>  
> +/*
> + * Do a double-increment of the ->dynticks counter to emulate a
> + * momentary idle-CPU quiescent state.
> + */
> +static void rcu_dynticks_momentary_idle(void)
> +{
> +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> +	int special = atomic_add_return(2, &rdtp->dynticks);
> +
> +	/* It is illegal to call this from idle state. */
> +	WARN_ON_ONCE(!(special & 0x1));
> +}
> +
>  DEFINE_PER_CPU_SHARED_ALIGNED(unsigned long, rcu_qs_ctr);
>  EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);
>  
> @@ -300,7 +313,6 @@ EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);
>  static void rcu_momentary_dyntick_idle(void)
>  {
>  	struct rcu_data *rdp;
> -	struct rcu_dynticks *rdtp;
>  	int resched_mask;
>  	struct rcu_state *rsp;
>  
> @@ -327,10 +339,7 @@ static void rcu_momentary_dyntick_idle(void)
>  		 * quiescent state, with no need for this CPU to do anything
>  		 * further.
>  		 */
> -		rdtp = this_cpu_ptr(&rcu_dynticks);
> -		smp_mb__before_atomic(); /* Earlier stuff before QS. */
> -		atomic_add(2, &rdtp->dynticks);  /* QS. */
> -		smp_mb__after_atomic(); /* Later stuff after QS. */
> +		rcu_dynticks_momentary_idle();
>  		break;
>  	}
>  }
> -- 
> 2.5.2
> 

[toc] | [prev] | [next] | [standalone]

#1559649 — Re: [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks momentary-idle operation

On Sun, Jan 15, 2017 at 11:39:51PM -0800, Josh Triplett wrote:
> On Sat, Jan 14, 2017 at 12:54:40AM -0800, Paul E. McKenney wrote:
> > This commit is the first step towards full abstraction of all accesses to
> > the ->dynticks counter, implementing the previously open-coded atomic add
> > of two in a new rcu_dynticks_momentary_idle() function.  This abstraction
> > will ease changes to the ->dynticks counter operation.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> This change has an additional effect not documented in the commit
> message: it eliminates the smp_mb__before_atomic and
> smp_mb__after_atomic calls.  Can you please document that in the commit
> message, and explain why that doesn't cause a problem?

The trick is that the old code used the non-value-returning atomic_add(),
which does not imply ordering, hence the smp_mb__before_atomic() and
smp_mb__after_atomic() calls.  The new code uses atomic_add_return(),
which does return a value, and therefore implies full ordering in and
of itself.

How would you like me to proceed?

							Thanx, Paul

> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -281,6 +281,19 @@ static DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
> >  #endif /* #ifdef CONFIG_NO_HZ_FULL_SYSIDLE */
> >  };
> >  
> > +/*
> > + * Do a double-increment of the ->dynticks counter to emulate a
> > + * momentary idle-CPU quiescent state.
> > + */
> > +static void rcu_dynticks_momentary_idle(void)
> > +{
> > +	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
> > +	int special = atomic_add_return(2, &rdtp->dynticks);
> > +
> > +	/* It is illegal to call this from idle state. */
> > +	WARN_ON_ONCE(!(special & 0x1));
> > +}
> > +
> >  DEFINE_PER_CPU_SHARED_ALIGNED(unsigned long, rcu_qs_ctr);
> >  EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);
> >  
> > @@ -300,7 +313,6 @@ EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);
> >  static void rcu_momentary_dyntick_idle(void)
> >  {
> >  	struct rcu_data *rdp;
> > -	struct rcu_dynticks *rdtp;
> >  	int resched_mask;
> >  	struct rcu_state *rsp;
> >  
> > @@ -327,10 +339,7 @@ static void rcu_momentary_dyntick_idle(void)
> >  		 * quiescent state, with no need for this CPU to do anything
> >  		 * further.
> >  		 */
> > -		rdtp = this_cpu_ptr(&rcu_dynticks);
> > -		smp_mb__before_atomic(); /* Earlier stuff before QS. */
> > -		atomic_add(2, &rdtp->dynticks);  /* QS. */
> > -		smp_mb__after_atomic(); /* Later stuff after QS. */
> > +		rcu_dynticks_momentary_idle();
> >  		break;
> >  	}
> >  }
> > -- 
> > 2.5.2
> > 
> 

[toc] | [prev] | [next] | [standalone]

#1559988 — Re: [PATCH tip/core/rcu 1/6] rcu: Abstract the dynticks momentary-idle operation

On Mon, Jan 16, 2017 at 03:22:39AM -0800, Paul E. McKenney wrote:
> On Sun, Jan 15, 2017 at 11:39:51PM -0800, Josh Triplett wrote:
> > On Sat, Jan 14, 2017 at 12:54:40AM -0800, Paul E. McKenney wrote:
> > > This commit is the first step towards full abstraction of all accesses to
> > > the ->dynticks counter, implementing the previously open-coded atomic add
> > > of two in a new rcu_dynticks_momentary_idle() function.  This abstraction
> > > will ease changes to the ->dynticks counter operation.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > This change has an additional effect not documented in the commit
> > message: it eliminates the smp_mb__before_atomic and
> > smp_mb__after_atomic calls.  Can you please document that in the commit
> > message, and explain why that doesn't cause a problem?
> 
> The trick is that the old code used the non-value-returning atomic_add(),
> which does not imply ordering, hence the smp_mb__before_atomic() and
> smp_mb__after_atomic() calls.  The new code uses atomic_add_return(),
> which does return a value, and therefore implies full ordering in and
> of itself.
> 
> How would you like me to proceed?

With the above explanation added to the commit message:

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

[toc] | [prev] | [next] | [standalone]

#1559501

On Sat, Jan 14, 2017 at 12:54:06AM -0800, Paul E. McKenney wrote:
> Hello!
> 
> This series provides dynticks updates:
> 
> 1-4.	Abstract access to the dyntick counter, replacing the current
> 	open-coding of atomic operations.
> 
> 5.	Check cond_resched_rcu_qs() state less often to reduce GP overhead.
> 
> 6.	Adjust FQS offline checks for exact online-CPU detection. 

I replied to patches 1 and 3 with feedback.  For patches 2 and 4-6:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
>  include/linux/rcutiny.h    |    6 +
>  include/trace/events/rcu.h |   10 -
>  kernel/rcu/tree.c          |  245 +++++++++++++++++++++++++++++++--------------
>  kernel/rcu/tree.h          |    2 
>  kernel/rcu/tree_exp.h      |   12 --
>  kernel/rcu/tree_plugin.h   |    2 
>  kernel/rcu/tree_trace.c    |    2 
>  7 files changed, 190 insertions(+), 89 deletions(-)
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web