Re: [RFC 1/3] rcu: Expedite the rcu quiescent state reporting if help needed

From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Cc: linux-kernel@vger.kernel.org,
	Josh Triplett <josh@joshtriplett.org>,
	kernel-team@android.com, Lai Jiangshan <jiangshanlai@gmail.com>,
	linux-kselftest@vger.kernel.org,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	rcu@vger.kernel.org,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Shuah Khan <shuah@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [RFC 1/3] rcu: Expedite the rcu quiescent state reporting if help needed
Date: Mon, 1 Jul 2019 20:47:30 -0700	[thread overview]
Message-ID: <20190702034730.GI26519@linux.ibm.com> (raw)
In-Reply-To: <20190701040415.219001-1-joel@joelfernandes.org>

On Mon, Jul 01, 2019 at 12:04:13AM -0400, Joel Fernandes (Google) wrote:
> The t->rcu_read_unlock_special union's need_qs bit can be set by the
> scheduler tick (in rcu_flavor_sched_clock_irq) to indicate that help is
> needed from the rcu_read_unlock path. When this help arrives however, we
> can do better to speed up the quiescent state reporting which if
> rcu_read_unlock_special::need_qs is set might be quite urgent. Make use
> of this information in deciding when to do heavy-weight softirq raising
> where possible.
> 
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>

Cute thought, but I am going to have to pass on this one.  The reason
is that by the time that ->rcu_read_unlock_special.b.need_qs gets set,
the grace period is already one full second old.  At that point, the
extra tick of waiting is down in the noise.

Right now, we do the extra work if we really are blocking an expedited
grace period (the first two lines of the original condition) or we are
running on a nohz_full CPU (which might never execute a scheduling clock
tick, thus potentially delaying forever).  And expedited grace periods
are supposed to complete in tens or maybe hundreds of microseconds,
assuming the RCU readers are being cooperative, which is a whole
different level of urgent.

Nevertheless, thank you for looking into this!

							Thanx, Paul

> ---
>  kernel/rcu/tree_plugin.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index c588ef98efd3..bff6410fac06 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -622,7 +622,8 @@ static void rcu_read_unlock_special(struct task_struct *t)
>  		t->rcu_read_unlock_special.b.exp_hint = false;
>  		exp = (t->rcu_blocked_node && t->rcu_blocked_node->exp_tasks) ||
>  		      (rdp->grpmask & rnp->expmask) ||
> -		      tick_nohz_full_cpu(rdp->cpu);
> +		      tick_nohz_full_cpu(rdp->cpu)  ||
> +		      t->rcu_read_unlock_special.b.need_qs;
>  		// Need to defer quiescent state until everything is enabled.
>  		if (irqs_were_disabled && use_softirq &&
>  		    (in_interrupt() ||
> -- 
> 2.22.0.410.gd8fdbe21b5-goog
> 