[PATCH] cpuidle: menu: Fall back to polling if next timer event is near

[PATCH] cpuidle: menu: Fall back to polling if next timer event is near

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit a9ceb78bc75c (cpuidle,menu: use interactivity_req to disable
polling) changed the behavior of the fallback state selection part
of menu_select() so it looks at interactivity_req instead of
data->next_timer_us when it makes its decision.  That effectively
caused polling to be used more often as fallback idle which led to
significant increases of energy consumption in some cases.

Commit e132b9b3bc7f (cpuidle: menu: use high confidence factors
only when considering polling) changed that logic again to be more
predictable, but that didn't help with the increased energy
consumption problem.

For this reason, go back to making decisions on which state to fall
back to based on data->next_timer_us which is the time we know for
sure something will happen rather than a prediction (which may be
inaccurate and turns out to be so often enough to be problematic).
However, take the target residency of the first proper idle state
(C1) into account, so that state is not used as the fallback one
if its target residency is greater than data->next_timer_us.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Doug, can you please see if this patch helps to address the problem
you're seeing?

Commit e132b9b3bc7f is in linux-next only ATM, but it is a rebased
version of https://patchwork.kernel.org/patch/8602101/.

---
 drivers/cpuidle/governors/menu.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -315,17 +315,21 @@ static int menu_select(struct cpuidle_dr
 	expected_interval = min(expected_interval, data->next_timer_us);
 
 	if (CPUIDLE_DRIVER_STATE_START > 0) {
-		data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1;
+		struct cpuidle_state *s = &drv->states[CPUIDLE_DRIVER_STATE_START];
+		unsigned int polling_threshold;
+
 		/*
 		 * We want to default to C1 (hlt), not to busy polling
 		 * unless the timer is happening really really soon, or
 		 * C1's exit latency exceeds the user configured limit.
 		 */
-		if (expected_interval > drv->states[CPUIDLE_DRIVER_STATE_START].target_residency &&
-		    latency_req > drv->states[CPUIDLE_DRIVER_STATE_START].exit_latency &&
-		    !drv->states[CPUIDLE_DRIVER_STATE_START].disabled &&
+		polling_threshold = max_t(unsigned int, 20, s->target_residency);
+		if (data->next_timer_us > polling_threshold &&
+		    latency_req > s->exit_latency && !s->disabled &&
 		    !dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable)
 			data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
+		else
+			data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1;
 	} else {
 		data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
 	}

RE: [PATCH] cpuidle: menu: Fall back to polling if next timer event is near

[Doug Smythies]

On 2016.03.19 17:34 Rafael J. Wysocki wrote:

> Commit a9ceb78bc75c (cpuidle,menu: use interactivity_req to disable
> polling) changed the behavior of the fallback state selection part
> of menu_select() so it looks at interactivity_req instead of
> data->next_timer_us when it makes its decision.  That effectively
> caused polling to be used more often as fallback idle which led to
> significant increases of energy consumption in some cases.
>
> Commit e132b9b3bc7f (cpuidle: menu: use high confidence factors
> only when considering polling) changed that logic again to be more
> predictable, but that didn't help with the increased energy
> consumption problem.
>
> For this reason, go back to making decisions on which state to fall
> back to based on data->next_timer_us which is the time we know for
> sure something will happen rather than a prediction (which may be
> inaccurate and turns out to be so often enough to be problematic).
> However, take the target residency of the first proper idle state
> (C1) into account, so that state is not used as the fallback one
> if its target residency is greater than data->next_timer_us.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> Doug, can you please see if this patch helps to address the problem
> you're seeing?
> 

Yes, it appears to work great.

>
> Commit e132b9b3bc7f is in linux-next only ATM, but it is a rebased
> version of https://patchwork.kernel.org/patch/8602101/.

So, I applied it to my existing kernel 4.5-rc7 work, i.e. the not
rebased version.

My reference = rvr7:
Kernel 4.5-rc7 +
Rafael's 3 patch set version 10 "cpufreq: Replace timers with utilization update callbacks" +
Rik's patch (rvr5) "cpuidle: menu: use high confidence factors only when considering polling" +
Rafael's patch from herein.

My reference: k45rc7-rjw10-rvr7

Aggregate Idle States in minutes for 2000 second test (some old data re-stated for reference):

State	k45rc7-rjw10	k45rc7-rjw10-reverted	k45rc7-rjw10-rvr7
0.00	18.07			0.92				0.41
1.00	12.35			19.51				21.17
2.00	3.96			4.28				4.40
3.00	1.55			1.53				1.66
4.00	138.96		141.99			150.77
			
total	174.90		168.24			178.41

Energy:
>>>> Kernel 4.5-rc7-rjw10: 61983 Joules
>>>> Kernel 4.5-rc7-rjw10-reverted: 48409 Joules (test 2 was 55040 Joules)
k45rc7-rjw10-rvr7: 54748 Joules

The trace data has a record low number of long durations at 71.

... Doug

Re: [PATCH] cpuidle: menu: Fall back to polling if next timer event is near

[Rafael J. Wysocki]

On Saturday, March 19, 2016 11:54:39 PM Doug Smythies wrote:
> On 2016.03.19 17:34 Rafael J. Wysocki wrote:
> 
> > Commit a9ceb78bc75c (cpuidle,menu: use interactivity_req to disable
> > polling) changed the behavior of the fallback state selection part
> > of menu_select() so it looks at interactivity_req instead of
> > data->next_timer_us when it makes its decision.  That effectively
> > caused polling to be used more often as fallback idle which led to
> > significant increases of energy consumption in some cases.
> >
> > Commit e132b9b3bc7f (cpuidle: menu: use high confidence factors
> > only when considering polling) changed that logic again to be more
> > predictable, but that didn't help with the increased energy
> > consumption problem.
> >
> > For this reason, go back to making decisions on which state to fall
> > back to based on data->next_timer_us which is the time we know for
> > sure something will happen rather than a prediction (which may be
> > inaccurate and turns out to be so often enough to be problematic).
> > However, take the target residency of the first proper idle state
> > (C1) into account, so that state is not used as the fallback one
> > if its target residency is greater than data->next_timer_us.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > 
> > Doug, can you please see if this patch helps to address the problem
> > you're seeing?
> > 
> 
> Yes, it appears to work great.
> 
> >
> > Commit e132b9b3bc7f is in linux-next only ATM, but it is a rebased
> > version of https://patchwork.kernel.org/patch/8602101/.
> 
> So, I applied it to my existing kernel 4.5-rc7 work, i.e. the not
> rebased version.
> 
> My reference = rvr7:
> Kernel 4.5-rc7 +
> Rafael's 3 patch set version 10 "cpufreq: Replace timers with utilization update callbacks" +
> Rik's patch (rvr5) "cpuidle: menu: use high confidence factors only when considering polling" +
> Rafael's patch from herein.
> 
> My reference: k45rc7-rjw10-rvr7
> 
> Aggregate Idle States in minutes for 2000 second test (some old data re-stated for reference):
> 
> State	k45rc7-rjw10	k45rc7-rjw10-reverted	k45rc7-rjw10-rvr7
> 0.00	18.07			0.92				0.41
> 1.00	12.35			19.51				21.17
> 2.00	3.96			4.28				4.40
> 3.00	1.55			1.53				1.66
> 4.00	138.96		141.99			150.77
> 			
> total	174.90		168.24			178.41
> 
> Energy:
> >>>> Kernel 4.5-rc7-rjw10: 61983 Joules
> >>>> Kernel 4.5-rc7-rjw10-reverted: 48409 Joules (test 2 was 55040 Joules)
> k45rc7-rjw10-rvr7: 54748 Joules
> 
> The trace data has a record low number of long durations at 71.

OK, thanks!

I'm going to apply it for 4.6, then.

Thanks,
Rafael