diff mbox series

[V3] cpufreq: Warn users while freeing active policy

Message ID 0af6e95f8da395f75c5a58363d8b1b40483252c8.1657187391.git.viresh.kumar@linaro.org
State Accepted
Commit a2f6a7ac60e234810a386aaa6aad106927c39c4f
Headers show
Series [V3] cpufreq: Warn users while freeing active policy | expand

Commit Message

Viresh Kumar July 7, 2022, 9:58 a.m. UTC
With the new design in place, the show() and store() callbacks check if
the policy is active or not before proceeding any further to avoid
potential races. And in order to guarantee that cpufreq_policy_free()
must be called after clearing the policy->cpus mask, i.e. by marking the
policy inactive.

In order to avoid introducing a bug around this later, print a warning
message if we end up freeing an active policy.

Also update cpufreq_online() a bit to make sure we clear the cpus mask
for each error case before calling cpufreq_policy_free().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V3: BUG_ON() -> pr_warn()

 drivers/cpufreq/cpufreq.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki July 15, 2022, 5:20 p.m. UTC | #1
On Thu, Jul 7, 2022 at 11:58 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> With the new design in place, the show() and store() callbacks check if
> the policy is active or not before proceeding any further to avoid
> potential races. And in order to guarantee that cpufreq_policy_free()
> must be called after clearing the policy->cpus mask, i.e. by marking the
> policy inactive.
>
> In order to avoid introducing a bug around this later, print a warning
> message if we end up freeing an active policy.
>
> Also update cpufreq_online() a bit to make sure we clear the cpus mask
> for each error case before calling cpufreq_policy_free().
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V3: BUG_ON() -> pr_warn()
>
>  drivers/cpufreq/cpufreq.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 73432360e6e9..954eef26685f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1273,6 +1273,13 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>         unsigned long flags;
>         int cpu;
>
> +       /*
> +        * The callers must ensure the policy is inactive by now, to avoid any
> +        * races with show()/store() callbacks.
> +        */
> +       if (unlikely(!policy_is_inactive(policy)))
> +               pr_warn("%s: Freeing active policy\n", __func__);
> +
>         /* Remove policy from list */
>         write_lock_irqsave(&cpufreq_driver_lock, flags);
>         list_del(&policy->policy_list);
> @@ -1527,8 +1534,6 @@ static int cpufreq_online(unsigned int cpu)
>         for_each_cpu(j, policy->real_cpus)
>                 remove_cpu_dev_symlink(policy, j, get_cpu_device(j));
>
> -       cpumask_clear(policy->cpus);
> -
>  out_offline_policy:
>         if (cpufreq_driver->offline)
>                 cpufreq_driver->offline(policy);
> @@ -1538,6 +1543,7 @@ static int cpufreq_online(unsigned int cpu)
>                 cpufreq_driver->exit(policy);
>
>  out_free_policy:
> +       cpumask_clear(policy->cpus);
>         up_write(&policy->rwsem);
>
>         cpufreq_policy_free(policy);
> --

Applied as 5.20 material, thanks!
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 73432360e6e9..954eef26685f 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1273,6 +1273,13 @@  static void cpufreq_policy_free(struct cpufreq_policy *policy)
 	unsigned long flags;
 	int cpu;
 
+	/*
+	 * The callers must ensure the policy is inactive by now, to avoid any
+	 * races with show()/store() callbacks.
+	 */
+	if (unlikely(!policy_is_inactive(policy)))
+		pr_warn("%s: Freeing active policy\n", __func__);
+
 	/* Remove policy from list */
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
 	list_del(&policy->policy_list);
@@ -1527,8 +1534,6 @@  static int cpufreq_online(unsigned int cpu)
 	for_each_cpu(j, policy->real_cpus)
 		remove_cpu_dev_symlink(policy, j, get_cpu_device(j));
 
-	cpumask_clear(policy->cpus);
-
 out_offline_policy:
 	if (cpufreq_driver->offline)
 		cpufreq_driver->offline(policy);
@@ -1538,6 +1543,7 @@  static int cpufreq_online(unsigned int cpu)
 		cpufreq_driver->exit(policy);
 
 out_free_policy:
+	cpumask_clear(policy->cpus);
 	up_write(&policy->rwsem);
 
 	cpufreq_policy_free(policy);