cpufreq: Fix GOV_LIMITS handling for the userspace governor
Currently, the userspace governor only updates frequency on GOV_LIMITS if policy->cur falls outside policy->{min/max}. However, it is also necessary to update current frequency on GOV_LIMITS to match the user requested value if it can be achieved within the new policy->{max/min}.
This was previously the behaviour in the governor until commit d1922f0 ("cpufreq: Simplify userspace governor") which incorrectly assumed that policy->cur == user requested frequency via scaling_setspeed. This won't be true if the user requested frequency falls outside policy->{min/max}.
Ex: a temporary thermal cap throttled the user requested frequency.
Fix this by doing a partial revert of commit d1922f0 to bring back the per-cpu cpu_set_freq variable that stores the user requested frequency. The governor will then try to achieve this request on every GOV_LIMITS.
--- drivers/cpufreq/cpufreq_userspace.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
(quoted diff) +0 -0
b/drivers/cpufreq/cpufreq_userspace.c
index 4d16f45..ae03ba7 100644
drivers/cpufreq/cpufreq_userspace.c +2 -0
@@ -18,6 +18,8 @@#include <linux/module.h> #include <linux/mutex.h> +static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by +userspace */ static DEFINE_PER_CPU(unsigned int, cpu_is_managed);static DEFINE_MUTEX (userspace_mutex);
@@ -38,6 +40,8 @@ static int cpufreq_set(struct cpufreq_policy *policy,
unsigned int freq)
if (!per_cpu(cpu_is_managed, policy->cpu))
goto err;
+ per_cpu(cpu_set_freq, policy->cpu) = freq;
+
ret = __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L);
err:
mutex_unlock(&userspace_mutex);
@@ -62,6 +66,7 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
mutex_lock(&userspace_mutex); per_cpu(cpu_is_managed, cpu) = 1;
+ per_cpu(cpu_set_freq, cpu) = policy->cur;
mutex_unlock(&userspace_mutex);
break;
case CPUFREQ_GOV_STOP:
@@ -69,20 +74,27 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
mutex_lock(&userspace_mutex); per_cpu(cpu_is_managed, cpu) = 0;
+ per_cpu(cpu_set_freq, cpu) = 0;
mutex_unlock(&userspace_mutex); break;
case CPUFREQ_GOV_LIMITS:
mutex_lock(&userspace_mutex);
- pr_debug("limit event for cpu %u: %u - %u kHz, currently %u kHz\n",
+ pr_debug("limit event for cpu %u: %u - %u kHz, "
+ "currently %u kHz, last set to %u kHz\n",
cpu, policy->min, policy->max,
- policy->cur);
+ policy->cur, per_cpu(cpu_set_freq, cpu));
- if (policy->max < policy->cur)
+ if (policy->max < per_cpu(cpu_set_freq, cpu)) {
__cpufreq_driver_target(policy, policy->max,
CPUFREQ_RELATION_H);
- else if (policy->min > policy->cur)
+ } else if (policy->min > per_cpu(cpu_set_freq, cpu)) {
__cpufreq_driver_target(policy, policy->min,
CPUFREQ_RELATION_L);
+ } else {
+ __cpufreq_driver_target(policy,
+ per_cpu(cpu_set_freq, cpu),
+ CPUFREQ_RELATION_L);
+ }
mutex_unlock(&userspace_mutex);
break;
}
Signature · 8 lines
2.1.4 ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
Viresh Kumar
maintainerQuoted 8 lines · Sai Gurrappadi
Currently, the userspace governor only updates frequency on
GOV_LIMITSifpolicy->curfalls outside policy->{min/max}. However, it is also necessary to update current frequency onGOV_LIMITSto match the user requested value if it can be achieved within the new policy->{max/min}.This was previously the behaviour in the governor until commit d1922f0 ("cpufreq: Simplify userspace governor") which incorrectly assumed that
policy->cur== user requested frequency viascaling_setspeed. This won't
Urg...
Quoted 7 lines
be true if the user requested frequency falls outside policy->{min/max}.
Ex: a temporary thermal cap throttled the user requested frequency.Fix this by doing a partial revert of commit d1922f0 to bring back the per-cpu
cpu_set_freqvariable that stores the user requested frequency. The governor will then try to achieve this request on everyGOV_LIMITS.
Add this here (just before your sob)
Quoted 11 lines
Signed-off-by Sai Gurrappadi <[email protected]>--- drivers/cpufreq/cpufreq_userspace.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)drivers/cpufreq/cpufreq_userspace.c +0
-0
drivers/cpufreq/cpufreq_userspace.c line 22
Quoted 5 lines
drivers/cpufreq/cpufreq_userspace.c +2
-0@@ -18,6 +18,8 @@#include <linux/module.h> #include <linux/mutex.h> +static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by +userspace */
We don't have to be this bad now. The policy structure has a governor_data field, please use that for storing any policy specific values.
Quoted 25 lines
drivers/cpufreq/cpufreq_userspace.c +2
-0static DEFINE_PER_CPU(unsigned int, cpu_is_managed);static DEFINE_MUTEX (userspace_mutex);@@ -38,6 +40,8 @@ static int cpufreq_set(struct cpufreq_policy *policy, unsigned int freq)if (!per_cpu (cpu_is_managed, policy->cpu)) goto err; +per_cpu (cpu_set_freq, policy->cpu) = freq; + ret =__cpufreq_driver_target (policy, freq,CPUFREQ_RELATION_L ); err:drivers/cpufreq/cpufreq_userspace.c +1
-0mutex_unlock (&userspace_mutex);@@ -62,6 +66,7 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,mutex_lock (&userspace_mutex);per_cpu (cpu_is_managed, cpu) =1 ; +per_cpu (cpu_set_freq, cpu) = policy->cur;mutex_unlock (&userspace_mutex);break ;drivers/cpufreq/cpufreq_userspace.c +0
-0case CPUFREQ_GOV_STOP :@@ -69,20 +74,27 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
drivers/cpufreq/cpufreq_userspace.c static int cpufreq_governor_userspace(struct · line 84
Quoted 10 lines
drivers/cpufreq/cpufreq_userspace.c +3
-1mutex_lock (&userspace_mutex);per_cpu (cpu_is_managed, cpu) =0 ; +per_cpu (cpu_set_freq, cpu) =0 ;mutex_unlock (&userspace_mutex);break ;case CPUFREQ_GOV_LIMITS :mutex_lock (&userspace_mutex); - pr_debug("limit event for cpu %u: %u - %u kHz, currently %u kHz\n", + pr_debug("limit event for cpu %u: %u - %u kHz, " + "currently %u kHz, last set to %u kHz\n",
*Never* break print messages into multiple lines, let them cross 80 columns.
Quoted 19 lines
drivers/cpufreq/cpufreq_userspace.c +8
-3cpu, policy->min, policy->max, - policy->cur); + policy->cur, per_cpu(cpu_set_freq, cpu)); -if (policy->max < policy->cur) +if (policy->max <per_cpu (cpu_set_freq, cpu)) {__cpufreq_driver_target (policy, policy->max,CPUFREQ_RELATION_H ); -else if (policy->min > policy->cur) + }else if (policy->min >per_cpu (cpu_set_freq, cpu)) {__cpufreq_driver_target (policy, policy->min,CPUFREQ_RELATION_L ); + }else { +__cpufreq_driver_target (policy, +per_cpu (cpu_set_freq, cpu), +CPUFREQ_RELATION_L ); + }mutex_unlock (&userspace_mutex);break ;
Quoted 1 line
}
Good catch :)
Signature · 1 line
viresh