| Message ID | 51D07E7F.2030709@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Monday, July 01, 2013 12:22:47 AM Srivatsa S. Bhat wrote: > On 06/30/2013 10:35 PM, Toralf Förster wrote: > > On 06/30/2013 06:33 PM, Srivatsa S. Bhat wrote: > >> Toralf, can you please > >> try out the below patch and see if it improves anything? (Don't revert anything, > >> just apply the below diff on a problematic kernel and see if it solves your > >> issue). > > > > applied on top of a66b2e5 - issue went away (either fixed or hidden now) > > > > Cool! So here is the proper patch, with changelog added. > > Regards, > Srivatsa S. Bhat > > > ----------------------------------------------------------------------------- > > From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > Subject: [PATCH] cpufreq: Fix cpufreq regression after suspend/resume > > Toralf Förster reported that the cpufreq ondemand governor behaves erratically > (doesn't scale well) after a suspend/resume cycle. The problem was that the > cpufreq subsystem's idea of the cpu frequencies differed from the actual > frequencies set in the hardware after a suspend/resume cycle. Toralf bisected > the problem to commit a66b2e5 (cpufreq: Preserve sysfs files across > suspend/resume). > > Among other (harmless) things, that commit skipped the call to > cpufreq_update_policy() in the resume path. But cpufreq_update_policy() plays > an important role during resume, because it is responsible for checking if > the BIOS changed the cpu frequencies behind our back and resynchronize the > cpufreq subsystem's knowledge of the cpu frequencies, and update them > accordingly. > > So, restore the call to cpufreq_update_policy() in the resume path to fix > the cpufreq regression. > > Reported-by: Toralf Förster <toralf.foerster@gmx.de> > Tested-by: Toralf Förster <toralf.foerster@gmx.de> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Thanks Srivatsa, I'll queue it up as 3.11 (and 3.10-stable) material. Thanks, Rafael > --- > > drivers/cpufreq/cpufreq_stats.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c > index fb65dec..591b6fb 100644 > --- a/drivers/cpufreq/cpufreq_stats.c > +++ b/drivers/cpufreq/cpufreq_stats.c > @@ -349,6 +349,7 @@ static int __cpuinit cpufreq_stat_cpu_callback(struct notifier_block *nfb, > > switch (action) { > case CPU_ONLINE: > + case CPU_ONLINE_FROZEN: > cpufreq_update_policy(cpu); > break; > case CPU_DOWN_PREPARE: > >
I tested the patch several times on top of a66b2e5 - the origin issue is fixed but - erratically another issue now appears : all 4 cores are runs after wakeup at 2.6 GHz. The temporary hot fix is to switch between governor performance and ondemand for all 4 cores. On 06/30/2013 08:52 PM, Srivatsa S. Bhat wrote: > On 06/30/2013 10:35 PM, Toralf Förster wrote: >> On 06/30/2013 06:33 PM, Srivatsa S. Bhat wrote: >>> Toralf, can you please >>> try out the below patch and see if it improves anything? (Don't revert anything, >>> just apply the below diff on a problematic kernel and see if it solves your >>> issue). >> >> applied on top of a66b2e5 - issue went away (either fixed or hidden now) >> > > Cool! So here is the proper patch, with changelog added. > > Regards, > Srivatsa S. Bhat > > > ----------------------------------------------------------------------------- > > From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > Subject: [PATCH] cpufreq: Fix cpufreq regression after suspend/resume > > Toralf Förster reported that the cpufreq ondemand governor behaves erratically > (doesn't scale well) after a suspend/resume cycle. The problem was that the > cpufreq subsystem's idea of the cpu frequencies differed from the actual > frequencies set in the hardware after a suspend/resume cycle. Toralf bisected > the problem to commit a66b2e5 (cpufreq: Preserve sysfs files across > suspend/resume). > > Among other (harmless) things, that commit skipped the call to > cpufreq_update_policy() in the resume path. But cpufreq_update_policy() plays > an important role during resume, because it is responsible for checking if > the BIOS changed the cpu frequencies behind our back and resynchronize the > cpufreq subsystem's knowledge of the cpu frequencies, and update them > accordingly. > > So, restore the call to cpufreq_update_policy() in the resume path to fix > the cpufreq regression. > > Reported-by: Toralf Förster <toralf.foerster@gmx.de> > Tested-by: Toralf Förster <toralf.foerster@gmx.de> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > --- > > drivers/cpufreq/cpufreq_stats.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c > index fb65dec..591b6fb 100644 > --- a/drivers/cpufreq/cpufreq_stats.c > +++ b/drivers/cpufreq/cpufreq_stats.c > @@ -349,6 +349,7 @@ static int __cpuinit cpufreq_stat_cpu_callback(struct notifier_block *nfb, > > switch (action) { > case CPU_ONLINE: > + case CPU_ONLINE_FROZEN: > cpufreq_update_policy(cpu); > break; > case CPU_DOWN_PREPARE: > > >
On Wed, 2013-07-10 at 22:50 +0200, Toralf Förster wrote: > I tested the patch several times on top of a66b2e5 - the origin issue is > fixed Srivatsa's patch became commit f51e1eb63d ("cpufreq: Fix cpufreq regression after suspend/resume"). > but - erratically another issue now appears : all 4 cores are runs > after wakeup at 2.6 GHz. Well, a laptop I use seems to run into something related: the second of its two cores can get stuck at a fixed frequency after resume. Often it's its maximum frequency. That makes the CPU run hot, without actually doing much work. But it can also get stuck at its lowest frequency. Please note that commit f51e1eb63d, which is part of v3.10.1-rc1, doesn't seem to cause this behavior. This issue can already be seen when running v3.10.0. So perhaps that commit just made this issue more noticeable to Toralf. But it's not clear to me what Toralf's origin(al?) issue actually was. Anyhow, I suspect that the stuck frequency is a regression introduced in v3.10.0. But I have been unable to pinpoint it to a commit added in the v3.10 cycle through bisecting. I must have marked a commit good that was actually bad, just because a few suspend and resume cycles didn't trigger this issue. To be continued... > The temporary hot fix is to switch between governor performance and > ondemand for all 4 cores. That workaround works here too. I switch the core that is stuck at a particular frequency to some other governor and then back to ondemand. Thanks, Paul Bolle -- 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 Sat, 2013-07-13 at 12:16 +0200, Paul Bolle wrote:
> I suspect that the stuck frequency is a regression introduced in v3.10.0.
The culprit apparently is commit a66b2e503f ("cpufreq: Preserve sysfs
files across suspend/resume"). Srivatsa submitted a patch to revert that
commit (see https://lkml.org/lkml/2013/7/11/661 ). That revert seems to
fix this issue too.
Paul Bolle
--
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 07/13/2013 06:22 PM, Paul Bolle wrote: > On Sat, 2013-07-13 at 12:16 +0200, Paul Bolle wrote: >> I suspect that the stuck frequency is a regression introduced in v3.10.0. > > The culprit apparently is commit a66b2e503f ("cpufreq: Preserve sysfs > files across suspend/resume"). Srivatsa submitted a patch to revert that > commit (see https://lkml.org/lkml/2013/7/11/661 ). That revert seems to > fix this issue too. > > Thanks a lot for your tests and for confirming that the complete revert fixes your cpufreq issue. Regards, Srivatsa S. Bhat -- 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/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c index fb65dec..591b6fb 100644 --- a/drivers/cpufreq/cpufreq_stats.c +++ b/drivers/cpufreq/cpufreq_stats.c @@ -349,6 +349,7 @@ static int __cpuinit cpufreq_stat_cpu_callback(struct notifier_block *nfb, switch (action) { case CPU_ONLINE: + case CPU_ONLINE_FROZEN: cpufreq_update_policy(cpu); break; case CPU_DOWN_PREPARE: