cpufreq: intel_pstate: set stale CPU frequency to minimum

PATCH Applied Doug Smythies 4 participants 7 messages Aug 20, 2023 – Sep 5, 2023

Subsystems (2)

CPU frequency scaling framework Intel pstate driver

lore

Share & tools

lore

Doug Smythies

author

2023-08-20 21:24 UTC #0 lore raw

PATCH v2 1/1 cpufreq: intel_pstate: set stale CPU frequency to minimum

The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.

This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.

Reported-by Yang Jie <[email protected]>

Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597

Signed-off-by Doug Smythies <[email protected]>

---

v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.

---
 drivers/cpufreq/intel_pstate.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/cpufreq/intel_pstate.c +5 -0

index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                        intel_pstate_clear_update_util_hook(policy->cpu);
                intel_pstate_hwp_set(policy->cpu);
        }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 
        mutex_unlock(&intel_pstate_limits_lock);

--
2.25.1

Rafael J. Wysocki

maintainer Applied

2023-08-22 11:48 UTC #1 lore raw

Quoted 41 lines · Doug Smythies

The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.
This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.
Reported-by Yang Jie <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597
Signed-off-by Doug Smythies <[email protected]>
---
v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.
---
 drivers/cpufreq/intel_pstate.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/cpufreq/intel_pstate.c +0 -0
index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                        intel_pstate_clear_update_util_hook(policy->cpu);
                intel_pstate_hwp_set(policy->cpu);

drivers/cpufreq/intel_pstate.c static int intel_pstate_set_policy(struct cpufreq_policy *policy)

Quoted 10 lines

drivers/cpufreq/intel_pstate.c +5 -1

        }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 
        mutex_unlock(&intel_pstate_limits_lock);
 
--

Applied as 6.6 material, with some mailer-induced white space damage fixed and the new comment adjusted to the kernel coding style.

Thanks!

Keyon Jie

2023-08-28 23:37 UTC #2 lore raw

Quoted 49 lines · Rafael J. Wysocki

Quoted 48 lines · Doug Smythies
The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.
This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.
Reported-by Yang Jie <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597
Signed-off-by Doug Smythies <[email protected]>
---
v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.
---
  drivers/cpufreq/intel_pstate.c | 5 +++++
  1 file changed, 5 insertions(+)
drivers/cpufreq/intel_pstate.c +5 -0
index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                         intel_pstate_clear_update_util_hook(policy->cpu);
                 intel_pstate_hwp_set(policy->cpu);
         }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 

Quoted 8 lines

drivers/cpufreq/intel_pstate.c static int intel_pstate_set_policy(struct cpufreq_policy *policy)
Quoted 3 lines
drivers/cpufreq/intel_pstate.c +0 -1
         mutex_unlock(&intel_pstate_limits_lock);
 
--
Applied as 6.6 material, with some mailer-induced white space damage fixed and the new comment adjusted to the kernel coding style.
Thanks!

Hi Doug and Rafael,

Thank you for making the fix happen.

Hi Greg,

Will this be picked to the stable linux-6.1.y and linux-6.4.y kernel, it could benefit to users there.

Thanks,
~Keyon

Greg KH

2023-08-29 08:58 UTC #3 lore raw

Quoted 61 lines · Keyon Jie

Quoted 49 lines · Rafael J. Wysocki
Quoted 48 lines · Doug Smythies
The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.
This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.
Reported-by Yang Jie <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597
Signed-off-by Doug Smythies <[email protected]>
---
v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.
---
  drivers/cpufreq/intel_pstate.c | 5 +++++
  1 file changed, 5 insertions(+)
drivers/cpufreq/intel_pstate.c +5 -0
index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                         intel_pstate_clear_update_util_hook(policy->cpu);
                 intel_pstate_hwp_set(policy->cpu);
         }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 
Quoted 8 lines
drivers/cpufreq/intel_pstate.c static int intel_pstate_set_policy(struct cpufreq_policy *policy)
Quoted 3 lines
drivers/cpufreq/intel_pstate.c +0 -1
         mutex_unlock(&intel_pstate_limits_lock);
 
--
Applied as 6.6 material, with some mailer-induced white space damage fixed and the new comment adjusted to the kernel coding style.
Thanks!

Quoted 8 lines

Hi Doug and Rafael,
Thank you for making the fix happen.
Hi Greg,
Will this be picked to the stable linux-6.1.y and linux-6.4.y kernel, it could benefit to users there.

Sure, when it hits Linus's tree, please follow the instructions in:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to get it merged.

thanks,

greg k-h

Keyon Jie

2023-09-05 18:20 UTC #4 lore raw

Quoted 66 lines · Greg KH

Quoted 65 lines · Keyon Jie
Quoted 49 lines · Rafael J. Wysocki
Quoted 48 lines · Doug Smythies
The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.
This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.
Reported-by Yang Jie <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597
Signed-off-by Doug Smythies <[email protected]>
---
v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.
---
   drivers/cpufreq/intel_pstate.c | 5 +++++
   1 file changed, 5 insertions(+)
drivers/cpufreq/intel_pstate.c +5 -0
index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                          intel_pstate_clear_update_util_hook(policy->cpu);
                  intel_pstate_hwp_set(policy->cpu);
          }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 
Quoted 8 lines
drivers/cpufreq/intel_pstate.c static int intel_pstate_set_policy(struct cpufreq_policy *policy)
Quoted 3 lines
drivers/cpufreq/intel_pstate.c +0 -1
          mutex_unlock(&intel_pstate_limits_lock);
 
--
Applied as 6.6 material, with some mailer-induced white space damage fixed and the new comment adjusted to the kernel coding style.
Thanks!
Hi Doug and Rafael,
Thank you for making the fix happen.

Quoted 8 lines

Quoted 4 lines
Hi Greg,
Will this be picked to the stable linux-6.1.y and linux-6.4.y kernel, it could benefit to users there.
Sure, when it hits Linus's tree, please follow the instructions in:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to get it merged.

Thank you Greg.

Hi Rafael,

As the issue happens from the 5.18 onward kernels, we need the fix to be picked for stable kernels after that, could you please help to add "Cc: [email protected]" in the sign-off area when you send it to the mainline?

Thank you,
~Keyon

Quoted 4 lines

thanks,
greg k-h

Rafael J. Wysocki

maintainer

2023-09-05 18:32 UTC #5 lore raw

Quoted 79 lines · Keyon Jie

Quoted 66 lines · Greg KH
Quoted 65 lines · Keyon Jie
Quoted 49 lines · Rafael J. Wysocki
Quoted 48 lines · Doug Smythies
The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.
This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.
Reported-by Yang Jie <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597
Signed-off-by Doug Smythies <[email protected]>
---
v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.
---
   drivers/cpufreq/intel_pstate.c | 5 +++++
   1 file changed, 5 insertions(+)
drivers/cpufreq/intel_pstate.c +5 -0
index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                          intel_pstate_clear_update_util_hook(policy->cpu);
                  intel_pstate_hwp_set(policy->cpu);
          }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 
Quoted 8 lines
drivers/cpufreq/intel_pstate.c static int intel_pstate_set_policy(struct cpufreq_policy *policy)
Quoted 3 lines
drivers/cpufreq/intel_pstate.c +0 -1
          mutex_unlock(&intel_pstate_limits_lock);
 
--
Applied as 6.6 material, with some mailer-induced white space damage fixed and the new comment adjusted to the kernel coding style.
Thanks!
Hi Doug and Rafael,
Thank you for making the fix happen.
Quoted 8 lines
Quoted 4 lines
Hi Greg,
Will this be picked to the stable linux-6.1.y and linux-6.4.y kernel, it could benefit to users there.
Sure, when it hits Linus's tree, please follow the instructions in:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to get it merged.

Quoted 8 lines

Thank you Greg.
Hi Rafael,
As the issue happens from the 5.18 onward kernels, we need the fix to be picked for stable kernels after that, could you please help to add "Cc:
[email protected]" in the sign-off area when you send it to the mainline?

It's already merged, as commit d51847acb018 ("cpufreq: intel_pstate:
set stale CPU frequency to minimum").

Please feel free to send an inclusion request for it to [email protected]

Thanks!

Keyon Jie

2023-09-05 22:23 UTC #6 lore raw

Quoted 89 lines · Rafael J. Wysocki

Quoted 79 lines · Keyon Jie
Quoted 66 lines · Greg KH
Quoted 65 lines · Keyon Jie
Quoted 49 lines · Rafael J. Wysocki
Quoted 48 lines · Doug Smythies
The intel_pstate CPU frequency scaling driver does not use policy->cur and it is 0.
When the CPU frequency is outdated arch_freq_get_on_cpu() will default to the nominal clock frequency when its call to cpufreq_quick_getpolicy_cur returns the never updated 0. Thus, the listed frequency might be outside of currently set limits. Some users are complaining about the high reported frequency, albeit stale, when their system is idle and/or it is above the reduced maximum they have set.
This patch will maintain policy_cur for the intel_pstate driver at the current minimum CPU frequency.
Reported-by Yang Jie <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=217597
Signed-off-by Doug Smythies <[email protected]>
---
v1 -> v2:
* v1 was a completely different approach, programming around the issue rather than fixing it at the source.
reference:
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
* v2 does not fix an issue with the intel_cpufreq CPU scaling driver (A.K.A. the intel_pstate driver in passive mode) and the schedutil CPU frequency scaling governor when HWP is enabled where limit changes are not reflected in the stale listed frequencies. A fix for that will be some future patch.
---
    drivers/cpufreq/intel_pstate.c | 5 +++++
    1 file changed, 5 insertions(+)
drivers/cpufreq/intel_pstate.c +5 -0
index 8ca2bce4341a..08284dee583a 100644
@@ -2609,6 +2609,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
                           intel_pstate_clear_update_util_hook(policy->cpu);
                   intel_pstate_hwp_set(policy->cpu);
           }
+       /* policy current is never updated with the intel_pstate driver
+        * but it is used as a stale frequency value. So, keep it within
+        * limits.
+        */
+       policy->cur = policy->min;
 
Quoted 8 lines
drivers/cpufreq/intel_pstate.c static int intel_pstate_set_policy(struct cpufreq_policy *policy)
Quoted 3 lines
drivers/cpufreq/intel_pstate.c +0 -1
           mutex_unlock(&intel_pstate_limits_lock);
 
--
Applied as 6.6 material, with some mailer-induced white space damage fixed and the new comment adjusted to the kernel coding style.
Thanks!
Hi Doug and Rafael,
Thank you for making the fix happen.
Quoted 8 lines
Quoted 4 lines
Hi Greg,
Will this be picked to the stable linux-6.1.y and linux-6.4.y kernel, it could benefit to users there.
Sure, when it hits Linus's tree, please follow the instructions in:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to get it merged.
Quoted 8 lines
Thank you Greg.
Hi Rafael,
As the issue happens from the 5.18 onward kernels, we need the fix to be picked for stable kernels after that, could you please help to add "Cc:
[email protected]" in the sign-off area when you send it to the mainline?

Quoted 4 lines

It's already merged, as commit d51847acb018 ("cpufreq: intel_pstate:
set stale CPU frequency to minimum").
Please feel free to send an inclusion request for it to [email protected]

Thank you Rafael, will do.

Thanks,
~Keyon

Quoted 2 lines

Thanks!