diff mbox series

[3/3] cpufreq: Drop unnecessary cpus locking from store()

Message ID 0a6620ff5a330126b21d00244d4fad62cff4f230.1653565641.git.viresh.kumar@linaro.org
State Accepted
Commit 9ab9b9d3fb9231cdcfda8e0fb3d9c24a2f95ed26
Headers show
Series cpufreq: Minor cleanups | expand

Commit Message

Viresh Kumar May 26, 2022, 11:51 a.m. UTC
This change was introduced long back by:

commit 4f750c930822 ("cpufreq: Synchronize the cpufreq store_*() routines with CPU hotplug")

Since then, both cpufreq and hotplug core have been reworked and have
much better locking in place. The race mentioned in commit 4f750c930822
isn't possible anymore.

Drop the unnecessary locking.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

Comments

Rafael J. Wysocki June 14, 2022, 1:51 p.m. UTC | #1
On Thu, May 26, 2022 at 1:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This change was introduced long back by:
>
> commit 4f750c930822 ("cpufreq: Synchronize the cpufreq store_*() routines with CPU hotplug")
>
> Since then, both cpufreq and hotplug core have been reworked and have
> much better locking in place. The race mentioned in commit 4f750c930822
> isn't possible anymore.
>
> Drop the unnecessary locking.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 53d163a84e06..bb237d1ce5e7 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -973,21 +973,10 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
>         if (!fattr->store)
>                 return -EIO;
>
> -       /*
> -        * cpus_read_trylock() is used here to work around a circular lock
> -        * dependency problem with respect to the cpufreq_register_driver().
> -        */
> -       if (!cpus_read_trylock())
> -               return -EBUSY;
> -
> -       if (cpu_online(policy->cpu)) {
> -               down_write(&policy->rwsem);
> -               if (likely(!policy_is_inactive(policy)))
> -                       ret = fattr->store(policy, buf, count);
> -               up_write(&policy->rwsem);
> -       }
> -
> -       cpus_read_unlock();
> +       down_write(&policy->rwsem);
> +       if (likely(!policy_is_inactive(policy)))
> +               ret = fattr->store(policy, buf, count);
> +       up_write(&policy->rwsem);
>
>         return ret;
>  }
> --

Applied along with the [1/3] as 5.20 material, thanks!
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 53d163a84e06..bb237d1ce5e7 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -973,21 +973,10 @@  static ssize_t store(struct kobject *kobj, struct attribute *attr,
 	if (!fattr->store)
 		return -EIO;
 
-	/*
-	 * cpus_read_trylock() is used here to work around a circular lock
-	 * dependency problem with respect to the cpufreq_register_driver().
-	 */
-	if (!cpus_read_trylock())
-		return -EBUSY;
-
-	if (cpu_online(policy->cpu)) {
-		down_write(&policy->rwsem);
-		if (likely(!policy_is_inactive(policy)))
-			ret = fattr->store(policy, buf, count);
-		up_write(&policy->rwsem);
-	}
-
-	cpus_read_unlock();
+	down_write(&policy->rwsem);
+	if (likely(!policy_is_inactive(policy)))
+		ret = fattr->store(policy, buf, count);
+	up_write(&policy->rwsem);
 
 	return ret;
 }