[PATCH 02/32] locking/lockdep: lock_class_is_held()

Kent Overstreet posted 32 patches 2 years, 7 months ago
[PATCH 02/32] locking/lockdep: lock_class_is_held()
Posted by Kent Overstreet 2 years, 7 months ago
From: Kent Overstreet <kent.overstreet@gmail.com>

This patch adds lock_class_is_held(), which can be used to assert that a
particular type of lock is not held.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
---
 include/linux/lockdep.h  |  4 ++++
 kernel/locking/lockdep.c | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 1023f349af..e858c288c7 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -339,6 +339,8 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
 #define lockdep_repin_lock(l,c)	lock_repin_lock(&(l)->dep_map, (c))
 #define lockdep_unpin_lock(l,c)	lock_unpin_lock(&(l)->dep_map, (c))
 
+int lock_class_is_held(struct lock_class_key *key);
+
 #else /* !CONFIG_LOCKDEP */
 
 static inline void lockdep_init_task(struct task_struct *task)
@@ -427,6 +429,8 @@ extern int lockdep_is_held(const void *);
 #define lockdep_repin_lock(l, c)		do { (void)(l); (void)(c); } while (0)
 #define lockdep_unpin_lock(l, c)		do { (void)(l); (void)(c); } while (0)
 
+static inline int lock_class_is_held(struct lock_class_key *key) { return 0; }
+
 #endif /* !LOCKDEP */
 
 enum xhlock_context_t {
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 50d4863974..e631464070 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -6487,6 +6487,26 @@ void debug_check_no_locks_held(void)
 }
 EXPORT_SYMBOL_GPL(debug_check_no_locks_held);
 
+#ifdef CONFIG_LOCKDEP
+int lock_class_is_held(struct lock_class_key *key)
+{
+	struct task_struct *curr = current;
+	struct held_lock *hlock;
+
+	if (unlikely(!debug_locks))
+		return 0;
+
+	for (hlock = curr->held_locks;
+	     hlock < curr->held_locks + curr->lockdep_depth;
+	     hlock++)
+		if (hlock->instance->key == key)
+			return 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lock_class_is_held);
+#endif
+
 #ifdef __KERNEL__
 void debug_show_all_locks(void)
 {
-- 
2.40.1
Re: [PATCH 02/32] locking/lockdep: lock_class_is_held()
Posted by Peter Zijlstra 2 years, 7 months ago
On Tue, May 09, 2023 at 12:56:27PM -0400, Kent Overstreet wrote:
> From: Kent Overstreet <kent.overstreet@gmail.com>
> 
> This patch adds lock_class_is_held(), which can be used to assert that a
> particular type of lock is not held.

How is lock_is_held_type() not sufficient? Which is what's used to
implement lockdep_assert_held*().
Re: [PATCH 02/32] locking/lockdep: lock_class_is_held()
Posted by Kent Overstreet 2 years, 7 months ago
On Tue, May 09, 2023 at 09:30:39PM +0200, Peter Zijlstra wrote:
> On Tue, May 09, 2023 at 12:56:27PM -0400, Kent Overstreet wrote:
> > From: Kent Overstreet <kent.overstreet@gmail.com>
> > 
> > This patch adds lock_class_is_held(), which can be used to assert that a
> > particular type of lock is not held.
> 
> How is lock_is_held_type() not sufficient? Which is what's used to
> implement lockdep_assert_held*().

I should've looked at that before - it returns a tristate, so it's
closer than I thought, but this is used in contexts where we don't have
a lock or lockdep_map to pass and need to pass the lock_class_key
instead.

e.g, when initializing a btree_trans, or waiting on btree node IO, we
need to assert that no btree node locks are held.

Looking at the code, __lock_is_held() -> match_held_lock() has to care
about a bunch of stuff related to subclasses that doesn't seem relevant
to lock_class_is_held() - lock_class_is_held() is practically no code in
comparison, so I'm inclined to think they should just be separate.

But I'm not the lockdep expert :) Thoughts?