powernv:cpuidle: Enable winkle idle state
Subsystems (2)
cover powernv:cpuidle: Enable winkle idle state
From: "Gautham R. Shenoy" <[email protected]>
Hi,
The patches in these series enable support for Winkle idle state in CPU-Idle.
The first patch is a platform-independent CPU-Idle patch that allows CPU-Idle states to be disabled at start (Currently they are all enabled by default).
The second patch adds the winkle enablement for powernv-cpuidle. By default, the winkle idle-state is disabled. It can be enabled by writing zero to the per-cpu cpuidle sysfs control file named "disable".
This series has been lightly tested on a 2-socket POWER8 system and the machine was pretty stable while running kernbench and ebizzy. I didn't see any regressions with those.
I haven't yet evaluated the impact that these patches might have on latency sensitive workloads. I hope to do that in a day or two.
On the power-savings front, I could observe 6-8% additional power-savings when winkle state was enabled on an idle system with SMT=on. With SMT=off, additional idle power-savings observed with winkle enabled were greater than 15%. The numbers indicate that it might be worth the while to pursue this!
Gautham R. Shenoy (2):
cpuidle: Allow idle-states to be disabled at start powernv:cpuidle: Enable winkle idle state in CPU-Idle.
drivers/cpuidle/cpuidle-powernv.c | 44 ++++++++++++++++++++++++++++++++------- drivers/cpuidle/cpuidle.c | 7 +++++++ include/linux/cpuidle.h | 7 ++++++- 3 files changed, 49 insertions(+), 9 deletions(-)
Signature · 1 line
1.9.4
PATCH 1/2 cpuidle: Allow idle-states to be disabled at start
From: "Gautham R. Shenoy" <[email protected]>
Currently all the idle states registered by a cpu-idle driver are enabled by default. This patch adds a mechanism which allows the driver to hint if an idle-state should start in a disabled state. The cpu-idle core will use this hint to appropriately initialize the usage->disable knob of the CPU device idle state.
The state can be enabled at run time by echo'ing a zero to the sysfs "disable" control file.
--- drivers/cpuidle/cpuidle.c | 7 +++++++ include/linux/cpuidle.h | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-)
drivers/cpuidle/cpuidle.c +7 -0
@@ -439,7 +439,14 @@ static void __cpuidle_unregister_device(struct cpuidle_device *dev)static void __cpuidle_device_init (struct cpuidle_device *dev) { +struct cpuidle_driver *drv =cpuidle_get_cpu_driver (dev); +int i; +memset (dev->states_usage,0 ,sizeof (dev->states_usage)); +for (i =0 ; i < drv->state_count; i++) { +if (drv->states[i].disable_use_at_start) + dev->states_usage[i].disable =1 ; + } dev->last_residency =0 ; }
include/linux/cpuidle.h +6 -1
@@ -44,7 +44,12 @@ struct cpuidle_state {int power_usage;/* in mW */ unsigned int target_residency;/* in US */ bool disabled;/* disabled on all CPUs */ - +/* +* disable_use_at_start: If true, then this idle state will be +* disabled by default. It can be enabled at runtime using the +* per-cpu cpuidle sysfs control file named "disable". +*/ +bool disable_use_at_start;int (*enter) (struct cpuidle_device *dev,struct cpuidle_driver *drv,int index);
Signature · 1 line
1.9.4
Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start
Quoted 7 lines · Gautham R. Shenoy
From: "Gautham R. Shenoy" <[email protected]>
Currently all the idle states registered by a cpu-idle driver are enabled by default. This patch adds a mechanism which allows the driver to hint if an idle-state should start in a disabled state. The cpu-idle core will use this hint to appropriately initialize the
usage->disableknob of the CPU device idle state.
Why do you need to do that ?
Quoted 2 lines
The state can be enabled at run time by echo'ing a zero to the sysfs "disable" control file.
... for each cpu.
Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start
Quoted 11 lines · Daniel Lezcano
Quoted 7 lines · Gautham R. Shenoy
From: "Gautham R. Shenoy" <[email protected]>
Currently all the idle states registered by a cpu-idle driver are enabled by default. This patch adds a mechanism which allows the driver to hint if an idle-state should start in a disabled state. The cpu-idle core will use this hint to appropriately initialize the
usage->disableknob of the CPU device idle state.Why do you need to do that ?
I think patch 2/2 explains the reason as it uses this infrastructure
Balbir Singh
Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start
Quoted 12 lines · Balbir Singh
Quoted 9 lines · Daniel Lezcano
Quoted 7 lines · Gautham R. Shenoy
From: "Gautham R. Shenoy" <[email protected]>
Currently all the idle states registered by a cpu-idle driver are enabled by default. This patch adds a mechanism which allows the driver to hint if an idle-state should start in a disabled state. The cpu-idle core will use this hint to appropriately initialize the
usage->disableknob of the CPU device idle state.
Quoted 4 lines
Quoted 2 lines
Why do you need to do that ?
I think patch 2/2 explains the reason as it uses this infrastructure
Ok, let me elaborate the question, I was not clear.
Why the userspace can't setup the system environment at boot time by disabling the state instead of adding extra code to disable it at boot time in the kernel and then re-enable it from userspace ?
-- Daniel
Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start
Quoted 16 lines · Daniel Lezcano
Quoted 15 lines · Balbir Singh
Quoted 11 lines · Daniel Lezcano
Quoted 7 lines · Gautham R. Shenoy
From: "Gautham R. Shenoy" <[email protected]>
Currently all the idle states registered by a cpu-idle driver are enabled by default. This patch adds a mechanism which allows the driver to hint if an idle-state should start in a disabled state. The cpu-idle core will use this hint to appropriately initialize the
usage->disableknob of the CPU device idle state.Why do you need to do that ?
Quoted 7 lines
Quoted 1 line
I think patch 2/2 explains the reason as it uses this infrastructure
Ok, let me elaborate the question, I was not clear.
Why the userspace can't setup the system environment at boot time by disabling the state instead of adding extra code to disable it at boot time in the kernel and then re-enable it from userspace ?
Gautham's patches don't want to have those states enabled by default. They are unlikely to be what production systems need, but likely what a knowledgeable person can look into selectively enable for experimentation.
@Gautham?
Balbir Singh.
Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start
Quoted 23 lines · Balbir Singh
Quoted 20 lines · Daniel Lezcano
Quoted 12 lines · Balbir Singh
Quoted 9 lines · Daniel Lezcano
Quoted 7 lines · Gautham R. Shenoy
From: "Gautham R. Shenoy" <[email protected]>
Currently all the idle states registered by a cpu-idle driver are enabled by default. This patch adds a mechanism which allows the driver to hint if an idle-state should start in a disabled state. The cpu-idle core will use this hint to appropriately initialize the
usage->disableknob of the CPU device idle state.Quoted 4 lines
Quoted 2 lines
Why do you need to do that ?
I think patch 2/2 explains the reason as it uses this infrastructure
Ok, let me elaborate the question, I was not clear.
Quoted 8 lines
Quoted 3 lines
Why the userspace can't setup the system environment at boot time by disabling the state instead of adding extra code to disable it at boot time in the kernel and then re-enable it from userspace ?
Gautham's patches don't want to have those states enabled by default. They are unlikely to be what production systems need, but likely what a knowledgeable person can look into selectively enable for experimentation.
Why not invert the logic ?
A knowledgeable person can look into selectively disable for production.
In addition, a kernel command line option to specify which state to disable would be appropriate and beneficial for all existing drivers.
Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH 2/2 powernv:cpuidle: Enable winkle idle state in CPU-Idle.
In reply to Gautham R. ShenoyFrom: "Gautham R. Shenoy" <[email protected]>
cpu-idle on powernv currently has support for only snooze, nap and fastsleep states. Winkle idle state was excluded due to its large exit-latency.
This patch adds winkle as a cpu-idle state for experimental purposes. This state is disabled at start by default. However, should an adventurous user want to enable it on a particular CPU(s), they can do so by echo'ing a zero into the per-cpu sysfs cpuidle control file named "disable" corresponding to this state.
--- drivers/cpuidle/cpuidle-powernv.c | 44 ++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-)
drivers/cpuidle/cpuidle-powernv.c +36 -8
@@ -20,7 +20,6 @@#include <asm/opal.h> #include <asm/runlatch.h> -#define POWERNV_THRESHOLD_LATENCY_NS 200000struct cpuidle_driver powernv_idle_driver = { .name ="powernv_idle" ,
@@ -95,6 +94,30 @@ static int fastsleep_loop(struct cpuidle_device *dev,return index; } + +static int winkle_loop (struct cpuidle_device *dev, +struct cpuidle_driver *drv, +int index) +{ +unsigned long old_lpcr =mfspr (SPRN_LPCR ); +unsigned long new_lpcr; + +if (unlikely (system_state <SYSTEM_RUNNING )) +return index; + + new_lpcr = old_lpcr; +/* Do not exit powersave upon decrementer as we've setup the timer +* offload. +*/ + new_lpcr &= ~LPCR_PECE1 ; + +mtspr (SPRN_LPCR , new_lpcr); +power7_winkle (); + +mtspr (SPRN_LPCR , old_lpcr); + +return index; +}#endif static int stop_loop(struct cpuidle_device *dev,
@@ -246,13 +269,6 @@ static int powernv_add_idle_states(void) "ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states);for (i =0 ; i < dt_idle_states; i++) { -/* -* If an idle state has exit latency beyond -* POWERNV_THRESHOLD_LATENCY_NS then don't use it -* in cpu-idle. -*/ -if (latency_ns[i] >POWERNV_THRESHOLD_LATENCY_NS ) -continue ;/* * Cpuidle accepts exit_latency and target_residency in us.
@@ -301,6 +317,18 @@ static int powernv_add_idle_states(void) powernv_states[nr_idle_states].enter = stop_loop; stop_psscr_table[nr_idle_states] = psscr_val[i]; } + +if (flags[i] &OPAL_PM_WINKLE_ENABLED ) { +int state_idx = nr_idle_states; + +strcpy (powernv_states[state_idx].name,"Winkle" ); +strcpy (powernv_states[state_idx].desc,"Winkle" ); + powernv_states[state_idx].flags = +CPUIDLE_FLAG_TIMER_STOP ; + powernv_states[state_idx].target_residency =500000 ; + powernv_states[state_idx].enter = winkle_loop; + powernv_states[state_idx].disable_use_at_start = true; + }#endif powernv_states[nr_idle_states].exit_latency = ((unsigned int )latency_ns[i]) /1000 ;
Signature · 1 line
1.9.4