1. Patchew
  2. linux
  3. View series

[PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost()

Seyediman Seyedarab posted 1 patch 9 months ago 
drivers/cpufreq/cpufreq.c | 3 ---
1 file changed, 3 deletions(-)
[PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost()
Posted by Seyediman Seyedarab 9 months ago 
Lockdep reports a possible circular locking dependency[1] when
cpu_hotplug_lock is acquired inside store_local_boost(), after
policy->rwsem has already been taken by store().

However, the boost update is strictly per-policy and does not
access shared state or iterate over all policies. Since policy->rwsem
is already held, this is enough to serialize against concurrent
topology changes for the current policy.

Remove the cpus_read_lock() to resolve the lockdep warning and
avoid unnecessary locking.

 [1]
 ======================================================
 WARNING: possible circular locking dependency detected
 6.15.0-rc6-debug-gb01fc4eca73c #1 Not tainted
 ------------------------------------------------------
 power-profiles-/588 is trying to acquire lock:
 ffffffffb3a7d910 (cpu_hotplug_lock){++++}-{0:0}, at: store_local_boost+0x56/0xd0

 but task is already holding lock:
 ffff8b6e5a12c380 (&policy->rwsem){++++}-{4:4}, at: store+0x37/0x90

 which lock already depends on the new lock.

 the existing dependency chain (in reverse order) is:

 -> #2 (&policy->rwsem){++++}-{4:4}:
        down_write+0x29/0xb0
        cpufreq_online+0x7e8/0xa40
        cpufreq_add_dev+0x82/0xa0
        subsys_interface_register+0x148/0x160
        cpufreq_register_driver+0x15d/0x260
        amd_pstate_register_driver+0x36/0x90
        amd_pstate_init+0x1e7/0x270
        do_one_initcall+0x68/0x2b0
        kernel_init_freeable+0x231/0x270
        kernel_init+0x15/0x130
        ret_from_fork+0x2c/0x50
        ret_from_fork_asm+0x11/0x20

 -> #1 (subsys mutex#3){+.+.}-{4:4}:
        __mutex_lock+0xc2/0x930
        subsys_interface_register+0x7f/0x160
        cpufreq_register_driver+0x15d/0x260
        amd_pstate_register_driver+0x36/0x90
        amd_pstate_init+0x1e7/0x270
        do_one_initcall+0x68/0x2b0
        kernel_init_freeable+0x231/0x270
        kernel_init+0x15/0x130
        ret_from_fork+0x2c/0x50
        ret_from_fork_asm+0x11/0x20

 -> #0 (cpu_hotplug_lock){++++}-{0:0}:
        __lock_acquire+0x10ed/0x1850
        lock_acquire.part.0+0x69/0x1b0
        cpus_read_lock+0x2a/0xc0
        store_local_boost+0x56/0xd0
        store+0x50/0x90
        kernfs_fop_write_iter+0x132/0x200
        vfs_write+0x2b3/0x590
        ksys_write+0x74/0xf0
        do_syscall_64+0xbb/0x1d0
        entry_SYSCALL_64_after_hwframe+0x56/0x5e

Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.com>
---
 drivers/cpufreq/cpufreq.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 731ecfc17..759dd178a 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -622,10 +622,7 @@ static ssize_t store_local_boost(struct cpufreq_policy *policy,
 	if (!policy->boost_supported)
 		return -EINVAL;
 
-	cpus_read_lock();
 	ret = policy_set_boost(policy, enable);
-	cpus_read_unlock();
-
 	if (!ret)
 		return count;
 
-- 
2.49.0
Re: [PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost()
Posted by Rafael J. Wysocki 8 months, 4 weeks ago 
On Tue, May 13, 2025 at 3:55 AM Seyediman Seyedarab <imandevel@gmail.com> wrote:
>
> Lockdep reports a possible circular locking dependency[1] when
> cpu_hotplug_lock is acquired inside store_local_boost(), after
> policy->rwsem has already been taken by store().
>
> However, the boost update is strictly per-policy and does not
> access shared state or iterate over all policies. Since policy->rwsem
> is already held, this is enough to serialize against concurrent
> topology changes for the current policy.
>
> Remove the cpus_read_lock() to resolve the lockdep warning and
> avoid unnecessary locking.
>
>  [1]
>  ======================================================
>  WARNING: possible circular locking dependency detected
>  6.15.0-rc6-debug-gb01fc4eca73c #1 Not tainted
>  ------------------------------------------------------
>  power-profiles-/588 is trying to acquire lock:
>  ffffffffb3a7d910 (cpu_hotplug_lock){++++}-{0:0}, at: store_local_boost+0x56/0xd0
>
>  but task is already holding lock:
>  ffff8b6e5a12c380 (&policy->rwsem){++++}-{4:4}, at: store+0x37/0x90
>
>  which lock already depends on the new lock.
>
>  the existing dependency chain (in reverse order) is:
>
>  -> #2 (&policy->rwsem){++++}-{4:4}:
>         down_write+0x29/0xb0
>         cpufreq_online+0x7e8/0xa40
>         cpufreq_add_dev+0x82/0xa0
>         subsys_interface_register+0x148/0x160
>         cpufreq_register_driver+0x15d/0x260
>         amd_pstate_register_driver+0x36/0x90
>         amd_pstate_init+0x1e7/0x270
>         do_one_initcall+0x68/0x2b0
>         kernel_init_freeable+0x231/0x270
>         kernel_init+0x15/0x130
>         ret_from_fork+0x2c/0x50
>         ret_from_fork_asm+0x11/0x20
>
>  -> #1 (subsys mutex#3){+.+.}-{4:4}:
>         __mutex_lock+0xc2/0x930
>         subsys_interface_register+0x7f/0x160
>         cpufreq_register_driver+0x15d/0x260
>         amd_pstate_register_driver+0x36/0x90
>         amd_pstate_init+0x1e7/0x270
>         do_one_initcall+0x68/0x2b0
>         kernel_init_freeable+0x231/0x270
>         kernel_init+0x15/0x130
>         ret_from_fork+0x2c/0x50
>         ret_from_fork_asm+0x11/0x20
>
>  -> #0 (cpu_hotplug_lock){++++}-{0:0}:
>         __lock_acquire+0x10ed/0x1850
>         lock_acquire.part.0+0x69/0x1b0
>         cpus_read_lock+0x2a/0xc0
>         store_local_boost+0x56/0xd0
>         store+0x50/0x90
>         kernfs_fop_write_iter+0x132/0x200
>         vfs_write+0x2b3/0x590
>         ksys_write+0x74/0xf0
>         do_syscall_64+0xbb/0x1d0
>         entry_SYSCALL_64_after_hwframe+0x56/0x5e
>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.com>
> ---
>  drivers/cpufreq/cpufreq.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 731ecfc17..759dd178a 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -622,10 +622,7 @@ static ssize_t store_local_boost(struct cpufreq_policy *policy,
>         if (!policy->boost_supported)
>                 return -EINVAL;
>
> -       cpus_read_lock();
>         ret = policy_set_boost(policy, enable);
> -       cpus_read_unlock();
> -
>         if (!ret)
>                 return count;
>
> --

Applied as 6.16 material, thanks!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.