From: Preeti U Murthy <hidden> Date: 2014-09-12 11:01:51
Today the procfs interface /proc/sys/kernel/powersave-nap is used to control
entry into deep idle states beyond snooze. Check for the value of this
parameter before entering fastsleep. We already do this check for nap in
power7_idle().
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2014-09-15 06:59:18
On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote:
quoted hunk
Today the procfs interface /proc/sys/kernel/powersave-nap is used to control
entry into deep idle states beyond snooze. Check for the value of this
parameter before entering fastsleep. We already do this check for nap in
power7_idle().
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev,unsignedlongold_lpcr=mfspr(SPRN_LPCR);unsignedlongnew_lpcr;+/*+*Verifyifsnoozeistheonlyvalidcpuidlestate+*/+if(!(powersave_nap>0))+returnindex;+if(unlikely(system_state<SYSTEM_RUNNING))returnindex;
Doesn't the above mean we are just going to keep trying to go into fastsleep
again and again? Or does the idle code work out that it didn't work based on
the fact that we didn't sleep for the right period?
We were talking about getting rid of powersave_nap altogether, but I think we
decided we couldn't, I forget.
cheers
From: Preeti U Murthy <hidden> Date: 2014-09-15 08:52:10
On 09/15/2014 12:29 PM, Michael Ellerman wrote:
On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote:
quoted
Today the procfs interface /proc/sys/kernel/powersave-nap is used to control
entry into deep idle states beyond snooze. Check for the value of this
parameter before entering fastsleep. We already do this check for nap in
power7_idle().
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev,unsignedlongold_lpcr=mfspr(SPRN_LPCR);unsignedlongnew_lpcr;+/*+*Verifyifsnoozeistheonlyvalidcpuidlestate+*/+if(!(powersave_nap>0))+returnindex;+if(unlikely(system_state<SYSTEM_RUNNING))returnindex;
Doesn't the above mean we are just going to keep trying to go into fastsleep
again and again? Or does the idle code work out that it didn't work based on
the fact that we didn't sleep for the right period?
Thats right. The idle code figures that its judgment to enter fastsleep
was not correct and applies a correction factor to its future decisions
. This correction factor is intended to influence the cpuidle governor's
decision on choosing an idle state for the cpu based on the history of
wakeups. Hence a shallower idle state will be chosen here on in the
above circumstance.
We were talking about getting rid of powersave_nap altogether, but I think we
decided we couldn't, I forget.
Isn't this a helpful knob to disable cpuidle at runtime? Currently we
check the value of powersave_nap before entering both nap and fastsleep.
Regards
Preeti U Murthy
From: Preeti U Murthy <hidden> Date: 2015-01-30 05:41:28
On 09/15/2014 02:22 PM, Preeti U Murthy wrote:
On 09/15/2014 12:29 PM, Michael Ellerman wrote:
quoted
On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote:
quoted
Today the procfs interface /proc/sys/kernel/powersave-nap is used to control
entry into deep idle states beyond snooze. Check for the value of this
parameter before entering fastsleep. We already do this check for nap in
power7_idle().
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev,unsignedlongold_lpcr=mfspr(SPRN_LPCR);unsignedlongnew_lpcr;+/*+*Verifyifsnoozeistheonlyvalidcpuidlestate+*/+if(!(powersave_nap>0))+returnindex;+if(unlikely(system_state<SYSTEM_RUNNING))returnindex;
Doesn't the above mean we are just going to keep trying to go into fastsleep
again and again? Or does the idle code work out that it didn't work based on
the fact that we didn't sleep for the right period?
Thats right. The idle code figures that its judgment to enter fastsleep
was not correct and applies a correction factor to its future decisions
. This correction factor is intended to influence the cpuidle governor's
decision on choosing an idle state for the cpu based on the history of
wakeups. Hence a shallower idle state will be chosen here on in the
above circumstance.
On second thoughts, I feel that this is not the best way to disable deep
idle states. Going down this path means the governor should identify
that fastsleep is not being entered into and take corrective action from
then on. If the governor is buggy, we pointlessly enter and exit
fastsleep_loop() during long idle periods.
So a better way to disable fastsleep during runtime is through the
disable flag in sysfs. To disable deep idle states at bootup one could
use powersave=off kernel cmd line parameter.
The functionality provided by powersave_nap and smt_snooze_delay (to
quote another example) carried value during the days that powernv did
not have a cpuidle driver. We retain them so as to not break userspace,
although they do not carry value today. Any future users of these
parameters should be avoided IMO. Hence I retract this patch.
Regards
Preeti U Murthy
quoted
We were talking about getting rid of powersave_nap altogether, but I think we
decided we couldn't, I forget.
Isn't this a helpful knob to disable cpuidle at runtime? Currently we
check the value of powersave_nap before entering both nap and fastsleep.
Regards
Preeti U Murthy