diff mbox series

[2/2] pseries/dtl: Remove locks held warning.

Message ID 20251112111051.826922-2-srikar@linux.ibm.com (mailing list archive)
State New
Headers show
Series [1/2] pseries/dtl: Use counter in !native case too | expand

Commit Message

Srikar Dronamraju Nov. 12, 2025, 11:10 a.m. UTC
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(-)

Comments

kernel test robot Nov. 13, 2025, 4:37 p.m. UTC | #1
Hi Srikar,

kernel test robot noticed the following build errors:

[auto build test ERROR on powerpc/next]
[also build test ERROR on powerpc/fixes linus/master v6.18-rc5 next-20251113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Srikar-Dronamraju/pseries-dtl-Remove-locks-held-warning/20251112-191217
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
patch link:    https://lore.kernel.org/r/20251112111051.826922-2-srikar%40linux.ibm.com
patch subject: [PATCH 2/2] pseries/dtl: Remove locks held warning.
config: powerpc64-randconfig-001-20251113 (https://download.01.org/0day-ci/archive/20251114/202511140025.2eZn4KhO-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 0bba1e76581bad04e7d7f09f5115ae5e2989e0d9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251114/202511140025.2eZn4KhO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511140025.2eZn4KhO-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: dtl_count
   >>> referenced by lpar.c
   >>>               arch/powerpc/platforms/pseries/lpar.o:(.toc+0x88) in archive vmlinux.a
diff mbox series

Patch

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;