* [PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost()
@ 2025-05-13 1:57 Seyediman Seyedarab
2025-05-14 12:43 ` Rafael J. Wysocki
0 siblings, 1 reply; 2+ messages in thread
From: Seyediman Seyedarab @ 2025-05-13 1:57 UTC (permalink / raw)
To: rafael, viresh.kumar
Cc: linux-pm, linux-kernel, skhan, linux-kernel-mentees,
Seyediman Seyedarab
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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost()
2025-05-13 1:57 [PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost() Seyediman Seyedarab
@ 2025-05-14 12:43 ` Rafael J. Wysocki
0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2025-05-14 12:43 UTC (permalink / raw)
To: Seyediman Seyedarab
Cc: rafael, viresh.kumar, linux-pm, linux-kernel, skhan,
linux-kernel-mentees
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!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-05-14 12:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-13 1:57 [PATCH] cpufreq: drop redundant cpus_read_lock() from store_local_boost() Seyediman Seyedarab
2025-05-14 12:43 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox