* [PATCH 2/2] pseries/dtl: Remove locks held warning.
2025-11-12 11:10 [PATCH 1/2] pseries/dtl: Use counter in !native case too Srikar Dronamraju
@ 2025-11-12 11:10 ` Srikar Dronamraju
2025-11-13 16:37 ` kernel test robot
0 siblings, 1 reply; 3+ messages in thread
From: Srikar Dronamraju @ 2025-11-12 11:10 UTC (permalink / raw)
To: linuxppc-dev
Cc: Christophe Leroy, linux-kernel, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Srikar Dronamraju
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 <srikar@linux.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread