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

Groups > linux.kernel > #1683892 > unrolled thread

[RFC PATCH v1 00/11] Create fast idle path for short idle periods

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH v1 00/11] Create fast idle path for short idle periods Aubrey Li <aubrey.li@intel.com> - 2017-07-10 03:50 +0200
    [RFC PATCH v1 04/11] sched/idle: make the fast idle path for short idle periods Aubrey Li <aubrey.li@intel.com> - 2017-07-10 03:50 +0200
      Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-11 15:00 +0200
        Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods Frederic Weisbecker <fweisbec@gmail.com> - 2017-07-11 18:40 +0200
          Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-11 20:20 +0200
            Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-12 05:30 +0200
              Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 07:10 +0200
                Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-12 07:30 +0200
          Re: [RFC PATCH v1 04/11] sched/idle: make the fast idle path for  short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 14:30 +0200
    [RFC PATCH v1 05/11] cpuidle: update idle statistics before cpuidle governor Aubrey Li <aubrey.li@intel.com> - 2017-07-10 03:50 +0200
    [RFC PATCH v1 08/11] cpuidle: menu: remove reduplicative implementation Aubrey Li <aubrey.li@intel.com> - 2017-07-10 04:00 +0200
    [RFC PATCH v1 07/11] cpuidle: make idle residency update more generic Aubrey Li <aubrey.li@intel.com> - 2017-07-10 04:00 +0200
    [RFC PATCH v1 03/11] cpuidle: introduce cpuidle governor for idle prediction Aubrey Li <aubrey.li@intel.com> - 2017-07-10 04:00 +0200
      Re: [RFC PATCH v1 03/11] cpuidle: introduce cpuidle governor for  idle prediction Peter Zijlstra <peterz@infradead.org> - 2017-07-12 14:20 +0200
    [RFC PATCH v1 09/11] cpuidle: menu: feed cpuidle prediction to menu governor Aubrey Li <aubrey.li@intel.com> - 2017-07-10 04:00 +0200
    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-10 10:50 +0200
      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Wanpeng Li <kernellwp@gmail.com> - 2017-07-10 11:40 +0200
      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-10 16:00 +0200
      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-10 16:50 +0200
        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-10 18:50 +0200
          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-10 19:30 +0200
            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-11 06:50 +0200
              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-11 11:50 +0200
                Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Frederic Weisbecker <fweisbec@gmail.com> - 2017-07-11 18:10 +0200
                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-11 18:40 +0200
                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-11 20:10 +0200
                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 14:00 +0200
                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 18:00 +0200
                          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 19:50 +0200
                            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 21:00 +0200
                              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 21:10 +0200
                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 14:30 +0200
                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 18:00 +0200
                          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 19:20 +0200
                            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 20:00 +0200
                              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 21:00 +0200
                            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-12 20:50 +0200
                              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-13 10:40 +0200
                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-12 06:20 +0200
                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 10:40 +0200
                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-12 23:40 +0200
                          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-13 10:40 +0200
                            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-13 16:50 +0200
                              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-13 17:00 +0200
                                Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-13 17:20 +0200
                                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-13 20:30 +0200
                                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-14 06:00 +0200
                                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-14 17:40 +0200
                                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-14 18:00 +0200
                                          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-14 18:10 +0200
                                            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-17 11:30 +0200
                                              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-17 15:50 +0200
                                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-14 18:00 +0200
                                          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-14 18:10 +0200
                                            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-14 18:30 +0200
                                              Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-17 21:30 +0200
                                                Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Arjan van de Ven <arjan@linux.intel.com> - 2017-07-17 21:30 +0200
                                                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Thomas Gleixner <tglx@linutronix.de> - 2017-07-17 21:50 +0200
                                                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Arjan van de Ven <arjan@linux.intel.com> - 2017-07-17 22:00 +0200
                                                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Thomas Gleixner <tglx@linutronix.de> - 2017-07-17 22:10 +0200
                                                Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Arjan van de Ven <arjan@linux.intel.com> - 2017-07-17 21:50 +0200
                                                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Thomas Gleixner <tglx@linutronix.de> - 2017-07-17 22:00 +0200
                                                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Arjan van de Ven <arjan@linux.intel.com> - 2017-07-17 22:00 +0200
                                                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-18 05:30 +0200
                                                Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-18 05:20 +0200
                                                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-18 06:50 +0200
                                                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Thomas Gleixner <tglx@linutronix.de> - 2017-07-18 08:50 +0200
                                                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Andi Kleen <ak@linux.intel.com> - 2017-07-18 09:00 +0200
                                                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Thomas Gleixner <tglx@linutronix.de> - 2017-07-18 09:20 +0200
                                                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Thomas Gleixner <tglx@linutronix.de> - 2017-07-18 09:30 +0200
                                                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-18 09:00 +0200
                                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Arjan van de Ven <arjan@linux.intel.com> - 2017-07-14 18:00 +0200
                                Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-13 17:30 +0200
                                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-14 05:50 +0200
                                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-14 06:10 +0200
                                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-17 15:30 +0200
                                        Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-17 16:00 +0200
                                          Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-17 16:10 +0200
                      Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 14:20 +0200
                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle  periods Christoph Lameter <cl@linux.com> - 2017-07-11 20:00 +0200
                    Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-12 04:10 +0200
                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods "Li, Aubrey" <aubrey.li@linux.intel.com> - 2017-07-12 04:40 +0200
                  Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-12 20:20 +0200
            Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Peter Zijlstra <peterz@infradead.org> - 2017-07-11 11:10 +0200

Page 2 of 5 — ← Prev page 1 [2] 3 4 5  Next page →

#1684490

On Mon, Jul 10, 2017 at 06:42:06PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 10, 2017 at 07:46:09AM -0700, Andi Kleen wrote:
> > > So how much of the gain is simply due to skipping NOHZ? Mike used to
> > > carry a patch that would throttle NOHZ. And that is a _far_ smaller and
> > > simpler patch to do.
> > 
> > Have you ever looked at a ftrace or PT trace of the idle entry?
> > 
> > There's just too much stuff going on there. NOHZ is just the tip
> > of the iceberg.
> 
> I have, and last time I did the actual poking at the LAPIC (to make NOHZ
> happen) was by far the slowest thing happening.

That must have been a long time ago because modern systems use TSC deadline
for a very long time ...  

It's still slow, but not as slow as the LAPIC.

> Data to indicate what hurts how much would be a very good addition to
> the Changelogs. Clearly you have some, you really should have shared.

Aubrey?

-Andi

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

#1684770

On 2017/7/11 1:27, Andi Kleen wrote:
> On Mon, Jul 10, 2017 at 06:42:06PM +0200, Peter Zijlstra wrote:
>> On Mon, Jul 10, 2017 at 07:46:09AM -0700, Andi Kleen wrote:
>>>> So how much of the gain is simply due to skipping NOHZ? Mike used to
>>>> carry a patch that would throttle NOHZ. And that is a _far_ smaller and
>>>> simpler patch to do.
>>>
>>> Have you ever looked at a ftrace or PT trace of the idle entry?
>>>
>>> There's just too much stuff going on there. NOHZ is just the tip
>>> of the iceberg.
>>
>> I have, and last time I did the actual poking at the LAPIC (to make NOHZ
>> happen) was by far the slowest thing happening.
> 
> That must have been a long time ago because modern systems use TSC deadline
> for a very long time ...  
> 
> It's still slow, but not as slow as the LAPIC.
> 
>> Data to indicate what hurts how much would be a very good addition to
>> the Changelogs. Clearly you have some, you really should have shared.
>
Here is an article indicates why we need to improve this:
https://cacm.acm.org/magazines/2017/4/215032-attack-of-the-killer-microseconds/fulltext

Given that we have a few new low-latency I/O devices like Xpoint 3D memory,
25/40GB Ethernet, etc, this proposal targets to improve the latency of
microsecond(us)-scale events as well.

Basically we are looking at how much we can improve(instead of what hurts),
the data is against v4.8.8.

In the idle loop,

- quiet_vmstat costs 5562ns - 6296ns
- tick_nohz_idle_enter costs 7058ns - 10726ns
- totally from arch_cpu_idle_enter entry to arch_cpu_idle_exit return costs
  9122ns - 15318ns.
  --In this period, rcu_idle_enter costs 1985ns - 2262ns, rcu_idle_exit costs
    1813ns - 3507ns
- tick_nohz_idle_exit costs 8372ns - 20850ns

Benchmark fio on a NVMe disk shows 3-4% improvement due to skipping nohz, extra
1-2% improvement overall

Benchmark netperf loopback in TCP Request-Response mode shows 6-7% improvement
due to skipping nohz, extra 2-3% improvement overall

Note, the data includes measurement overhead, and it could be varied on the 
different platforms, different CPU frequency, and different workload, but they
are consistent once the testing configuration is fixed.

Thanks,
-Aubrey

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

#1684931

On Tue, Jul 11, 2017 at 12:40:06PM +0800, Li, Aubrey wrote:
> > On Mon, Jul 10, 2017 at 06:42:06PM +0200, Peter Zijlstra wrote:

> >> Data to indicate what hurts how much would be a very good addition to
> >> the Changelogs. Clearly you have some, you really should have shared.

> In the idle loop,
> 
> - quiet_vmstat costs 5562ns - 6296ns

Urgh, that thing is horrible, also I think its placed wrong. The comment
near that function says it should be called when we enter NOHZ.

Which suggests something like so:

---
 kernel/sched/idle.c      | 1 -
 kernel/time/tick-sched.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 6c23e30c0e5c..ef63adce0c9c 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -219,7 +219,6 @@ static void do_idle(void)
 	 */
 
 	__current_set_polling();
-	quiet_vmstat();
 	tick_nohz_idle_enter();
 
 	while (!need_resched()) {
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index c7a899c5ce64..eb0e9753db8f 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -787,6 +787,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
 	if (!ts->tick_stopped) {
 		calc_load_nohz_start();
 		cpu_load_update_nohz_start();
+		quiet_vmstat();
 
 		ts->last_tick = hrtimer_get_expires(&ts->sched_timer);
 		ts->tick_stopped = 1;


> - tick_nohz_idle_enter costs 7058ns - 10726ns
> - tick_nohz_idle_exit costs 8372ns - 20850ns

Right, those are horrible expensive, but skipping them isn't 'hard', the
only tricky bit is finding a condition that makes sense.

See Mike's patch: https://patchwork.kernel.org/patch/2839221/

Combined with the above, and possibly a better condition, that should
get rid of most of this.

> - totally from arch_cpu_idle_enter entry to arch_cpu_idle_exit return costs
>   9122ns - 15318ns.
>   --In this period, rcu_idle_enter costs 1985ns - 2262ns, rcu_idle_exit costs
>     1813ns - 3507ns

Is that the POPF being painful? or something else?

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

#1685174

On Tue, Jul 11, 2017 at 11:41:57AM +0200, Peter Zijlstra wrote:
> On Tue, Jul 11, 2017 at 12:40:06PM +0800, Li, Aubrey wrote:
> > > On Mon, Jul 10, 2017 at 06:42:06PM +0200, Peter Zijlstra wrote:
> 
> > >> Data to indicate what hurts how much would be a very good addition to
> > >> the Changelogs. Clearly you have some, you really should have shared.
> 
> > In the idle loop,
> > 
> > - quiet_vmstat costs 5562ns - 6296ns
> 
> Urgh, that thing is horrible, also I think its placed wrong. The comment
> near that function says it should be called when we enter NOHZ.
> 
> Which suggests something like so:
> 
> ---
>  kernel/sched/idle.c      | 1 -
>  kernel/time/tick-sched.c | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 6c23e30c0e5c..ef63adce0c9c 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -219,7 +219,6 @@ static void do_idle(void)
>  	 */
>  
>  	__current_set_polling();
> -	quiet_vmstat();
>  	tick_nohz_idle_enter();
>  
>  	while (!need_resched()) {
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index c7a899c5ce64..eb0e9753db8f 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -787,6 +787,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
>  	if (!ts->tick_stopped) {
>  		calc_load_nohz_start();
>  		cpu_load_update_nohz_start();
> +		quiet_vmstat();

This patch seems to make sense. Christoph?

>  
>  		ts->last_tick = hrtimer_get_expires(&ts->sched_timer);
>  		ts->tick_stopped = 1;
> 
> 
> > - tick_nohz_idle_enter costs 7058ns - 10726ns
> > - tick_nohz_idle_exit costs 8372ns - 20850ns
> 
> Right, those are horrible expensive, but skipping them isn't 'hard', the
> only tricky bit is finding a condition that makes sense.

Note you can statically disable it with nohz=0 boot parameter.

> 
> See Mike's patch: https://patchwork.kernel.org/patch/2839221/
> 
> Combined with the above, and possibly a better condition, that should
> get rid of most of this.

Such a patch could work well if the decision from the scheduler to not stop the tick
happens on idle entry.

Now if sched_needs_cpu() first allows to stop the tick then refuses it later
in the end of an idle IRQ, this won't have the desired effect. As long as ts->tick_stopped=1,
it stays so until we really restart the tick. So the whole costly nohz machinery stays on.

I guess it doesn't matter though, as we are talking about making fast idle entry so the
decision not to stop the tick is likely to be done once on idle entry, when ts->tick_stopped=0.

One exception though: if the tick is already stopped when we enter idle (full nohz case). And
BTW stopping the tick outside idle shouldn't be concerned here.

So I'd rather put that on can_stop_idle_tick().

> 
> > - totally from arch_cpu_idle_enter entry to arch_cpu_idle_exit return costs
> >   9122ns - 15318ns.
> >   --In this period, rcu_idle_enter costs 1985ns - 2262ns, rcu_idle_exit costs
> >     1813ns - 3507ns
> 
> Is that the POPF being painful? or something else?

Probably that and the atomic_add_return().

Thanks.

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

#1685202

On Tue, Jul 11, 2017 at 06:09:27PM +0200, Frederic Weisbecker wrote:

> > > - tick_nohz_idle_enter costs 7058ns - 10726ns
> > > - tick_nohz_idle_exit costs 8372ns - 20850ns
> > 
> > Right, those are horrible expensive, but skipping them isn't 'hard', the
> > only tricky bit is finding a condition that makes sense.
> 
> Note you can statically disable it with nohz=0 boot parameter.

Yeah, but that's bad for power usage, nobody wants that.

> > See Mike's patch: https://patchwork.kernel.org/patch/2839221/
> > 
> > Combined with the above, and possibly a better condition, that should
> > get rid of most of this.
> 
> Such a patch could work well if the decision from the scheduler to not stop the tick
> happens on idle entry.
> 
> Now if sched_needs_cpu() first allows to stop the tick then refuses it later
> in the end of an idle IRQ, this won't have the desired effect. As long as ts->tick_stopped=1,
> it stays so until we really restart the tick. So the whole costly nohz machinery stays on.
> 
> I guess it doesn't matter though, as we are talking about making fast idle entry so the
> decision not to stop the tick is likely to be done once on idle entry, when ts->tick_stopped=0.
> 
> One exception though: if the tick is already stopped when we enter idle (full nohz case). And
> BTW stopping the tick outside idle shouldn't be concerned here.
> 
> So I'd rather put that on can_stop_idle_tick().

Mike's patch much predates the existence of that function I think ;-) But
sure..

> > 
> > > - totally from arch_cpu_idle_enter entry to arch_cpu_idle_exit return costs
> > >   9122ns - 15318ns.
> > >   --In this period, rcu_idle_enter costs 1985ns - 2262ns, rcu_idle_exit costs
> > >     1813ns - 3507ns
> > 
> > Is that the POPF being painful? or something else?
> 
> Probably that and the atomic_add_return().

I got properly lost in the RCU machinery. It wasn't at all clear to me
if rcu_eqs_enter_common() was a slow-path function or not.

Also, RCU_FAST_NO_HZ will make a fairly large difference here.. Paul
what's the state of that thing, do we actually want that or not?


But I think we can at the very least do this; it only gets called from
kernel/sched/idle.c and both callsites have IRQs explicitly disabled by
that point.


diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 51d4c3acf32d..dccf2dc8155a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -843,13 +843,8 @@ static void rcu_eqs_enter(bool user)
  */
 void rcu_idle_enter(void)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
 	rcu_eqs_enter(false);
-	local_irq_restore(flags);
 }
-EXPORT_SYMBOL_GPL(rcu_idle_enter);
 
 #ifdef CONFIG_NO_HZ_FULL
 /**

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

#1685268

On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 11, 2017 at 06:09:27PM +0200, Frederic Weisbecker wrote:
> 
> > > > - tick_nohz_idle_enter costs 7058ns - 10726ns
> > > > - tick_nohz_idle_exit costs 8372ns - 20850ns
> > > 
> > > Right, those are horrible expensive, but skipping them isn't 'hard', the
> > > only tricky bit is finding a condition that makes sense.
> > 
> > Note you can statically disable it with nohz=0 boot parameter.
> 
> Yeah, but that's bad for power usage, nobody wants that.
> 
> > > See Mike's patch: https://patchwork.kernel.org/patch/2839221/
> > > 
> > > Combined with the above, and possibly a better condition, that should
> > > get rid of most of this.
> > 
> > Such a patch could work well if the decision from the scheduler to not stop the tick
> > happens on idle entry.
> > 
> > Now if sched_needs_cpu() first allows to stop the tick then refuses it later
> > in the end of an idle IRQ, this won't have the desired effect. As long as ts->tick_stopped=1,
> > it stays so until we really restart the tick. So the whole costly nohz machinery stays on.
> > 
> > I guess it doesn't matter though, as we are talking about making fast idle entry so the
> > decision not to stop the tick is likely to be done once on idle entry, when ts->tick_stopped=0.
> > 
> > One exception though: if the tick is already stopped when we enter idle (full nohz case). And
> > BTW stopping the tick outside idle shouldn't be concerned here.
> > 
> > So I'd rather put that on can_stop_idle_tick().
> 
> Mike's patch much predates the existence of that function I think ;-) But
> sure..
> 
> > > 
> > > > - totally from arch_cpu_idle_enter entry to arch_cpu_idle_exit return costs
> > > >   9122ns - 15318ns.
> > > >   --In this period, rcu_idle_enter costs 1985ns - 2262ns, rcu_idle_exit costs
> > > >     1813ns - 3507ns
> > > 
> > > Is that the POPF being painful? or something else?
> > 
> > Probably that and the atomic_add_return().
> 
> I got properly lost in the RCU machinery. It wasn't at all clear to me
> if rcu_eqs_enter_common() was a slow-path function or not.

It is called on pretty much every transition to idle.

> Also, RCU_FAST_NO_HZ will make a fairly large difference here.. Paul
> what's the state of that thing, do we actually want that or not?

If you are battery powered and don't have tight real-time latency
constraints, you want it -- it has represent a 30-40% boost in battery
lifetime for some low-utilization battery-powered devices.  Otherwise,
probably not.

> But I think we can at the very least do this; it only gets called from
> kernel/sched/idle.c and both callsites have IRQs explicitly disabled by
> that point.
> 
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 51d4c3acf32d..dccf2dc8155a 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -843,13 +843,8 @@ static void rcu_eqs_enter(bool user)
>   */
>  void rcu_idle_enter(void)
>  {
> -	unsigned long flags;
> -
> -	local_irq_save(flags);

With this addition, I am all for it:

RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");

If you are OK with this addition, may I please have your Signed-off-by?

							Thanx, Paul

>  	rcu_eqs_enter(false);
> -	local_irq_restore(flags);
>  }
> -EXPORT_SYMBOL_GPL(rcu_idle_enter);
> 
>  #ifdef CONFIG_NO_HZ_FULL
>  /**
> 

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

#1685726

On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote:
> On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:

> > But I think we can at the very least do this; it only gets called from
> > kernel/sched/idle.c and both callsites have IRQs explicitly disabled by
> > that point.
> > 
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 51d4c3acf32d..dccf2dc8155a 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -843,13 +843,8 @@ static void rcu_eqs_enter(bool user)
> >   */
> >  void rcu_idle_enter(void)
> >  {
> > -	unsigned long flags;
> > -
> > -	local_irq_save(flags);
> 
> With this addition, I am all for it:
> 
> RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");
> 
> If you are OK with this addition, may I please have your Signed-off-by?

Sure,

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> 
> >  	rcu_eqs_enter(false);
> > -	local_irq_restore(flags);
> >  }
> > -EXPORT_SYMBOL_GPL(rcu_idle_enter);
> > 
> >  #ifdef CONFIG_NO_HZ_FULL
> >  /**
> > 
> 

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

#1685872

On Wed, Jul 12, 2017 at 01:54:51PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote:
> > On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:
> 
> > > But I think we can at the very least do this; it only gets called from
> > > kernel/sched/idle.c and both callsites have IRQs explicitly disabled by
> > > that point.
> > > 
> > > 
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 51d4c3acf32d..dccf2dc8155a 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -843,13 +843,8 @@ static void rcu_eqs_enter(bool user)
> > >   */
> > >  void rcu_idle_enter(void)
> > >  {
> > > -	unsigned long flags;
> > > -
> > > -	local_irq_save(flags);
> > 
> > With this addition, I am all for it:
> > 
> > RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");
> > 
> > If you are OK with this addition, may I please have your Signed-off-by?
> 
> Sure,
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Very good, I have queued the patch below.  I left out the removal of
the export as I need to work out why the export was there.  If it turns
out not to be needed, I will remove the related ones as well.

Fair enough?

							Thanx, Paul

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

commit 95f3e587ce6388028a51f0c852800fca944e7032
Author: Peter Zijlstra (Intel) <peterz@infradead.org>
Date:   Wed Jul 12 07:59:54 2017 -0700

    rcu: Make rcu_idle_enter() rely on callers disabling irqs
    
    All callers to rcu_idle_enter() have irqs disabled, so there is no
    point in rcu_idle_enter disabling them again.  This commit therefore
    replaces the irq disabling with a RCU_LOCKDEP_WARN().
    
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 6de6b1c5ee53..c78b076653ce 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -840,11 +840,8 @@ static void rcu_eqs_enter(bool user)
  */
 void rcu_idle_enter(void)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
+	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");
 	rcu_eqs_enter(false);
-	local_irq_restore(flags);
 }
 EXPORT_SYMBOL_GPL(rcu_idle_enter);
 

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

#1685935

On Wed, Jul 12, 2017 at 08:56:51AM -0700, Paul E. McKenney wrote:
> Very good, I have queued the patch below.  I left out the removal of
> the export as I need to work out why the export was there.  If it turns
> out not to be needed, I will remove the related ones as well.

'git grep rcu_idle_enter' shows no callers other than
kernel/sched/idle.c. Which seems a clear indication its not needed.

You also have to ask yourself, do I want joe module author to ever call
this. To which I suspect the answer is: hell no ;-)

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

#1685976

On Wed, Jul 12, 2017 at 07:46:42PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 12, 2017 at 08:56:51AM -0700, Paul E. McKenney wrote:
> > Very good, I have queued the patch below.  I left out the removal of
> > the export as I need to work out why the export was there.  If it turns
> > out not to be needed, I will remove the related ones as well.
> 
> 'git grep rcu_idle_enter' shows no callers other than
> kernel/sched/idle.c. Which seems a clear indication its not needed.
> 
> You also have to ask yourself, do I want joe module author to ever call
> this. To which I suspect the answer is: hell no ;-)

The other question is "why did I do this in the first place".

The only case where there will turn out to have been a still-valid reason
is if I remove it without checking first.  ;-)

						Thanx, Paul

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

#1685980

On Wed, Jul 12, 2017 at 11:53:06AM -0700, Paul E. McKenney wrote:
> On Wed, Jul 12, 2017 at 07:46:42PM +0200, Peter Zijlstra wrote:
> > On Wed, Jul 12, 2017 at 08:56:51AM -0700, Paul E. McKenney wrote:
> > > Very good, I have queued the patch below.  I left out the removal of
> > > the export as I need to work out why the export was there.  If it turns
> > > out not to be needed, I will remove the related ones as well.
> > 
> > 'git grep rcu_idle_enter' shows no callers other than
> > kernel/sched/idle.c. Which seems a clear indication its not needed.
> > 
> > You also have to ask yourself, do I want joe module author to ever call
> > this. To which I suspect the answer is: hell no ;-)
> 
> The other question is "why did I do this in the first place".
> 
> The only case where there will turn out to have been a still-valid reason
> is if I remove it without checking first.  ;-)

And I have now checked.  Please see below.

							Thanx, Paul

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

commit 31b2fd02abe5f036d7e83461bd19bbca3636d62e
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Wed Jul 12 11:55:21 2017 -0700

    rcu: Remove exports from rcu_idle_exit() and rcu_idle_enter()
    
    The rcu_idle_exit() and rcu_idle_enter() functions are exported because
    they were originally used by RCU_NONIDLE(), which was intended to
    be usable from modules.  However, RCU_NONIDLE() now instead uses
    rcu_irq_enter_irqson() and rcu_irq_exit_irqson(), which are not
    exported, and there have been no complaints.
    
    This commit therefore removes the exports from rcu_idle_exit() and
    rcu_idle_enter().
    
    Reported-by: Peter Zijlstra <peterz@infradead.org>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0798d2585e87..860d1c147606 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -843,7 +843,6 @@ void rcu_idle_enter(void)
 	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");
 	rcu_eqs_enter(false);
 }
-EXPORT_SYMBOL_GPL(rcu_idle_enter);
 
 #ifdef CONFIG_NO_HZ_FULL
 /**
@@ -976,7 +975,6 @@ void rcu_idle_exit(void)
 	rcu_eqs_exit(false);
 	local_irq_restore(flags);
 }
-EXPORT_SYMBOL_GPL(rcu_idle_exit);
 
 #ifdef CONFIG_NO_HZ_FULL
 /**

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

#1685733

On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote:
> On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:
> > Also, RCU_FAST_NO_HZ will make a fairly large difference here.. Paul
> > what's the state of that thing, do we actually want that or not?
> 
> If you are battery powered and don't have tight real-time latency
> constraints, you want it -- it has represent a 30-40% boost in battery
> lifetime for some low-utilization battery-powered devices.  Otherwise,
> probably not.

Would it make sense to hook that off of tick_nohz_idle_enter(); in
specific the part where we actually stop the tick; instead of every
idle?

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

#1685879

On Wed, Jul 12, 2017 at 02:22:49PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote:
> > On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:
> > > Also, RCU_FAST_NO_HZ will make a fairly large difference here.. Paul
> > > what's the state of that thing, do we actually want that or not?
> > 
> > If you are battery powered and don't have tight real-time latency
> > constraints, you want it -- it has represent a 30-40% boost in battery
> > lifetime for some low-utilization battery-powered devices.  Otherwise,
> > probably not.
> 
> Would it make sense to hook that off of tick_nohz_idle_enter(); in
> specific the part where we actually stop the tick; instead of every
> idle?

The actions RCU takes on RCU_FAST_NO_HZ depend on the current state of
the CPU's callback lists, so it seems to me that the decision has to
be made on each idle entry.

Now it might be possible to make the checks more efficient, and doing
that is on my list.

Or am I missing your point?

							Thanx, Paul

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

#1685919

On Wed, Jul 12, 2017 at 08:54:58AM -0700, Paul E. McKenney wrote:
> On Wed, Jul 12, 2017 at 02:22:49PM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote:
> > > On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:
> > > > Also, RCU_FAST_NO_HZ will make a fairly large difference here.. Paul
> > > > what's the state of that thing, do we actually want that or not?
> > > 
> > > If you are battery powered and don't have tight real-time latency
> > > constraints, you want it -- it has represent a 30-40% boost in battery
> > > lifetime for some low-utilization battery-powered devices.  Otherwise,
> > > probably not.
> > 
> > Would it make sense to hook that off of tick_nohz_idle_enter(); in
> > specific the part where we actually stop the tick; instead of every
> > idle?
> 
> The actions RCU takes on RCU_FAST_NO_HZ depend on the current state of
> the CPU's callback lists, so it seems to me that the decision has to
> be made on each idle entry.
> 
> Now it might be possible to make the checks more efficient, and doing
> that is on my list.
> 
> Or am I missing your point?

Could be I'm just not remembering how all that works.. But I was
wondering if we can do the expensive bits if we've decided to actually
go NOHZ and avoid doing it on every idle entry.

IIRC the RCU fast NOHZ bits try and flush the callback list (or paw it
off to another CPU?) such that we can go NOHZ sooner. Having a !empty
callback list avoid NOHZ from happening.

Now if we've already decided we can't in fact go NOHZ due to other
concerns, flushing the callback list is pointless work. So I'm thinking
we can find a better place to do this.

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

#1685940

On Wed, Jul 12, 2017 at 07:17:56PM +0200, Peter Zijlstra wrote:
> Could be I'm just not remembering how all that works.. But I was
> wondering if we can do the expensive bits if we've decided to actually
> go NOHZ and avoid doing it on every idle entry.
> 
> IIRC the RCU fast NOHZ bits try and flush the callback list (or paw it
> off to another CPU?) such that we can go NOHZ sooner. Having a !empty
> callback list avoid NOHZ from happening.
> 
> Now if we've already decided we can't in fact go NOHZ due to other
> concerns, flushing the callback list is pointless work. So I'm thinking
> we can find a better place to do this.

I'm a wee bit confused by the split between rcu_prepare_for_idle() and
rcu_needs_cpu().

There's a fair amount overlap there.. that said, I'm thinking we should
be calling rcu_needs_cpu() as the very last test, not the very first,
such that we can bail out of tick_nohz_stop_sched_tick() without having
to incur the penalty of flushing callbacks.

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

#1685972

On Wed, Jul 12, 2017 at 07:57:32PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 12, 2017 at 07:17:56PM +0200, Peter Zijlstra wrote:
> > Could be I'm just not remembering how all that works.. But I was
> > wondering if we can do the expensive bits if we've decided to actually
> > go NOHZ and avoid doing it on every idle entry.
> > 
> > IIRC the RCU fast NOHZ bits try and flush the callback list (or paw it
> > off to another CPU?) such that we can go NOHZ sooner. Having a !empty
> > callback list avoid NOHZ from happening.
> > 
> > Now if we've already decided we can't in fact go NOHZ due to other
> > concerns, flushing the callback list is pointless work. So I'm thinking
> > we can find a better place to do this.
> 
> I'm a wee bit confused by the split between rcu_prepare_for_idle() and
> rcu_needs_cpu().
> 
> There's a fair amount overlap there.. that said, I'm thinking we should
> be calling rcu_needs_cpu() as the very last test, not the very first,
> such that we can bail out of tick_nohz_stop_sched_tick() without having
> to incur the penalty of flushing callbacks.

Maybe or maybe not, please see my earlier email for more details.

TL;DR:  No, callbacks are no longer flushed.  Yes, there is dependency.
Not hard to make rcu_prepare_for_idle() deal with rcu_needs_cpu() not
having been called, but it does need to happen.  Putting rcu_needs_cpu()
last is not necessarily a good thing.  If CPUs going idle normally
don't have callbacks, it won't help.  So we need hard evidence that
rcu_needs_cpu() is consuming significant time before hacking.

						Thanx, Paul

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

#1685962

On Wed, Jul 12, 2017 at 07:17:56PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 12, 2017 at 08:54:58AM -0700, Paul E. McKenney wrote:
> > On Wed, Jul 12, 2017 at 02:22:49PM +0200, Peter Zijlstra wrote:
> > > On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote:
> > > > On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote:
> > > > > Also, RCU_FAST_NO_HZ will make a fairly large difference here.. Paul
> > > > > what's the state of that thing, do we actually want that or not?
> > > > 
> > > > If you are battery powered and don't have tight real-time latency
> > > > constraints, you want it -- it has represent a 30-40% boost in battery
> > > > lifetime for some low-utilization battery-powered devices.  Otherwise,
> > > > probably not.
> > > 
> > > Would it make sense to hook that off of tick_nohz_idle_enter(); in
> > > specific the part where we actually stop the tick; instead of every
> > > idle?
> > 
> > The actions RCU takes on RCU_FAST_NO_HZ depend on the current state of
> > the CPU's callback lists, so it seems to me that the decision has to
> > be made on each idle entry.
> > 
> > Now it might be possible to make the checks more efficient, and doing
> > that is on my list.
> > 
> > Or am I missing your point?
> 
> Could be I'm just not remembering how all that works.. But I was
> wondering if we can do the expensive bits if we've decided to actually
> go NOHZ and avoid doing it on every idle entry.
> 
> IIRC the RCU fast NOHZ bits try and flush the callback list (or paw it
> off to another CPU?) such that we can go NOHZ sooner. Having a !empty
> callback list avoid NOHZ from happening.

The code did indeed attempt to flush the callback list back in the day,
but that proved to not actually save any power.  There were several
variations in the meantime, but what it does now is to check to see if
there are callbacks at rcu_needs_cpu() time:

1.	If there are none, RCU tells the caller that it doesn't need
	the CPU.

2.	If there are some, and some of them are non-lazy (as in doing
	something other than just freeing memory), RCU updates its idea
	of which grace period the callbacks are waiting for, otherwise
	leaves the callbacks alone, but returns saying that it needs
	the CPU around four jiffies (by default), but rounded to allow
	one wakeup to handle all CPUs in the power domain.  Use the
	rcu_idle_gp_delay boot/sysfs parameter to adjust the wait
	duration if required.  (I haven't heard of adjustment ever
	being required.)

	Note that a non-lazy callback might well be synchronize_rcu(),
	so we cannot wait too long, or we will be delaying things
	too much.

3.	If there are some callbacks, and all of them are lazy, RCU
	again updates its idea of which grace period the callbacks are
	waiting for, otherwise leaves the callbacks alone, but returns
	saying that it needs the CPU around six seconds (by default)
	in the future, but using round_jiffies(), again to share wakeups
	within a power domain.  Use the rcu_idle_lazy_gp_delay
	boot/sysfs parameter to adjust the wait, and again, as far as
	I know adjustment never has been necessary.

When the CPU is awakened, it will update its callback based on any
grace periods that have elapsed in the meantime.  There is a bit
of work later at rcu_idle_enter() time, but it is quite small.

> Now if we've already decided we can't in fact go NOHZ due to other
> concerns, flushing the callback list is pointless work. So I'm thinking
> we can find a better place to do this.

True, if the tick will still be happening, there is little point
in bothering RCU about it.  And if CPUs tend to go idle with RCU
callbacks, then it would be cheaper to check arch_needs_cpu() and
irq_work_needs_cpu() first.  If CPUs tend to be free of callbacks
when they go idle, this won't help, and might be counterproductive.

But if rcu_needs_cpu() or rcu_prepare_for_idle() is showing up on
profiles, I could adjust things.  This would include making
rcu_prepare_for_idle() no longer expect that rcu_needs_cpu() had
previously been called on the current path to idle.  (Not a big
deal, just that the obvious chnage to tick_nohz_stop_sched_tick()
won't necessarily do what you want.)

So please let me know if rcu_needs_cpu() or rcu_prepare_for_idle() are
prominent contributors to to-idle latency.

							Thanx, Paul

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

#1686383

On Wed, Jul 12, 2017 at 11:46:17AM -0700, Paul E. McKenney wrote:
> So please let me know if rcu_needs_cpu() or rcu_prepare_for_idle() are
> prominent contributors to to-idle latency.

Right, some actual data would be good.

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

#1685510

On 2017/7/12 0:34, Peter Zijlstra wrote:
> On Tue, Jul 11, 2017 at 06:09:27PM +0200, Frederic Weisbecker wrote:
> 
>>>> - tick_nohz_idle_enter costs 7058ns - 10726ns
>>>> - tick_nohz_idle_exit costs 8372ns - 20850ns
>>>
>>> Right, those are horrible expensive, but skipping them isn't 'hard', the
>>> only tricky bit is finding a condition that makes sense.
>>
>> Note you can statically disable it with nohz=0 boot parameter.
> 
> Yeah, but that's bad for power usage, nobody wants that.
> 
>>> See Mike's patch: https://patchwork.kernel.org/patch/2839221/
>>>
>>> Combined with the above, and possibly a better condition, that should
>>> get rid of most of this.
>>
>> Such a patch could work well if the decision from the scheduler to not stop the tick
>> happens on idle entry.
>>
>> Now if sched_needs_cpu() first allows to stop the tick then refuses it later
>> in the end of an idle IRQ, this won't have the desired effect. As long as ts->tick_stopped=1,
>> it stays so until we really restart the tick. So the whole costly nohz machinery stays on.
>>
>> I guess it doesn't matter though, as we are talking about making fast idle entry so the
>> decision not to stop the tick is likely to be done once on idle entry, when ts->tick_stopped=0.
>>
>> One exception though: if the tick is already stopped when we enter idle (full nohz case). And
>> BTW stopping the tick outside idle shouldn't be concerned here.
>>
>> So I'd rather put that on can_stop_idle_tick().
> 
> Mike's patch much predates the existence of that function I think ;-) But
> sure..
> 

Okay, the difference is that Mike's patch uses a very simple algorithm to make the decision.

/*
 * delta is wakeup_timestamp - idle_timestamp
 */
update_avg(&rq->avg_idle, delta);
...
static void update_avg(u64 *avg, u64 sample)
{
        s64 diff = sample - *avg;
        *avg += diff >> 3;
}

While my proposal is trying to leverage the prediction functionality of the existing idle menu
governor, which works very well for a long time.

I know the the code change is big and the running overhead is a bit higher than rq->avg_idle, but
should we make a comparison for some typical workloads?

Thanks,
-Aubrey

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

#1685631

On Wed, Jul 12, 2017 at 12:15:08PM +0800, Li, Aubrey wrote:
> Okay, the difference is that Mike's patch uses a very simple algorithm to make the decision.

No, the difference is that we don't end up with duplication of a metric
ton of code.

It uses the normal idle path, it just makes the NOHZ enter fail.

The condition Mike uses is why that patch never really went anywhere and
needs work.


For the condition I tend to prefer something auto-adjusting vs a tunable
threshold that everybody + dog needs to manually adjust.

So add something that measures the cost of tick_nohz_idle_{enter,exit}()
and base the threshold off of that. Then of course, there's the question
which of the idle estimates to use.

The cpuidle idle estimate includes IRQs, which is important for actual
idle states, but not all interrupts re-enable the tick.

The scheduler idle estimate only considers task activity, which tends to
re-enable the tick.

So the cpuidle estimate is pessimistic in that it'll vastly under
estimate the actual nohz period, while the scheduler estimate will over
estimate. I suspsect the scheduler one is closer to the actual nohz
duration, but this is something we'll have to play with.

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

Page 2 of 5 — ← Prev page 1 [2] 3 4 5  Next page →

Back to top | Article view | linux.kernel

csiph-web