[PATCH 2/2] pseries/dtl: Remove locks held warning.
From: Srikar Dronamraju <hidden>
Date: 2025-11-12 11:11:28
Also in:
lkml
Subsystem:
linux for powerpc (32-bit and 64-bit), the rest · Maintainers:
Madhavan Srinivasan, Michael Ellerman, Linus Torvalds
With CONFIG_LOCKDEP enabled, echo 1 | tee /proc/powerpc/vcpudispatch_stats
will result in the below splat.
WARNING: tee/6324 still has locks held!
6.18.0-rc5 Not tainted
------------------------------------
1 lock held by tee/6324:
#0: c00000000293ddf8 (dtl_access_lock){....}-{3:3}, at: vcpudispatch_stats_write+0x2d8/0x4e8
stack backtrace:
CPU: 24 UID: 0 PID: 6324 Comm: tee Kdump: loaded Not tainted 6.18.0-rc5 #3 VOLUNTARY
Hardware name: IBM,9080-HEU Power11 (architected) 0x820200 0xf000007 of:IBM,FW1110.00 (OK1110_066) hv:phyp pSeries
Call Trace:
dump_stack_lvl+0x138/0x150 (unreliable)
debug_check_no_locks_held+0x130/0x14c
do_exit+0x2e0/0x5dc
do_group_exit+0x4c/0xc0
pid_child_should_wake+0x0/0x84
system_call_exception+0x134/0x340
system_call_vectored_common+0x15c/0x2ec
Currently, rwsem is held from the time vcpudispatch_stats is enabled
till the vcpudispatch_stats is disabled. Enabling and disabling of
vcpudispatch_stats may be done either by the same process or by two
different independent processes. If the enabling process exits before
disabling vcpudispatch_stats, this splat is seen. This lock will
eventually be released by the process disabling the
vcpudispatch_stats.
Currently enabling and disabling vcpudispatch_stats takes mutex lock to
synchronize between multiple tasks that may be trying to act at the same
time. Also a rwsem is used to synchronize between vcpudispatch_stats and
dtl.
With this change, rwsem is used along with the dtl_count to not only
synchronize between multiple vcpudispatch_stats and dtl requests.
Now, instead of holding the rwsem lock for the entire duration, rwsem
gets released after dtl_count gets set to -1.
dtl_count of -1 indicates vcpudispatch_stats is enabled.
dtl_count of 0 indicates not active user of dtl
dtl_count of 1 and above indicates active dtl users.
Signed-off-by: Srikar Dronamraju <redacted>
---
arch/powerpc/include/asm/dtl.h | 1 +
arch/powerpc/platforms/pseries/dtl.c | 13 ++++++++++++-
arch/powerpc/platforms/pseries/lpar.c | 13 +++++++------
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/dtl.h b/arch/powerpc/include/asm/dtl.h
index a5c21bc623cb..78fddfc9314d 100644
--- a/arch/powerpc/include/asm/dtl.h
+++ b/arch/powerpc/include/asm/dtl.h@@ -40,4 +40,5 @@ extern struct rw_semaphore dtl_access_lock; extern void register_dtl_buffer(int cpu); extern void alloc_dtl_buffers(unsigned long *time_limit); +extern atomic_t dtl_count; #endif /* _ASM_POWERPC_DTL_H */
diff --git a/arch/powerpc/platforms/pseries/dtl.c b/arch/powerpc/platforms/pseries/dtl.c
index 6c95781cafb7..e4b7b55ddadc 100644
--- a/arch/powerpc/platforms/pseries/dtl.c
+++ b/arch/powerpc/platforms/pseries/dtl.c@@ -36,8 +36,14 @@ static u8 dtl_event_mask = DTL_LOG_ALL; * not cross a 4k boundary. */ static int dtl_buf_entries = N_DISPATCH_LOG; -static atomic_t dtl_count; +/* + * dtl_count indicates the number and type of dtl users. + * 0 indicates no active users of dtl / vcpu_dispatchstats. + * -1 indicates vcpudispatch_stats user is active. + * 1 and above indicates active dtl users. + */ +atomic_t dtl_count; #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
@@ -195,6 +201,11 @@ static int dtl_enable(struct dtl *dtl) if (!down_read_trylock(&dtl_access_lock)) return -EBUSY; + if (atomic_read(&dtl_count) == -1) { + up_read(&dtl_access_lock); + return -EBUSY; + } + n_entries = dtl_buf_entries; buf = kmem_cache_alloc_node(dtl_cache, GFP_KERNEL, cpu_to_node(dtl->cpu)); if (!buf) {
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 6a415febc53b..b210beefe67b 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c@@ -175,7 +175,6 @@ static DEFINE_PER_CPU(struct vcpu_dispatch_data, vcpu_disp_data); static DEFINE_PER_CPU(u64, dtl_entry_ridx); static DEFINE_PER_CPU(struct dtl_worker, dtl_workers); static enum cpuhp_state dtl_worker_state; -static DEFINE_MUTEX(dtl_enable_mutex); static int vcpudispatch_stats_on __read_mostly; static int vcpudispatch_stats_freq = 50; static __be32 *vcpu_associativity, *pcpu_associativity;
@@ -464,7 +463,8 @@ static int dtl_worker_enable(unsigned long *time_limit) { int rc = 0, state; - if (!down_write_trylock(&dtl_access_lock)) { + /* Return if dtl is already active */ + if (atomic_read(&dtl_count) != 0) { rc = -EBUSY; goto out; }
@@ -480,11 +480,11 @@ static int dtl_worker_enable(unsigned long *time_limit) pr_err("vcpudispatch_stats: unable to setup workqueue for DTL processing\n"); free_dtl_buffers(time_limit); reset_global_dtl_mask(); - up_write(&dtl_access_lock); rc = -EINVAL; goto out; } dtl_worker_state = state; + atomic_set(&dtl_count, -1); out: return rc;
@@ -495,7 +495,7 @@ static void dtl_worker_disable(unsigned long *time_limit) cpuhp_remove_state(dtl_worker_state); free_dtl_buffers(time_limit); reset_global_dtl_mask(); - up_write(&dtl_access_lock); + atomic_set(&dtl_count, 0); } static ssize_t vcpudispatch_stats_write(struct file *file, const char __user *p,
@@ -519,7 +519,8 @@ static ssize_t vcpudispatch_stats_write(struct file *file, const char __user *p, return rc ? rc : -EINVAL; } - mutex_lock(&dtl_enable_mutex); + if (!down_write_trylock(&dtl_access_lock)) + return -EBUSY; if ((cmd == 0 && !vcpudispatch_stats_on) || (cmd == 1 && vcpudispatch_stats_on))
@@ -551,7 +552,7 @@ static ssize_t vcpudispatch_stats_write(struct file *file, const char __user *p, vcpudispatch_stats_on = cmd; out: - mutex_unlock(&dtl_enable_mutex); + up_write(&dtl_access_lock); if (rc) return rc; return count;
--
2.43.7