[PATCH v3] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty

[PATCH v3] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty

[Zqiang]

Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads
enter polling mode. however, due to only insert CPU's rdp which belong to
rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog
kthread have been de-offloaded, these cause the 'nocb_head_rdp' list
served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty,
the rcuog kthread in polling mode not actually do anything. fix it by
exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise
entering polling mode.

Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Zqiang <qiang1.zhang@intel.com>
---
 v1->v2:
 Move rcu_nocb_poll flags check from rdp_offload_toggle() to 
 rcu_nocb_rdp_offload/deoffload(), avoid unnecessary setting of 
 rdp_gp->nocb_gp_sleep flags, because when rcu_nocb_poll is set  the 
 rdp_gp->nocb_gp_sleep is not used.
 
 v2->v3:
 When nocb_head_rdp list is empty. put rcuog kthreads in nocb_gp_wq
 waitqueue to wait offloading.

 kernel/rcu/tree_nocb.h | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index fa8e4f82e60c..a8f574d8850d 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -584,6 +584,14 @@ static int nocb_gp_toggle_rdp(struct rcu_data *rdp,
 	return ret;
 }
 
+static void nocb_gp_sleep(struct rcu_data *my_rdp, int cpu)
+{
+	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
+	swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
+					!READ_ONCE(my_rdp->nocb_gp_sleep));
+	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
+}
+
 /*
  * No-CBs GP kthreads come here to wait for additional callbacks to show up
  * or for grace periods to end.
@@ -701,13 +709,19 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
 		/* Polling, so trace if first poll in the series. */
 		if (gotcbs)
 			trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll"));
-		schedule_timeout_idle(1);
+		if (list_empty(&my_rdp->nocb_head_rdp)) {
+			raw_spin_lock_irqsave(&my_rdp->nocb_gp_lock, flags);
+			if (!my_rdp->nocb_toggling_rdp)
+				WRITE_ONCE(my_rdp->nocb_gp_sleep, true);
+			raw_spin_unlock_irqrestore(&my_rdp->nocb_gp_lock, flags);
+			/* Wait for any offloading rdp */
+			nocb_gp_sleep(my_rdp, cpu);
+		} else {
+			schedule_timeout_idle(1);
+		}
 	} else if (!needwait_gp) {
 		/* Wait for callbacks to appear. */
-		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
-		swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
-				!READ_ONCE(my_rdp->nocb_gp_sleep));
-		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
+		nocb_gp_sleep(my_rdp, cpu);
 	} else {
 		rnp = my_rdp->mynode;
 		trace_rcu_this_gp(rnp, my_rdp, wait_gp_seq, TPS("StartWait"));
-- 
2.25.1

Re: [PATCH v3] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty

[Paul E. McKenney]

On Sat, Jun 11, 2022 at 07:00:44PM +0800, Zqiang wrote:
> Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads
> enter polling mode. however, due to only insert CPU's rdp which belong to
> rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog
> kthread have been de-offloaded, these cause the 'nocb_head_rdp' list
> served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty,
> the rcuog kthread in polling mode not actually do anything. fix it by
> exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise
> entering polling mode.
> 
> Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>

Much better, thank you!  One additional question below.

(And we of course need Frederic's "D'accord" before I send this sort of
thing to mainline.)

							Thanx, Paul

> ---
>  v1->v2:
>  Move rcu_nocb_poll flags check from rdp_offload_toggle() to 
>  rcu_nocb_rdp_offload/deoffload(), avoid unnecessary setting of 
>  rdp_gp->nocb_gp_sleep flags, because when rcu_nocb_poll is set  the 
>  rdp_gp->nocb_gp_sleep is not used.
>  
>  v2->v3:
>  When nocb_head_rdp list is empty. put rcuog kthreads in nocb_gp_wq
>  waitqueue to wait offloading.
> 
>  kernel/rcu/tree_nocb.h | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index fa8e4f82e60c..a8f574d8850d 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -584,6 +584,14 @@ static int nocb_gp_toggle_rdp(struct rcu_data *rdp,
>  	return ret;
>  }
>  
> +static void nocb_gp_sleep(struct rcu_data *my_rdp, int cpu)
> +{
> +	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
> +	swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
> +					!READ_ONCE(my_rdp->nocb_gp_sleep));
> +	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
> +}
> +
>  /*
>   * No-CBs GP kthreads come here to wait for additional callbacks to show up
>   * or for grace periods to end.
> @@ -701,13 +709,19 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
>  		/* Polling, so trace if first poll in the series. */
>  		if (gotcbs)
>  			trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll"));
> -		schedule_timeout_idle(1);
> +		if (list_empty(&my_rdp->nocb_head_rdp)) {
> +			raw_spin_lock_irqsave(&my_rdp->nocb_gp_lock, flags);
> +			if (!my_rdp->nocb_toggling_rdp)

If this "if" condition is false, what prevents this kthread from being
in a CPU-bound loop?

> +				WRITE_ONCE(my_rdp->nocb_gp_sleep, true);
> +			raw_spin_unlock_irqrestore(&my_rdp->nocb_gp_lock, flags);
> +			/* Wait for any offloading rdp */
> +			nocb_gp_sleep(my_rdp, cpu);
> +		} else {
> +			schedule_timeout_idle(1);
> +		}
>  	} else if (!needwait_gp) {
>  		/* Wait for callbacks to appear. */
> -		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
> -		swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
> -				!READ_ONCE(my_rdp->nocb_gp_sleep));
> -		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
> +		nocb_gp_sleep(my_rdp, cpu);
>  	} else {
>  		rnp = my_rdp->mynode;
>  		trace_rcu_this_gp(rnp, my_rdp, wait_gp_seq, TPS("StartWait"));
> -- 
> 2.25.1
> 

RE: [PATCH v3] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty

[Zhang, Qiang1]

On Sat, Jun 11, 2022 at 07:00:44PM +0800, Zqiang wrote:
> Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog 
> kthreads enter polling mode. however, due to only insert CPU's rdp 
> which belong to rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp 
> served by rcuog kthread have been de-offloaded, these cause the 
> 'nocb_head_rdp' list served by rcuog kthread is empty, when the 
> 'nocb_head_rdp' is empty, the rcuog kthread in polling mode not 
> actually do anything. fix it by exiting polling mode when the 
> 'nocb_head_rdp'list is empty, otherwise entering polling mode.
> 
> Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>

>Much better, thank you!  One additional question below.
>
>(And we of course need Frederic's "D'accord" before I send this sort of thing to mainline.)
>
>							Thanx, Paul
>
> ---
>  v1->v2:
>  Move rcu_nocb_poll flags check from rdp_offload_toggle() to  
> rcu_nocb_rdp_offload/deoffload(), avoid unnecessary setting of  
> rdp_gp->nocb_gp_sleep flags, because when rcu_nocb_poll is set  the  
> rdp_gp->nocb_gp_sleep is not used.
>  
>  v2->v3:
>  When nocb_head_rdp list is empty. put rcuog kthreads in nocb_gp_wq  
> waitqueue to wait offloading.
> 
>  kernel/rcu/tree_nocb.h | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h index 
> fa8e4f82e60c..a8f574d8850d 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -584,6 +584,14 @@ static int nocb_gp_toggle_rdp(struct rcu_data *rdp,
>  	return ret;
>  }
>  
> +static void nocb_gp_sleep(struct rcu_data *my_rdp, int cpu) {
> +	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
> +	swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
> +					!READ_ONCE(my_rdp->nocb_gp_sleep));
> +	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep")); }
> +
>  /*
>   * No-CBs GP kthreads come here to wait for additional callbacks to show up
>   * or for grace periods to end.
> @@ -701,13 +709,19 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
>  		/* Polling, so trace if first poll in the series. */
>  		if (gotcbs)
>  			trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll"));
> -		schedule_timeout_idle(1);
> +		if (list_empty(&my_rdp->nocb_head_rdp)) {
> +			raw_spin_lock_irqsave(&my_rdp->nocb_gp_lock, flags);
> +			if (!my_rdp->nocb_toggling_rdp)

>If this "if" condition is false, what prevents this kthread from being
>in a CPU-bound loop?

If this "if" condition is false, (my_rdp->nocb_toggling_rdp is not NULL),  it means the offload action
be executed, because the 'my_rdp->nocb_toggling_rdp' will only be set 'rdp' pointer for (de-)offload. 
It also means this kthread  will insert 'rdp' into 'my_rdp->nocb_head_rdp' list. in the next cycle the
'my_rdp->nocb_head_rdp' is not empty.

Thanks
Zqiang

> +				WRITE_ONCE(my_rdp->nocb_gp_sleep, true);
> +			raw_spin_unlock_irqrestore(&my_rdp->nocb_gp_lock, flags);
> +			/* Wait for any offloading rdp */
> +			nocb_gp_sleep(my_rdp, cpu);
> +		} else {
> +			schedule_timeout_idle(1);
> +		}
>  	} else if (!needwait_gp) {
>  		/* Wait for callbacks to appear. */
> -		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
> -		swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
> -				!READ_ONCE(my_rdp->nocb_gp_sleep));
> -		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
> +		nocb_gp_sleep(my_rdp, cpu);
>  	} else {
>  		rnp = my_rdp->mynode;
>  		trace_rcu_this_gp(rnp, my_rdp, wait_gp_seq, TPS("StartWait"));
> -- 
> 2.25.1
> 

Re: [PATCH v3] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty

[Frederic Weisbecker]

On Sat, Jun 11, 2022 at 07:00:44PM +0800, Zqiang wrote:
> Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads
> enter polling mode. however, due to only insert CPU's rdp which belong to
> rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog
> kthread have been de-offloaded, these cause the 'nocb_head_rdp' list
> served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty,
> the rcuog kthread in polling mode not actually do anything. fix it by
> exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise
> entering polling mode.
> 
> Co-developed-by: Frederic Weisbecker <frederic@kernel.org>

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

Thanks!

(might be worth testing with TREE01 and --bootargs "rcu_gp_poll" )

Re: [PATCH v3] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty

[Paul E. McKenney]

On Tue, Jun 14, 2022 at 01:42:19PM +0200, Frederic Weisbecker wrote:
> On Sat, Jun 11, 2022 at 07:00:44PM +0800, Zqiang wrote:
> > Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads
> > enter polling mode. however, due to only insert CPU's rdp which belong to
> > rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog
> > kthread have been de-offloaded, these cause the 'nocb_head_rdp' list
> > served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty,
> > the rcuog kthread in polling mode not actually do anything. fix it by
> > exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise
> > entering polling mode.
> > 
> > Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
> 
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> 
> Thanks!
> 
> (might be worth testing with TREE01 and --bootargs "rcu_gp_poll" )

A short run passes, so I am queuing this for further review and
testing, thank you both!

With the usual wordsmithing, so please check.

							Thanx, Paul

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

commit 5d3a7cbf3bc24dd5540ecf5be7a3e0e94b9a15d8
Author: Zqiang <qiang1.zhang@intel.com>
Date:   Sat Jun 11 19:00:44 2022 +0800

    rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty
    
    Currently, If the 'rcu_nocb_poll' kernel boot parameter is enabled, all
    rcuog kthreads enter polling mode.  However, if all of a given group
    of rcuo kthreads correspond to CPUs that have been de-offloaded, the
    corresponding rcuog kthread will nonetheless still wake up periodically,
    unnecessarily consuming power and perturbing workloads.  Fortunately,
    this situation is easily detected by the fact that the rcuog kthread's
    CPU's rcu_data structure's ->nocb_head_rdp list is empty.
    
    This commit saves power and avoids unnecessarily perturbing workloads
    by putting an rcuog kthread to sleep during any time period when all of
    its rcuo kthreads' CPUs are de-offloaded.
    
    Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
    Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
    Signed-off-by: Zqiang <qiang1.zhang@intel.com>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index fa8e4f82e60c0..a8f574d8850d2 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -584,6 +584,14 @@ static int nocb_gp_toggle_rdp(struct rcu_data *rdp,
 	return ret;
 }
 
+static void nocb_gp_sleep(struct rcu_data *my_rdp, int cpu)
+{
+	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
+	swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
+					!READ_ONCE(my_rdp->nocb_gp_sleep));
+	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
+}
+
 /*
  * No-CBs GP kthreads come here to wait for additional callbacks to show up
  * or for grace periods to end.
@@ -701,13 +709,19 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
 		/* Polling, so trace if first poll in the series. */
 		if (gotcbs)
 			trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll"));
-		schedule_timeout_idle(1);
+		if (list_empty(&my_rdp->nocb_head_rdp)) {
+			raw_spin_lock_irqsave(&my_rdp->nocb_gp_lock, flags);
+			if (!my_rdp->nocb_toggling_rdp)
+				WRITE_ONCE(my_rdp->nocb_gp_sleep, true);
+			raw_spin_unlock_irqrestore(&my_rdp->nocb_gp_lock, flags);
+			/* Wait for any offloading rdp */
+			nocb_gp_sleep(my_rdp, cpu);
+		} else {
+			schedule_timeout_idle(1);
+		}
 	} else if (!needwait_gp) {
 		/* Wait for callbacks to appear. */
-		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
-		swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
-				!READ_ONCE(my_rdp->nocb_gp_sleep));
-		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
+		nocb_gp_sleep(my_rdp, cpu);
 	} else {
 		rnp = my_rdp->mynode;
 		trace_rcu_this_gp(rnp, my_rdp, wait_gp_seq, TPS("StartWait"));