Re: [PATCH] rcu: Call wake_nocb_leader_defer() with 'FORCE' when nocb_q_count is high

[Paul E. McKenney]

On Wed, Feb 28, 2018 at 02:49:02PM +0900, Byungchul Park wrote:
> If many callbacks have been queued and waking up the nocb leader should
> be deferred, then we should wake up the leader unconditionally when
> it's safe to do so.
> 
> It was well managed in commit fbce7497ee(rcu: Parallelize and economize
> NOCB kthread wakeups) at first, but the commit 8be6e1b15c(rcu: Use timer
> as backstop for NOCB deferred wakeups) made a mistake which passed
> RCU_NOCB_WAKE instead of RCU_NOCB_WAKE_FORCE as a parameter. Fix it.
> 
> Signed-off-by: Byungchul Park <byungchul.park@xxxxxxx>

Good eyes and good fix, thank you!!!

Queued for v4.18 and for testing and further review.

I did update the commit log, please see below and please let me know if
I messed anything up.

							Thanx, Paul

> ---
>  kernel/rcu/tree_plugin.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index b0d7f9b..c06cf87 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1987,7 +1987,7 @@ static void __call_rcu_nocb_enqueue(struct rcu_data *rdp,
>  			trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
>  					    TPS("WakeOvf"));
>  		} else {
> -			wake_nocb_leader_defer(rdp, RCU_NOCB_WAKE,
> +			wake_nocb_leader_defer(rdp, RCU_NOCB_WAKE_FORCE,
>  					       TPS("WakeOvfIsDeferred"));
>  		}
>  		rdp->qlen_last_fqs_check = LONG_MAX / 2;

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

commit 5f56c391238eb237b80c8a6f2f91d97e70d547f1
Author: Byungchul Park <byungchul.park@xxxxxxx>
Date:   Wed Feb 28 14:49:02 2018 +0900

    rcu: Call wake_nocb_leader_defer() with 'FORCE' when nocb_q_count is high
    
    If an excessive number of callbacks have been queued, but the NOCB
    leader kthread's wakeup must be deferred, then we should wake up the
    leader unconditionally once it is safe to do so.
    
    This was handled correctly in commit fbce7497ee ("rcu: Parallelize and
    economize NOCB kthread wakeups"), but then commit 8be6e1b15c ("rcu:
    Use timer as backstop for NOCB deferred wakeups") passed RCU_NOCB_WAKE
    instead of the correct RCU_NOCB_WAKE_FORCE to wake_nocb_leader_defer().
    As an interesting aside, RCU_NOCB_WAKE_FORCE is never passed to anything,
    which should have been taken as a hint.  ;-)
    
    This commit therefore passes RCU_NOCB_WAKE_FORCE instead of RCU_NOCB_WAKE
    to wake_nocb_leader_defer() when a callback is queued onto a NOCB CPU
    that already has an excessive number of callbacks pending.
    
    Signed-off-by: Byungchul Park <byungchul.park@xxxxxxx>
    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index fb3d20d868ed..f4ea2d8027b4 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1987,7 +1987,7 @@ static void __call_rcu_nocb_enqueue(struct rcu_data *rdp,
 			trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
 					    TPS("WakeOvf"));
 		} else {
-			wake_nocb_leader_defer(rdp, RCU_NOCB_WAKE,
+			wake_nocb_leader_defer(rdp, RCU_NOCB_WAKE_FORCE,
 					       TPS("WakeOvfIsDeferred"));
 		}
 		rdp->qlen_last_fqs_check = LONG_MAX / 2;