Messages in this thread Patch in this message | | | Date | Tue, 26 Jun 2018 14:38:50 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH tip/core/rcu 06/27] rcu: Mark task as .need_qs less aggressively |
| |
On Tue, Jun 26, 2018 at 11:03:03AM -0700, Paul E. McKenney wrote: > On Tue, Jun 26, 2018 at 07:08:12PM +0200, Peter Zijlstra wrote: > > On Mon, Jun 25, 2018 at 05:34:52PM -0700, Paul E. McKenney wrote: > > > If any scheduling-clock interrupt interrupts an RCU-preempt read-side > > > critical section, the interrupted task's ->rcu_read_unlock_special.b.need_qs > > > field is set. This causes the outermost rcu_read_unlock() to incur the > > > extra overhead of calling into rcu_read_unlock_special(). This commit > > > reduces that overhead by setting ->rcu_read_unlock_special.b.need_qs only > > > if the grace period has been in effect for more than one second. > > > > Even less agressive is never setting it at all. > > True, but if the CPU has been in an RCU read-side critical section for > a full second (which is the case with high probability when .b.need_qs > is set after this change), we might want to respond to the end of that > critical section sooner rather than later. > > > Changelog fails to explain why not setting it every tick is correct, nor > > why 1s is a 'safe' value to use. > > The RCU CPU stall warning cannot be set to less than 3s, so 1s is > reasonable. It is a tradeoff -- setting it lower causes a greater > fraction of RCU read-side critical sections to incur extra overhead at > rcu_read_unlock() time, while setting it higher keeps a lazy approach > to reporting the quiescent state to core RCU for longer critical sections. > > The upcoming RCU-bh/RCU-preempt/RCU-sched consolidation will raise > contention and overhead, so this is one of several things done to > lower overhead and contention to compensate for that.
And does the following updated commit log help?
Thanx, Paul
------------------------------------------------------------------------
commit aaf8af680740afc363583a6ed9549b08b613dd3d Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Date: Wed May 16 14:41:41 2018 -0700
rcu: Mark task as .need_qs less aggressively If any scheduling-clock interrupt interrupts an RCU-preempt read-side critical section, the interrupted task's ->rcu_read_unlock_special.b.need_qs field is set. This causes the outermost rcu_read_unlock() to incur the extra overhead of calling into rcu_read_unlock_special(). This commit reduces that overhead by setting ->rcu_read_unlock_special.b.need_qs only if the grace period has been in effect for more than one second. Why one second? Because this is comfortably smaller than the minimum RCU CPU stall-warning timeout of three seconds, but long enough that the .need_qs marking should happen quite rarely. And if your RCU read-side critical section has run on-CPU for a full second, it is not unreasonable to invest some CPU time in ending the grace period quickly. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index dbfe90191e19..0239cf8a4be6 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -730,6 +730,7 @@ rcu_preempt_check_blocked_tasks(struct rcu_state *rsp, struct rcu_node *rnp) */ static void rcu_preempt_check_callbacks(void) { + struct rcu_state *rsp = &rcu_preempt_state; struct task_struct *t = current; if (t->rcu_read_lock_nesting == 0) { @@ -738,7 +739,9 @@ static void rcu_preempt_check_callbacks(void) } if (t->rcu_read_lock_nesting > 0 && __this_cpu_read(rcu_data_p->core_needs_qs) && - __this_cpu_read(rcu_data_p->cpu_no_qs.b.norm)) + __this_cpu_read(rcu_data_p->cpu_no_qs.b.norm) && + !t->rcu_read_unlock_special.b.need_qs && + time_after(jiffies, rsp->gp_start + HZ)) t->rcu_read_unlock_special.b.need_qs = true; }
| |