2023-09-12 23:30:17

by Liao, Chang

[permalink] [raw]
Subject: [PATCH 2/2] cpufreq: userspace: Move is_managed indicator into per-policy structure

The userspace governor use the 'cpu' field of cpufreq_policy structure
to track if it is allowed to set the speed of policy. However, there is
a window where the 'cpu' field is equal to the value of nr_cpus_id when
all affected CPUs of policy are offline, which is an illegal value to
get the per-CPU variable. To avoid this issue, this patch uses a
per-policy indicator to track if the policy is managed.

Signed-off-by: Liao Chang <[email protected]>
---
drivers/cpufreq/cpufreq_userspace.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
index 442e31060d62..2c42fee76daa 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -15,9 +15,8 @@
#include <linux/mutex.h>
#include <linux/slab.h>

-static DEFINE_PER_CPU(unsigned int, cpu_is_managed);
-
struct userspace_policy {
+ unsigned int is_managed;
unsigned int setspeed;
struct mutex mutex;
};
@@ -37,7 +36,7 @@ static int cpufreq_set(struct cpufreq_policy *policy, unsigned int freq)
pr_debug("cpufreq_set for cpu %u, freq %u kHz\n", policy->cpu, freq);

mutex_lock(&userspace->mutex);
- if (!per_cpu(cpu_is_managed, policy->cpu))
+ if (!userspace->is_managed)
goto err;

userspace->setspeed = freq;
@@ -85,7 +84,7 @@ static int cpufreq_userspace_policy_start(struct cpufreq_policy *policy)
pr_debug("started managing cpu %u\n", policy->cpu);

mutex_lock(&userspace->mutex);
- per_cpu(cpu_is_managed, policy->cpu) = 1;
+ userspace->is_managed = 1;
userspace->setspeed = policy->cur;
mutex_unlock(&userspace->mutex);
return 0;
@@ -98,7 +97,7 @@ static void cpufreq_userspace_policy_stop(struct cpufreq_policy *policy)
pr_debug("managing cpu %u stopped\n", policy->cpu);

mutex_lock(&userspace->mutex);
- per_cpu(cpu_is_managed, policy->cpu) = 0;
+ userspace->is_managed = 0;
userspace->setspeed = 0;
mutex_unlock(&userspace->mutex);
}
--
2.34.1


2023-10-05 14:26:20

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 2/2] cpufreq: userspace: Move is_managed indicator into per-policy structure

On 12-09-23, 06:10, Liao Chang wrote:
> The userspace governor use the 'cpu' field of cpufreq_policy structure
> to track if it is allowed to set the speed of policy. However, there is
> a window where the 'cpu' field is equal to the value of nr_cpus_id when
> all affected CPUs of policy are offline, which is an illegal value to
> get the per-CPU variable. To avoid this issue, this patch uses a
> per-policy indicator to track if the policy is managed.
>
> Signed-off-by: Liao Chang <[email protected]>
> ---
> drivers/cpufreq/cpufreq_userspace.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
> index 442e31060d62..2c42fee76daa 100644
> --- a/drivers/cpufreq/cpufreq_userspace.c
> +++ b/drivers/cpufreq/cpufreq_userspace.c
> @@ -15,9 +15,8 @@
> #include <linux/mutex.h>
> #include <linux/slab.h>
>
> -static DEFINE_PER_CPU(unsigned int, cpu_is_managed);
> -
> struct userspace_policy {
> + unsigned int is_managed;
> unsigned int setspeed;
> struct mutex mutex;
> };
> @@ -37,7 +36,7 @@ static int cpufreq_set(struct cpufreq_policy *policy, unsigned int freq)
> pr_debug("cpufreq_set for cpu %u, freq %u kHz\n", policy->cpu, freq);
>
> mutex_lock(&userspace->mutex);
> - if (!per_cpu(cpu_is_managed, policy->cpu))
> + if (!userspace->is_managed)
> goto err;
>
> userspace->setspeed = freq;
> @@ -85,7 +84,7 @@ static int cpufreq_userspace_policy_start(struct cpufreq_policy *policy)
> pr_debug("started managing cpu %u\n", policy->cpu);
>
> mutex_lock(&userspace->mutex);
> - per_cpu(cpu_is_managed, policy->cpu) = 1;
> + userspace->is_managed = 1;
> userspace->setspeed = policy->cur;
> mutex_unlock(&userspace->mutex);
> return 0;
> @@ -98,7 +97,7 @@ static void cpufreq_userspace_policy_stop(struct cpufreq_policy *policy)
> pr_debug("managing cpu %u stopped\n", policy->cpu);
>
> mutex_lock(&userspace->mutex);
> - per_cpu(cpu_is_managed, policy->cpu) = 0;
> + userspace->is_managed = 0;
> userspace->setspeed = 0;
> mutex_unlock(&userspace->mutex);
> }

Acked-by: Viresh Kumar <[email protected]>

--
viresh