[PATCH 12/14] lockdep: Stop lockdep_assert_held*() from assuming lock ownership

[Bart Van Assche]

Since commit 7c451541743c ("lockdep: Annotate lockdep assertions for
context analysis"), lockdep_assert_held(), lockdep_assert_held_write()
and lockdep_assert_held_read() instruct Clang's context analysis to
assume that a lock is held. This has the following disadvantages:
- lockdep_assert_held*() silences context analysis warnings in functions
  that lack a __must_hold() or __must_hold_shared() annotation. Hence,
  the compiler neither reports the missing function annotation nor
  checks at compile time whether callers hold the lock.
- lockdep_assert_held() makes context analysis assume exclusive lock
  ownership, even if a reader-writer lock is held in shared mode.
- lockdep_assert_held*() can only be used with lock types that are
  defined with context_lock_struct().

Hence, remove the context assumptions from lockdep_assert_held*() for
both CONFIG_LOCKDEP and !CONFIG_LOCKDEP builds. Code that needs to
inform context analysis that a lock is held should use
lockdep_assume_ctx_held() or lockdep_assume_shared_ctx_held() instead.
Mention this in the kernel-doc comments of these macros.

Convert the __assume_ctx_lock() calls in kernel/locking/rtmutex.c into
lockdep_assume_ctx_held() calls such that the lock ownership assumptions
are verified at runtime. Remove the lockdep_assert_held(&lock->wait_lock)
call from __rt_mutex_slowlock() because lockdep_assume_ctx_held() now
performs the same runtime check.

Update Documentation/dev-tools/context-analysis.rst accordingly.

Cc: Marco Elver <elver@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
 Documentation/dev-tools/context-analysis.rst | 18 ++++++++++++------
 include/linux/lockdep.h                      | 18 ++++++++++++------
 kernel/locking/rtmutex.c                     |  7 +++----
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/Documentation/dev-tools/context-analysis.rst b/Documentation/dev-tools/context-analysis.rst
index 8e71e1e75b5b..88b4ed6a182f 100644
--- a/Documentation/dev-tools/context-analysis.rst
+++ b/Documentation/dev-tools/context-analysis.rst
@@ -111,12 +111,18 @@ checking): either with a ``context_unsafe(var = init)`` expression, or by
 marking small initialization functions with the ``__context_unsafe(init)``
 attribute.
 
-Lockdep assertions, such as `lockdep_assert_held()`, inform the compiler's
-context analysis that the associated synchronization primitive is held after
-the assertion. This avoids false positives in complex control-flow scenarios
-and encourages the use of Lockdep where static analysis is limited. For
-example, this is useful when a function doesn't *always* require a lock, making
-`__must_hold()` inappropriate.
+`lockdep_assume_ctx_held()` and `lockdep_assume_shared_ctx_held()` inform the
+compiler's context analysis that the associated synchronization primitive is
+held and verify this at runtime via Lockdep. This avoids false positives in
+complex control-flow scenarios while still checking lock ownership with Lockdep
+where static analysis is limited. For example, this is useful when a function
+doesn't *always* require a lock, making `__must_hold()` inappropriate.
+
+Lockdep assertions, such as `lockdep_assert_held()`, only perform a runtime
+check and do not inform the compiler's context analysis that a lock is held.
+For context locks that are not tracked by Lockdep, e.g. bit spinlocks, use
+`__assume_ctx_lock()` or `__assume_shared_ctx_lock()` instead. These only
+inform the compiler's context analysis and do not perform a runtime check.
 
 Keywords
 ~~~~~~~~
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index f774b22a35c5..ac2f1d2997a1 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -285,16 +285,16 @@ extern u32 lock_sequence(struct lockdep_map *lock);
 	do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0)
 
 #define lockdep_assert_held(l)		\
-	do { lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD); __assume_ctx_lock(l); } while (0)
+	lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
 
 #define lockdep_assert_not_held(l)	\
 	lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD)
 
 #define lockdep_assert_held_write(l)	\
-	do { lockdep_assert(lockdep_is_held_type(l, 0)); __assume_ctx_lock(l); } while (0)
+	lockdep_assert(lockdep_is_held_type(l, 0))
 
 #define lockdep_assert_held_read(l)	\
-	do { lockdep_assert(lockdep_is_held_type(l, 1)); __assume_shared_ctx_lock(l); } while (0)
+	lockdep_assert(lockdep_is_held_type(l, 1))
 
 #define lockdep_assert_held_once(l)		\
 	lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
@@ -392,10 +392,10 @@ extern int lockdep_is_held(const void *);
 #define lockdep_assert(c)			do { } while (0)
 #define lockdep_assert_once(c)			do { } while (0)
 
-#define lockdep_assert_held(l)			__assume_ctx_lock(l)
+#define lockdep_assert_held(l)			do { (void)(l); } while (0)
 #define lockdep_assert_not_held(l)		do { (void)(l); } while (0)
-#define lockdep_assert_held_write(l)		__assume_ctx_lock(l)
-#define lockdep_assert_held_read(l)		__assume_shared_ctx_lock(l)
+#define lockdep_assert_held_write(l)		do { (void)(l); } while (0)
+#define lockdep_assert_held_read(l)		do { (void)(l); } while (0)
 #define lockdep_assert_held_once(l)		do { (void)(l); } while (0)
 #define lockdep_assert_none_held_once()	do { } while (0)
 
@@ -436,6 +436,9 @@ extern int lockdep_is_held(const void *);
  * has a dep_map member. For context locks that are not tracked by lockdep,
  * e.g. bit spinlocks and the RCU context tokens, use
  * __assume_ctx_lock() instead.
+ *
+ * Note: lockdep_assert_held() does not inform the compiler's context
+ * analysis that a lock is held. Use this macro if that is needed.
  */
 #define lockdep_assume_ctx_held(var)					\
 	_lockdep_assume_ctx_held(var, __UNIQUE_ID(ctx_lock))
@@ -467,6 +470,9 @@ extern int lockdep_is_held(const void *);
  * @var must point at a lock object that is tracked by lockdep, i.e. that
  * has a dep_map member. For context locks that are not tracked by lockdep,
  * e.g. the RCU context tokens, use __assume_shared_ctx_lock() instead.
+ *
+ * Note: lockdep_assert_held_read() does not inform the compiler's context
+ * analysis that a lock is held. Use this macro if that is needed.
  */
 #define lockdep_assume_shared_ctx_held(var)				\
 	_lockdep_assume_shared_ctx_held(var, __UNIQUE_ID(ctx_lock))
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 5a9534c715b8..424cd38a72d2 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1259,7 +1259,7 @@ static int __sched task_blocks_on_rt_mutex(struct rt_mutex_base *lock,
 
 		/* Check whether the waiter should back out immediately */
 		rtm = container_of(lock, struct rt_mutex, rtmutex);
-		__assume_ctx_lock(&rtm->rtmutex.wait_lock);
+		lockdep_assume_ctx_held(&rtm->rtmutex.wait_lock);
 		res = __ww_mutex_add_waiter(waiter, rtm, ww_ctx, wake_q);
 		if (res) {
 			raw_spin_lock(&task->pi_lock);
@@ -1634,7 +1634,7 @@ static int __sched rt_mutex_slowlock_block(struct rt_mutex_base *lock,
 	struct task_struct *owner;
 	int ret = 0;
 
-	__assume_ctx_lock(&rtm->rtmutex.wait_lock);
+	lockdep_assume_ctx_held(&rtm->rtmutex.wait_lock);
 
 	lockevent_inc(rtmutex_slow_block);
 	for (;;) {
@@ -1724,8 +1724,7 @@ static int __sched __rt_mutex_slowlock(struct rt_mutex_base *lock,
 	struct ww_mutex *ww = ww_container_of(rtm);
 	int ret;
 
-	__assume_ctx_lock(&rtm->rtmutex.wait_lock);
-	lockdep_assert_held(&lock->wait_lock);
+	lockdep_assume_ctx_held(&rtm->rtmutex.wait_lock);
 	lockevent_inc(rtmutex_slowlock);
 
 	/* Try to acquire the lock again: */