* [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).