| Message ID | 20140912110131.7311.86417.stgit@preeti.in.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote: > 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 <preeti@linux.vnet.ibm.com> > --- > > drivers/cpuidle/cpuidle-powernv.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c > index a64be57..b8ba52e 100644 > --- a/drivers/cpuidle/cpuidle-powernv.c > +++ b/drivers/cpuidle/cpuidle-powernv.c > @@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev, > unsigned long old_lpcr = mfspr(SPRN_LPCR); > unsigned long new_lpcr; > > + /* > + * Verify if snooze is the only valid cpuidle state > + */ > + if (!(powersave_nap > 0)) > + return index; > + > if (unlikely(system_state < SYSTEM_RUNNING)) > return index; 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2014 12:29 PM, Michael Ellerman wrote: > On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote: >> 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 <preeti@linux.vnet.ibm.com> >> --- >> >> drivers/cpuidle/cpuidle-powernv.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c >> index a64be57..b8ba52e 100644 >> --- a/drivers/cpuidle/cpuidle-powernv.c >> +++ b/drivers/cpuidle/cpuidle-powernv.c >> @@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev, >> unsigned long old_lpcr = mfspr(SPRN_LPCR); >> unsigned long new_lpcr; >> >> + /* >> + * Verify if snooze is the only valid cpuidle state >> + */ >> + if (!(powersave_nap > 0)) >> + return index; >> + >> if (unlikely(system_state < SYSTEM_RUNNING)) >> return index; > > 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 > > cheers > > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2014-09-15 at 14:22 +0530, Preeti U Murthy wrote: > > 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. It's useful when diagnosing some problems that can be caused by idle states. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2014 02:22 PM, Preeti U Murthy wrote: > On 09/15/2014 12:29 PM, Michael Ellerman wrote: >> On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote: >>> 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 <preeti@linux.vnet.ibm.com> >>> --- >>> >>> drivers/cpuidle/cpuidle-powernv.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c >>> index a64be57..b8ba52e 100644 >>> --- a/drivers/cpuidle/cpuidle-powernv.c >>> +++ b/drivers/cpuidle/cpuidle-powernv.c >>> @@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev, >>> unsigned long old_lpcr = mfspr(SPRN_LPCR); >>> unsigned long new_lpcr; >>> >>> + /* >>> + * Verify if snooze is the only valid cpuidle state >>> + */ >>> + if (!(powersave_nap > 0)) >>> + return index; >>> + >>> if (unlikely(system_state < SYSTEM_RUNNING)) >>> return index; >> >> 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 > >> >> 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 >> >> cheers >> >> >> _______________________________________________ >> Linuxppc-dev mailing list >> Linuxppc-dev@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/linuxppc-dev >> > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index a64be57..b8ba52e 100644 --- a/drivers/cpuidle/cpuidle-powernv.c +++ b/drivers/cpuidle/cpuidle-powernv.c @@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev, unsigned long old_lpcr = mfspr(SPRN_LPCR); unsigned long new_lpcr; + /* + * Verify if snooze is the only valid cpuidle state + */ + if (!(powersave_nap > 0)) + return index; + if (unlikely(system_state < SYSTEM_RUNNING)) return index;
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 <preeti@linux.vnet.ibm.com> --- drivers/cpuidle/cpuidle-powernv.c | 6 ++++++ 1 file changed, 6 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html