cpufreq: Fix GOV_LIMITS handling for the userspace governor
PATCH v2 1/1 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 storing the user requested frequency in a seperate variable. The governor will then try to achieve this request on every GOV_LIMITS change.
---
Changes in v2:
- Used policy->governor_data rather than using a per-cpu variable
drivers/cpufreq/cpufreq_userspace.c | 43 ++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-)
(quoted diff) +0 -0
b/drivers/cpufreq/cpufreq_userspace.c
index 4d16f45..9f3dec9 100644
drivers/cpufreq/cpufreq_userspace.c +2 -0
@@ -17,6 +17,7 @@#include <linux/init.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/slab.h> static DEFINE_PER_CPU(unsigned int, cpu_is_managed);static DEFINE_MUTEX (userspace_mutex);
@@ -31,6 +32,7 @@ static DEFINE_MUTEX(userspace_mutex);static int cpufreq_set (struct cpufreq_policy *policy,unsigned int freq) {int ret = -EINVAL ; +unsigned int *setspeed = policy->governor_data;pr_debug ("cpufreq_set for cpu %u, freq %u kHz\n" , policy->cpu, freq);
@@ -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;
+ *setspeed = freq;
+
ret = __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L);
err:
mutex_unlock(&userspace_mutex);
@@ -49,19 +53,45 @@ static ssize_t show_speed(struct cpufreq_policy *policy, char *buf)
return sprintf(buf, "%u\n", policy->cur);
}
+static int cpufreq_userspace_policy_init(struct cpufreq_policy *policy) +{
+ unsigned int *setspeed;
+
+ setspeed = kzalloc(sizeof(*setspeed), GFP_KERNEL);
+ if (!setspeed)
+ return -ENOMEM;
+
+ policy->governor_data = setspeed;
+ return 0;
+}
+
static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
unsigned int event)
{
+ unsigned int *setspeed = policy->governor_data;
unsigned int cpu = policy->cpu;
int rc = 0;
+ if (event == CPUFREQ_GOV_POLICY_INIT)
+ return cpufreq_userspace_policy_init(policy);
+
+ if (!setspeed)
+ return -EINVAL;
+
switch (event) {
+ case CPUFREQ_GOV_POLICY_EXIT:
+ mutex_lock(&userspace_mutex);
+ policy->governor_data = NULL;
+ kfree(setspeed);
+ mutex_unlock(&userspace_mutex);
+ break;
case CPUFREQ_GOV_START:
BUG_ON(!policy->cur); pr_debug("started managing cpu %u\n", cpu);
mutex_lock(&userspace_mutex); per_cpu(cpu_is_managed, cpu) = 1;
+ *setspeed = policy->cur;
mutex_unlock(&userspace_mutex);
break;
case CPUFREQ_GOV_STOP:
@@ -69,20 +99,23 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
mutex_lock(&userspace_mutex);
per_cpu(cpu_is_managed, cpu) = 0;
+ *setspeed = 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",
- cpu, policy->min, policy->max,
- policy->cur);
+ 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, *setspeed);
- if (policy->max < policy->cur)
+ if (policy->max < *setspeed)
__cpufreq_driver_target(policy, policy->max,
CPUFREQ_RELATION_H);
- else if (policy->min > policy->cur)
+ else if (policy->min > *setspeed)
__cpufreq_driver_target(policy, policy->min,
CPUFREQ_RELATION_L);
+ else
+ __cpufreq_driver_target(policy, *setspeed,
+ CPUFREQ_RELATION_L);
mutex_unlock(&userspace_mutex);
break;
}
Signature · 1 line
2.1.4
Quoted 17 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 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 storing the user requested frequency in a seperate variable. The governor will then try to achieve this request on every
GOV_LIMITSchange.Fixes d1922f02562f ("cpufreq: Simplify userspace governor")
Quoted 8 lines
Signed-off-by Sai Gurrappadi <[email protected]>---
Changes in v2:
- Usedpolicy->governor_datarather than using a per-cpu variabledrivers/cpufreq/cpufreq_userspace.c | 43 ++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-)
Signature · 1 line
viresh
Quoted 21 lines · Viresh Kumar
Quoted 20 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 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 storing the user requested frequency in a seperate variable. The governor will then try to achieve this request on every
GOV_LIMITSchange.Fixes d1922f02562f ("cpufreq: Simplify userspace governor")Signed-off-by Sai Gurrappadi <[email protected]>---
Quoted 8 lines
Quoted 5 lines
Changes in v2:
- Usedpolicy->governor_datarather than using a per-cpu variabledrivers/cpufreq/cpufreq_userspace.c | 43 ++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-)Acked-by Viresh Kumar <[email protected]>
Thanks! Do I just wait for Rafael to review/pick it up? Or is there anything else I must do?
-Sai
Quoted 25 lines · Sai Gurrappadi
Quoted 24 lines · Viresh Kumar
Quoted 23 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 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 storing the user requested frequency in a seperate variable. The governor will then try to achieve this request on every
GOV_LIMITSchange.Fixes d1922f02562f ("cpufreq: Simplify userspace governor")Signed-off-by Sai Gurrappadi <[email protected]>---
Changes in v2:
- Usedpolicy->governor_datarather than using a per-cpu variable
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
drivers/cpufreq/cpufreq_userspace.c | 43 ++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-)Acked-by Viresh Kumar <[email protected]>Thanks! Do I just wait for Rafael to review/pick it up? Or is there anything else I must do?
That's been applied already, but your e-mail client munges lines that are more than 80 characters long, so it wasn't fun to apply it.
Please fix that, I'm not going to apply mangled patches from you next time.
Thanks,
Rafael
Quoted 8 lines
Quoted 2 lines
Thanks! Do I just wait for Rafael to review/pick it up? Or is there anything else I must do?
That's been applied already, but your e-mail client munges lines that are more than 80 characters long, so it wasn't fun to apply it.
Please fix that, I'm not going to apply mangled patches from you next time.
Whoops, I thought I fixed that. Sorry, will fix next time!
Thanks.
-Sai