* [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed
@ 2017-08-25 16:43 Chen Yu
2017-08-25 16:43 ` [PATCH 1/2][RFC] ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement Chen Yu
2017-08-25 16:43 ` [PATCH 2/2][RFC] ACPI / PM: Disable the MSR T-state during CPU online Chen Yu
0 siblings, 2 replies; 8+ messages in thread
From: Chen Yu @ 2017-08-25 16:43 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-pm, Rafael J. Wysocki, Len Brown, Chen Yu
There is a growing number of reports that the MSR throttling has
been enabled after resumed back from suspend to ram, which impacts
the system performance. This patchset tries to address this issue
by turning off the T-state after resumed back.
Chen Yu (2):
ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
ACPI / PM: Disable the MSR T-state during CPU online
drivers/acpi/sleep.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2][RFC] ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
2017-08-25 16:43 [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed Chen Yu
@ 2017-08-25 16:43 ` Chen Yu
2017-08-25 16:43 ` [PATCH 2/2][RFC] ACPI / PM: Disable the MSR T-state during CPU online Chen Yu
1 sibling, 0 replies; 8+ messages in thread
From: Chen Yu @ 2017-08-25 16:43 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-pm, Rafael J. Wysocki, Len Brown, Chen Yu, linux-kernel
There might be other actions to be taken during the
acpi syscore suspend/resume phase, thus reuse the
acpi_sleep_syscore_ops in case other operations are
added into the acpi_sleep_syscore_ops.
No functional change.
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
drivers/acpi/sleep.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index fa8243c..cad1a0f 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -865,9 +865,19 @@ static void acpi_restore_bm_rld(void)
acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld);
}
+static int acpi_syscore_suspend(void)
+{
+ return acpi_save_bm_rld();
+}
+
+static void acpi_syscore_restore(void)
+{
+ acpi_restore_bm_rld();
+}
+
static struct syscore_ops acpi_sleep_syscore_ops = {
- .suspend = acpi_save_bm_rld,
- .resume = acpi_restore_bm_rld,
+ .suspend = acpi_syscore_suspend,
+ .resume = acpi_syscore_restore,
};
void acpi_sleep_syscore_init(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2][RFC] ACPI / PM: Disable the MSR T-state during CPU online
2017-08-25 16:43 [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed Chen Yu
2017-08-25 16:43 ` [PATCH 1/2][RFC] ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement Chen Yu
@ 2017-08-25 16:43 ` Chen Yu
1 sibling, 0 replies; 8+ messages in thread
From: Chen Yu @ 2017-08-25 16:43 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-pm, Rafael J. Wysocki, Len Brown, Chen Yu, linux-kernel
In 2015 a bug was once reported that on a Broadwell
platform, after resumed from S3, the CPU was running at
an anomalously low speed, due to the BIOS has enabled the
MSR throttling across S3. This was a BIOS issue and the
solution to that was to introduce a quirk to save/restore
T-state MSR register around suspend/resume, in
Commit 7a9c2dd08ead ("x86/pm: Introduce quirk framework to
save/restore extra MSR registers around suspend/resume").
However there are still three problems left:
1. More and more reports show that other platforms also
encountered the same issue, so the quirk list might
be endless.
2. Each CPUs should take the save/restore operation into
consideration, rather than the boot CPU alone.
3. Normally ACPI T-state re-evaluation should be taken care
of during resume in the ACPI throttling driver, however
there is no _TSS on that bogus platform, thus the
re-evaluation code does not run on that machine.
Solution:
This patch is based on the fact that, we generally should not
expect the system to come back from resume(or event CPU been
brought online) with throttling enabled, but leverage the OS
components to deal with it, so we simply clear the MSR T-state
after that CPU has been brought online. In addition to that,
print the warning if the T-state is found to be enabled.
The side effect of this patch is that, we might lose the T-state
evaluation value in the ACPI throttling driver during CPU online
stage, because we can not guarantee that the clear action we
introduced is invoked strictly before the T-state evaluation in
the ACPI throttling driver. But anyway it is expected that there
should be an event later to adjust the T-state for us.
Besides, we can remove the quirk later.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=90041
Reported-by: Kadir <kadir@colakoglu.nl>
Reported-by: Victor Trac <victor.trac@gmail.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
drivers/acpi/sleep.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index cad1a0f..8802ffd 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -870,8 +870,51 @@ static int acpi_syscore_suspend(void)
return acpi_save_bm_rld();
}
+#ifdef CONFIG_X86
+static long msr_fix_fn(void *data)
+{
+ u64 msr;
+
+ if (this_cpu_read(cpu_info.x86_vendor) != X86_VENDOR_INTEL)
+ return 0;
+
+ /*
+ * It was found after resumed from suspend to ram, some BIOSes would
+ * adjust the MSR tstate, however on these platforms no _PSS is provided
+ * thus we never have a chance to adjust the MSR T-state anymore.
+ * Thus force clearing it if MSR T-state is enabled, because generally
+ * we never expect to come back from resume(or CPU online) with
+ * throttling enabled. Later let other components to adjust the
+ * T-state if necessary.
+ */
+ if (!rdmsrl_safe(MSR_IA32_THERM_CONTROL, &msr) && msr) {
+ pr_err("PM: The MSR T-state is enabled after CPU%d online, clear it.\n",
+ smp_processor_id());
+ wrmsrl_safe(MSR_IA32_THERM_CONTROL, 0);
+ }
+ return 0;
+}
+
+static int msr_fix_cpu_online(unsigned int cpu)
+{
+ work_on_cpu(cpu, msr_fix_fn, NULL);
+ return 0;
+}
+#else
+static long msr_fix_fn(void *data)
+{
+ return 0;
+}
+static int msr_fix_cpu_online(unsigned int cpu)
+{
+ return 0;
+}
+#endif
+
static void acpi_syscore_restore(void)
{
+ /* Fix the boot CPU. */
+ msr_fix_fn(NULL);
acpi_restore_bm_rld();
}
@@ -883,6 +926,9 @@ static struct syscore_ops acpi_sleep_syscore_ops = {
void acpi_sleep_syscore_init(void)
{
register_syscore_ops(&acpi_sleep_syscore_ops);
+ cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
+ "msr_fix:online",
+ msr_fix_cpu_online, NULL);
}
#else
static inline void acpi_sleep_syscore_init(void) {}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed
@ 2017-08-25 21:40 Doug Smythies
2017-08-26 6:08 ` Chen Yu
2017-08-26 14:26 ` Doug Smythies
0 siblings, 2 replies; 8+ messages in thread
From: Doug Smythies @ 2017-08-25 21:40 UTC (permalink / raw)
To: 'Chen Yu', linux-acpi
Cc: linux-pm, 'Rafael J. Wysocki', 'Len Brown'
On 2017.08.25 09:43 Chen Yu wrote:
> There is a growing number of reports that the MSR throttling has
> been enabled after resumed back from suspend to ram, which impacts
> the system performance. This patchset tries to address this issue
> by turning off the T-state after resumed back.
>
> Chen Yu (2):
> ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
> ACPI / PM: Disable the MSR T-state during CPU online
>
> drivers/acpi/sleep.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 58 insertions(+), 2 deletions(-)
Hi Chen,
I'll just copy and paste what I wrote in a couple of the bug reports ([1] for example) a couple of days ago:
> I believe that pending changes to the intel-pstate CPU frequency scaling driver,
> proposed by Rafael, I think for kernel 4.14-rc1, will eliminate the ongoing
> troubles with Clock Modulation and the driver.
>
> I'm saying that the issue will no longer exist, and that the intel_pstate
> CPU frequency scaling driver will respond "properly" to Clock Modulation events.
> Of course, I'll check when kernel 4.14-rc1 becomes available, or before if I can
> apply all the patches.
>
> For reference see the patch set related to:
>
> [PATCH 0/2] cpufreq: intel_pstate: Eliminate the PID controller
> 2017.07.24 03:22 (or 10:22 UTC, I think)
>
> https://marc.info/?l=linux-pm&m=150093486908759&w=2
> https://marc.info/?l=linux-pm&m=150093484308751&w=2
> https://marc.info/?l=linux-pm&m=150093486808758&w=2
[1] https://bugzilla.kernel.org/show_bug.cgi?id=189861
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed
2017-08-25 21:40 [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed Doug Smythies
@ 2017-08-26 6:08 ` Chen Yu
2017-08-26 14:26 ` Doug Smythies
1 sibling, 0 replies; 8+ messages in thread
From: Chen Yu @ 2017-08-26 6:08 UTC (permalink / raw)
To: Doug Smythies
Cc: linux-acpi, linux-pm, 'Rafael J. Wysocki',
'Len Brown'
Hi Doug,
On Fri, Aug 25, 2017 at 02:40:19PM -0700, Doug Smythies wrote:
> On 2017.08.25 09:43 Chen Yu wrote:
>
> > There is a growing number of reports that the MSR throttling has
> > been enabled after resumed back from suspend to ram, which impacts
> > the system performance. This patchset tries to address this issue
> > by turning off the T-state after resumed back.
> >
> > Chen Yu (2):
> > ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
> > ACPI / PM: Disable the MSR T-state during CPU online
> >
> > drivers/acpi/sleep.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 58 insertions(+), 2 deletions(-)
>
> Hi Chen,
>
> I'll just copy and paste what I wrote in a couple of the bug reports ([1] for example) a couple of days ago:
>
> > I believe that pending changes to the intel-pstate CPU frequency scaling driver,
> > proposed by Rafael, I think for kernel 4.14-rc1, will eliminate the ongoing
> > troubles with Clock Modulation and the driver.
> >
> > I'm saying that the issue will no longer exist, and that the intel_pstate
> > CPU frequency scaling driver will respond "properly" to Clock Modulation events.
> > Of course, I'll check when kernel 4.14-rc1 becomes available, or before if I can
> > apply all the patches.
> >
> > For reference see the patch set related to:
> >
> > [PATCH 0/2] cpufreq: intel_pstate: Eliminate the PID controller
> > 2017.07.24 03:22 (or 10:22 UTC, I think)
> >
> > https://marc.info/?l=linux-pm&m=150093486908759&w=2
> > https://marc.info/?l=linux-pm&m=150093484308751&w=2
> > https://marc.info/?l=linux-pm&m=150093486808758&w=2
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=189861
>
>
I did not quite catup with the relationship between the intel_pstate
patch and the tstate patch here, if the Clock Modulation is enabled,
will the 'performance' governor be able to reach the maximum freq?
Besides, what if the user is using acpi-freq rather than intel_pstate?
Thanks,
Yu
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed
2017-08-25 21:40 [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed Doug Smythies
2017-08-26 6:08 ` Chen Yu
@ 2017-08-26 14:26 ` Doug Smythies
2017-08-26 16:07 ` Chen Yu
2017-08-27 18:52 ` Doug Smythies
1 sibling, 2 replies; 8+ messages in thread
From: Doug Smythies @ 2017-08-26 14:26 UTC (permalink / raw)
To: 'Chen Yu'
Cc: linux-acpi, linux-pm, 'Rafael J. Wysocki',
'Len Brown'
On 2017.08.25 23:08 Chen Yu wrote:
> Hi Doug,
> On Fri, Aug 25, 2017 at 02:40:19PM -0700, Doug Smythies wrote:
>> On 2017.08.25 09:43 Chen Yu wrote:
>>
>>> There is a growing number of reports that the MSR throttling has
>>> been enabled after resumed back from suspend to ram, which impacts
>>> the system performance. This patchset tries to address this issue
>>> by turning off the T-state after resumed back.
>>>
>>> Chen Yu (2):
>>> ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
>>> ACPI / PM: Disable the MSR T-state during CPU online
>>>
>>> drivers/acpi/sleep.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>
>> Hi Chen,
>>
>> I'll just copy and paste what I wrote in a couple of the bug reports
>> ([1] for example) a couple of days ago:
>>
>>> I believe that pending changes to the intel-pstate CPU frequency scaling driver,
>>> proposed by Rafael, I think for kernel 4.14-rc1, will eliminate the ongoing
>>> troubles with Clock Modulation and the driver.
>>>
>>> I'm saying that the issue will no longer exist, and that the intel_pstate
>>> CPU frequency scaling driver will respond "properly" to Clock Modulation events.
>>> Of course, I'll check when kernel 4.14-rc1 becomes available, or before if I can
>>> apply all the patches.
>>>
>>> For reference see the patch set related to:
>>>
>>> [PATCH 0/2] cpufreq: intel_pstate: Eliminate the PID controller
>>> 2017.07.24 03:22 (or 10:22 UTC, I think)
>>>
>>> https://marc.info/?l=linux-pm&m=150093486908759&w=2
>>> https://marc.info/?l=linux-pm&m=150093484308751&w=2
>>> https://marc.info/?l=linux-pm&m=150093486808758&w=2
>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=189861
>>
>>
> I did not quite catup with the relationship between the intel_pstate
> patch and the tstate patch here, if the Clock Modulation is enabled,
> will the 'performance' governor be able to reach the maximum freq?
No, it'll only reach the max * modulation %.
However, I submit that has never been the problem.
The problem, and why the complaints started, is with the intel_pstate
"powersave" governor and the "get_target_pstate_use_performance" path
therein.
Recall 1: At one time the "get_target_pstate_use_performance" path was
the only path through the driver, and that many of the complaints date
back to then. i.e. for many users that would now use the
"get_target_pstate_use_cpu_load" path by default, their reason for complaining
has already been solved.
Recall 2: The "get_target_pstate_use_performance" path in the intel_pstate
CPU scaling driver is fundamentally incompatible with Clock Modulation and will
never, under any conditions, raise the CPU frequency above the minimum * the
modulation %. This is where the complaining came from.
> Besides, what if the user is using acpi-freq rather than intel_pstate?
As with any "load" based algorithm, it works fine with Clock Modulation,
resulting in desired frequency * modulation percent.
I submit that this is what the various manufacturers (Dell, in particular)
that mess with Clock Modulation want.
Conclusion:
Since the above referenced Rafael patch set removes the
"get_target_pstate_use_performance" path entirely, there is no longer
a problem that needs to be solved.
... Doug
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed
2017-08-26 14:26 ` Doug Smythies
@ 2017-08-26 16:07 ` Chen Yu
2017-08-27 18:52 ` Doug Smythies
1 sibling, 0 replies; 8+ messages in thread
From: Chen Yu @ 2017-08-26 16:07 UTC (permalink / raw)
To: Doug Smythies
Cc: linux-acpi, linux-pm, 'Rafael J. Wysocki',
'Len Brown'
Hi Doug,
On Sat, Aug 26, 2017 at 07:26:18AM -0700, Doug Smythies wrote:
> On 2017.08.25 23:08 Chen Yu wrote:
> > Hi Doug,
> > On Fri, Aug 25, 2017 at 02:40:19PM -0700, Doug Smythies wrote:
> >> On 2017.08.25 09:43 Chen Yu wrote:
> >>
> >>> There is a growing number of reports that the MSR throttling has
> >>> been enabled after resumed back from suspend to ram, which impacts
> >>> the system performance. This patchset tries to address this issue
> >>> by turning off the T-state after resumed back.
> >>>
> >>> Chen Yu (2):
> >>> ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
> >>> ACPI / PM: Disable the MSR T-state during CPU online
> >>>
> >>> drivers/acpi/sleep.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>> 1 file changed, 58 insertions(+), 2 deletions(-)
> >>
> >> Hi Chen,
> >>
> >> I'll just copy and paste what I wrote in a couple of the bug reports
> >> ([1] for example) a couple of days ago:
> >>
> >>> I believe that pending changes to the intel-pstate CPU frequency scaling driver,
> >>> proposed by Rafael, I think for kernel 4.14-rc1, will eliminate the ongoing
> >>> troubles with Clock Modulation and the driver.
> >>>
> >>> I'm saying that the issue will no longer exist, and that the intel_pstate
> >>> CPU frequency scaling driver will respond "properly" to Clock Modulation events.
> >>> Of course, I'll check when kernel 4.14-rc1 becomes available, or before if I can
> >>> apply all the patches.
> >>>
> >>> For reference see the patch set related to:
> >>>
> >>> [PATCH 0/2] cpufreq: intel_pstate: Eliminate the PID controller
> >>> 2017.07.24 03:22 (or 10:22 UTC, I think)
> >>>
> >>> https://marc.info/?l=linux-pm&m=150093486908759&w=2
> >>> https://marc.info/?l=linux-pm&m=150093484308751&w=2
> >>> https://marc.info/?l=linux-pm&m=150093486808758&w=2
> >>
> >> [1] https://bugzilla.kernel.org/show_bug.cgi?id=189861
> >>
> >>
> > I did not quite catup with the relationship between the intel_pstate
> > patch and the tstate patch here, if the Clock Modulation is enabled,
> > will the 'performance' governor be able to reach the maximum freq?
>
> No, it'll only reach the max * modulation %.
> However, I submit that has never been the problem.
> The problem, and why the complaints started, is with the intel_pstate
> "powersave" governor and the "get_target_pstate_use_performance" path
> therein.
>
Ok, I think I get the point now: The current PID prediction algorithom
does not take tstate into consideration, so it might behave wrong.
And according to my understanding, the problem in PID does not
involve with the suspend/resume process. However my concern is
that, why did the BIOS enable the tstate during S3? And if the
BIOS has enabled the tstate, does the kernel have a chance to
adjust it?
bugzilla 189861 and 90041(also reported on another server) have shown
that the kernel failed to re-evaluate the tstate after resumed, and
keep it throttled all the time. Actually in current code there is a
chance the kernel could re-evaluate the tstate once resumed back, but
due to the insufficient ACPI information provided by the BIOS on these
platforms, the tstate keeps un-touched after resumed back.
Thus this appears to be BIOS issue.
To work around the BIOS issue, we can manipulate the MSR by msr-tools,
but it might not be a robusty solution, instead we can clear the tstate
after resumed back, that is to say, we do not trust the modification during
S3 by the BIOS, and let the OS decide if it wants the throttling or not.
> Recall 1: At one time the "get_target_pstate_use_performance" path was
> the only path through the driver, and that many of the complaints date
> back to then. i.e. for many users that would now use the
> "get_target_pstate_use_cpu_load" path by default, their reason for complaining
> has already been solved.
>
> Recall 2: The "get_target_pstate_use_performance" path in the intel_pstate
> CPU scaling driver is fundamentally incompatible with Clock Modulation and will
> never, under any conditions, raise the CPU frequency above the minimum * the
> modulation %. This is where the complaining came from.
>
> > Besides, what if the user is using acpi-freq rather than intel_pstate?
>
> As with any "load" based algorithm, it works fine with Clock Modulation,
> resulting in desired frequency * modulation percent.
> I submit that this is what the various manufacturers (Dell, in particular)
> that mess with Clock Modulation want.
>
> Conclusion:
> Since the above referenced Rafael patch set removes the
> "get_target_pstate_use_performance" path entirely, there is no longer
> a problem that needs to be solved.
>
Well, it's hard to say that if our customers would be happy if the throttling
is enabled wrongly by the BIOS, and they don't know why they can not get good
performance : )
Thanks,
Yu
> ... Doug
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed
2017-08-26 14:26 ` Doug Smythies
2017-08-26 16:07 ` Chen Yu
@ 2017-08-27 18:52 ` Doug Smythies
1 sibling, 0 replies; 8+ messages in thread
From: Doug Smythies @ 2017-08-27 18:52 UTC (permalink / raw)
To: 'Chen Yu'
Cc: linux-acpi, linux-pm, 'Rafael J. Wysocki',
'Len Brown'
Hi Yu,
On 2017.08.26 09:07 Chen Yu wrote:
> On Sat, Aug 26, 2017 at 07:26:18AM -0700, Doug Smythies wrote:
>> On 2017.08.25 23:08 Chen Yu wrote:
>>> Hi Doug,
>>> On Fri, Aug 25, 2017 at 02:40:19PM -0700, Doug Smythies wrote:
>>>> On 2017.08.25 09:43 Chen Yu wrote:
>>>>
>>>>> There is a growing number of reports that the MSR throttling has
>>>>> been enabled after resumed back from suspend to ram, which impacts
>>>>> the system performance. This patchset tries to address this issue
>>>>> by turning off the T-state after resumed back.
>>>>>
>>>>> Chen Yu (2):
>>>>> ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement
>>>>> ACPI / PM: Disable the MSR T-state during CPU online
>>>>>
>>>>> drivers/acpi/sleep.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>>>>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>>>
>>>> Hi Chen,
>>>>
>>>> I'll just copy and paste what I wrote in a couple of the bug reports
>>>> ([1] for example) a couple of days ago:
>>>>
>>>>> I believe that pending changes to the intel-pstate CPU frequency scaling driver,
>>>>> proposed by Rafael, I think for kernel 4.14-rc1, will eliminate the ongoing
>>>>> troubles with Clock Modulation and the driver.
>>>>>
>>>>> I'm saying that the issue will no longer exist, and that the intel_pstate
>>>>> CPU frequency scaling driver will respond "properly" to Clock Modulation events.
>>>>> Of course, I'll check when kernel 4.14-rc1 becomes available, or before if I can
>>>>> apply all the patches.
>>>>>
>>>>> For reference see the patch set related to:
>>>>>
>>>>> [PATCH 0/2] cpufreq: intel_pstate: Eliminate the PID controller
>>>>> 2017.07.24 03:22 (or 10:22 UTC, I think)
>>>>>
>>>>> https://marc.info/?l=linux-pm&m=150093486908759&w=2
>>>>> https://marc.info/?l=linux-pm&m=150093484308751&w=2
>>>>> https://marc.info/?l=linux-pm&m=150093486808758&w=2
>>>>
>>>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=189861
>>>>
>>>>
>>> I did not quite catup with the relationship between the intel_pstate
>>> patch and the tstate patch here, if the Clock Modulation is enabled,
>>> will the 'performance' governor be able to reach the maximum freq?
>>
>> No, it'll only reach the max * modulation %.
>> However, I submit that has never been the problem.
>> The problem, and why the complaints started, is with the intel_pstate
>> "powersave" governor and the "get_target_pstate_use_performance" path
>> therein.
>>
> Ok, I think I get the point now: The current PID prediction algorithom
> does not take tstate into consideration, so it might behave wrong.
There is no "might" about it. It does behave wrong. We went through
all of this almost 2 years ago. The CPU frequency will basically
freeze at the minimum * the modulation setting.
What does this mean? Let's say, for example, the processor minimum
pstate is 8 and the maximum pstate is 37. At 75% Clock Modulation
the system will lock at 600 MHz, whereas it would normally be expected
to still be able to get to 2.7 GHz. Another way to say it is that the
performance becomes limited to 16.2% of the normal maximum instead of
the expected 75%.
> And according to my understanding, the problem in PID does not
> involve with the suspend/resume process. However my concern is
> that, why did the BIOS enable the tstate during S3?
I'm saying that in some cases (some Dell LapTops, in particular),
it is changed on purpose and as a function of emerging from S3 on
AC power or DC power.
However, and having reviewed all the notes and e-mails from almost
2 years ago, I do now recall that you had a case, where the Clock
Modulation was set to an undefined value, with different results on
different processors. At the time we (O.K. I) concluded (from [1]):
"Yu's specific example is special, because the Clock Modulation
percent is undefined (reserved), and thus clearly unintentional,
or just plain wrong if it is intentional."
> And if the
> BIOS has enabled the tstate, does the kernel have a chance to
> adjust it?
>
> bugzilla 189861 and 90041(also reported on another server) have shown
> that the kernel failed to re-evaluate the tstate after resumed, and
> keep it throttled all the time. Actually in current code there is a
> chance the kernel could re-evaluate the tstate once resumed back, but
> due to the insufficient ACPI information provided by the BIOS on these
> platforms, the tstate keeps un-touched after resumed back.
>
> Thus this appears to be BIOS issue.
Is it?
I am saying that sometimes the manufacturer has done it on purpose.
> To work around the BIOS issue, we can manipulate the MSR by msr-tools,
> but it might not be a robusty solution, instead we can clear the tstate
> after resumed back, that is to say, we do not trust the modification during
> S3 by the BIOS, and let the OS decide if it wants the throttling or not.
If indeed, we do not want to trust the change, then O.K I'll stop bothering
you about it.
>> Recall 1: At one time the "get_target_pstate_use_performance" path was
>> the only path through the driver, and that many of the complaints date
>> back to then. i.e. for many users that would now use the
>> "get_target_pstate_use_cpu_load" path by default, their reason for complaining
>> has already been solved.
>>
>> Recall 2: The "get_target_pstate_use_performance" path in the intel_pstate
>> CPU scaling driver is fundamentally incompatible with Clock Modulation and will
>> never, under any conditions, raise the CPU frequency above the minimum * the
>> modulation %. This is where the complaining came from.
>>
>>> Besides, what if the user is using acpi-freq rather than intel_pstate?
>>
>> As with any "load" based algorithm, it works fine with Clock Modulation,
>> resulting in desired frequency * modulation percent.
>> I submit that this is what the various manufacturers (Dell, in particular)
>> that mess with Clock Modulation want.
>>
>> Conclusion:
>> Since the above referenced Rafael patch set removes the
>> "get_target_pstate_use_performance" path entirely, there is no longer
>> a problem that needs to be solved.
>>
> Well, it's hard to say that if our customers would be happy if the throttling
> is enabled wrongly by the BIOS,
Wrongly or because that is what the manufacturer wants?
And should we override that?
> and they don't know why they can not get good
> performance : )
Keep in mind how dramatic the reduction in performance was with
the "get_target_pstate_use_performance" algorithm verses the others.
The difference is enormous.
[1] https://marc.info/?l=linux-kernel&m=144441695612734&w=2
... Doug
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-08-27 18:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-25 16:43 [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed Chen Yu
2017-08-25 16:43 ` [PATCH 1/2][RFC] ACPI / PM: Reuse the acpi_sleep_syscore_ops for future requirement Chen Yu
2017-08-25 16:43 ` [PATCH 2/2][RFC] ACPI / PM: Disable the MSR T-state during CPU online Chen Yu
-- strict thread matches above, loose matches on Subject: below --
2017-08-25 21:40 [PATCH 0/2][RFC] ACPI / PM: Disable MSR T-state after resumed Doug Smythies
2017-08-26 6:08 ` Chen Yu
2017-08-26 14:26 ` Doug Smythies
2017-08-26 16:07 ` Chen Yu
2017-08-27 18:52 ` Doug Smythies
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).