Skip to contentSkip to conversation

locking: contended_release tracepoint instrumentation

PATCH Superseded Dmitry Ilvokhin 5 participants 20 messages Mar 26 – Apr 15

Subsystems (9)
Memory management x86 MIPS Tracing +5 more subsystems

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v4 cover locking: contended_release tracepoint instrumentation

The existing contention_begin/contention_end tracepoints fire on the waiter side. The lock holder's identity and stack can be captured at contention_begin time (e.g. perf lock contention --lock-owner), but this reflects the holder's state when a waiter arrives, not when the lock is actually released.

This series adds a contended_release tracepoint that fires on the holder side when a lock with waiters is released. This provides:

- Hold time estimation: when the holder's own acquisition was contended, its contention_end (acquisition) and contended_release can be correlated to measure how long the lock was held under contention.

- The holder's stack at release time, which may differ from what perf lock contention --lock-owner captures if the holder does significant work between the waiter's arrival and the unlock.

Note: for reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

v3 -> v4:

- Fix spurious events in __percpu_up_read(): guard with rcuwait_active(&sem->writer) to avoid tracing during the RCU grace period after a writer releases (Sashiko).
- Fix possible use-after-free in semaphore up(): move trace_contended_release() inside the sem->lock critical section (Sashiko).
- Fix build failure with CONFIG_PARAVIRT_SPINLOCKS=y: introduce queued_spin_release() as the arch-overridable unlock primitive, so queued_spin_unlock() can be a generic tracing wrapper. Convert x86 (paravirt) and MIPS overrides (Sashiko).
- Add EXPORT_TRACEPOINT_SYMBOL_GPL(contended_release) for module support (Sashiko).
- Split spinning locks patch: factor out queued_spin_release() as a separate preparatory commit (Sashiko).
- Make read unlock tracepoint behavior consistent across all reader/writer lock types: fire for every reader releasing while a writer is waiting (rwsem, rwbase_rt were previously last-reader only).

v2 -> v3:

- Added new patch: extend contended_release tracepoint to queued spinlocks and queued rwlocks (marked as RFC, requesting feedback). This is prompted by Matthew Wilcox's suggestion to try to come up with generic instrumentation, instead of instrumenting each "special" lock manually. See [1] for the discussion.
- Reworked tracepoint placement to fire before the lock is released and before the waiter is woken where possible, for consistency with spinning locks where there is no explicit wake (inspired by Usama Arif's suggestion).
- Remove unnecessary linux/sched.h include from trace/events/lock.h.

RFC -> v2:

- Add trace_contended_release_enabled() guard before waiter checks that exist only for the tracepoint (Steven Rostedt).
- Rename __percpu_up_read_slowpath() to __percpu_up_read() (Peter Zijlstra).
- Add extern for __percpu_up_read() (Peter Zijlstra).
- Squashed tracepoint introduction and usage commits (Masami Hiramatsu).

v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
RFC: https://lore.kernel.org/all/[email protected]/

[1]: https://lore.kernel.org/all/[email protected]/

Dmitry Ilvokhin (5):
tracing/lock: Remove unnecessary linux/sched.h include locking/percpu-rwsem: Extract __percpu_up_read()
locking: Add contended_release tracepoint to sleepable locks
locking: Factor out queued_spin_release()
locking: Add contended_release tracepoint to spinning locks

 arch/mips/include/asm/spinlock.h         |  6 +--
 arch/x86/include/asm/paravirt-spinlock.h |  6 +--
 include/asm-generic/qrwlock.h            | 48 ++++++++++++++++++++----
 include/asm-generic/qspinlock.h          | 33 ++++++++++++++--
 include/linux/percpu-rwsem.h             | 15 ++------
 include/trace/events/lock.h              | 18 ++++++++-
 kernel/locking/mutex.c                   |  4 ++
 kernel/locking/percpu-rwsem.c            | 29 ++++++++++++++
 kernel/locking/qrwlock.c                 | 16 ++++++++
 kernel/locking/qspinlock.c               |  8 ++++
 kernel/locking/rtmutex.c                 |  1 +
 kernel/locking/rwbase_rt.c               |  6 +++
 kernel/locking/rwsem.c                   | 10 ++++-
 kernel/locking/semaphore.c               |  4 ++
 14 files changed, 172 insertions(+), 32 deletions(-)
Signature · 1 line
2.52.0
#1 lore raw

PATCH v4 1/5 tracing/lock: Remove unnecessary linux/sched.h include

None of the trace events in lock.h reference anything from linux/sched.h. Remove the unnecessary include.

---
 include/trace/events/lock.h | 1 -
 1 file changed, 1 deletion(-)
include/trace/events/lock.h +0 -1
index 8e89baa3775f..da978f2afb45 100644
@@ -5,7 +5,6 @@
 #if !defined(_TRACE_LOCK_H) || defined(TRACE_HEADER_MULTI_READ)
 #define _TRACE_LOCK_H
 
-#include <linux/sched.h>
 #include <linux/tracepoint.h>
 
 /* flags for lock:contention_begin */
Signature · 1 line
2.52.0
#3 lore raw

PATCH v4 2/5 locking/percpu-rwsem: Extract __percpu_up_read()

In reply to Dmitry Ilvokhin

Move the percpu_up_read() slowpath out of the inline function into a new __percpu_up_read() to avoid binary size increase from adding a tracepoint to an inlined function.

---
 include/linux/percpu-rwsem.h  | 15 +++------------
 kernel/locking/percpu-rwsem.c | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 12 deletions(-)
include/linux/percpu-rwsem.h +3 -12
index c8cb010d655e..39d5bf8e6562 100644
@@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 	return ret;
 }
 
+extern void __percpu_up_read(struct percpu_rw_semaphore *sem);
+
 static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);
@@ -118,18 +120,7 @@ static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 	if (likely(rcu_sync_is_idle(&sem->rss))) {
 		this_cpu_dec(*sem->read_count);
 	} else {
-		/*
-		 * slowpath; reader will only ever wake a single blocked
-		 * writer.
-		 */
-		smp_mb(); /* B matches C */
-		/*
-		 * In other words, if they see our decrement (presumably to
-		 * aggregate zero, as that is the only time it matters) they
-		 * will also see our critical section.
-		 */
-		this_cpu_dec(*sem->read_count);
-		rcuwait_wake_up(&sem->writer);
+		__percpu_up_read(sem);
 	}
 	preempt_enable();
 }
kernel/locking/percpu-rwsem.c +18 -0
index ef234469baac..f3ee7a0d6047 100644
@@ -288,3 +288,21 @@ void percpu_up_write(struct percpu_rw_semaphore *sem)
 	rcu_sync_exit(&sem->rss);
 }
 EXPORT_SYMBOL_GPL(percpu_up_write);
+
+void __percpu_up_read(struct percpu_rw_semaphore *sem)
+{
+	lockdep_assert_preemption_disabled();
+	/*
+	 * slowpath; reader will only ever wake a single blocked
+	 * writer.
+	 */
+	smp_mb(); /* B matches C */
+	/*
+	 * In other words, if they see our decrement (presumably to
+	 * aggregate zero, as that is the only time it matters) they
+	 * will also see our critical section.
+	 */
+	this_cpu_dec(*sem->read_count);
+	rcuwait_wake_up(&sem->writer);
+}
+EXPORT_SYMBOL_GPL(__percpu_up_read);
Signature · 1 line
2.52.0
#4 lore raw

PATCH v4 3/5 locking: Add contended_release tracepoint to sleepable locks

In reply to Dmitry Ilvokhin

Add the contended_release trace event. This tracepoint fires on the holder side when a contended lock is released, complementing the existing contention_begin/contention_end tracepoints which fire on the waiter side.

This enables correlating lock hold time under contention with waiter events by lock address.

Add trace_contended_release() calls to the slowpath unlock paths of sleepable locks: mutex, rtmutex, semaphore, rwsem, percpu-rwsem, and RT-specific rwbase locks.

Where possible, trace_contended_release() fires before the lock is released and before the waiter is woken. For some lock types, the tracepoint fires after the release but before the wake. Making the placement consistent across all lock types is not worth the added complexity.

For reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

---
 include/trace/events/lock.h   | 17 +++++++++++++++++
 kernel/locking/mutex.c        |  4 ++++
 kernel/locking/percpu-rwsem.c | 11 +++++++++++
 kernel/locking/rtmutex.c      |  1 +
 kernel/locking/rwbase_rt.c    |  6 ++++++
 kernel/locking/rwsem.c        | 10 ++++++++--
 kernel/locking/semaphore.c    |  4 ++++
 7 files changed, 51 insertions(+), 2 deletions(-)
include/trace/events/lock.h +17 -0
index da978f2afb45..1ded869cd619 100644
@@ -137,6 +137,23 @@ TRACE_EVENT(contention_end,
 	TP_printk("%p (ret=%d)", __entry->lock_addr, __entry->ret)
 );
 
+TRACE_EVENT(contended_release,
+
+	TP_PROTO(void *lock),
+
+	TP_ARGS(lock),
+
+	TP_STRUCT__entry(
+		__field(void *, lock_addr)
+	),
+
+	TP_fast_assign(
+		__entry->lock_addr = lock;
+	),
+
+	TP_printk("%p", __entry->lock_addr)
+);
+
 #endif /* _TRACE_LOCK_H */
 
 /* This part must be outside protection */
kernel/locking/mutex.c +4 -0
index 427187ff02db..6c2c9312eb8f 100644
@@ -997,6 +997,9 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 		wake_q_add(&wake_q, next);
 	}
 
+	if (trace_contended_release_enabled() && waiter)
+		trace_contended_release(lock);
+
 	if (owner & MUTEX_FLAG_HANDOFF)
 		__mutex_handoff(lock, next);
 
@@ -1194,6 +1197,7 @@ EXPORT_SYMBOL(ww_mutex_lock_interruptible);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(contention_begin);
 EXPORT_TRACEPOINT_SYMBOL_GPL(contention_end);
+EXPORT_TRACEPOINT_SYMBOL_GPL(contended_release);
 
 /**
  * atomic_dec_and_mutex_lock - return holding mutex if we dec to 0
kernel/locking/percpu-rwsem.c +11 -0
index f3ee7a0d6047..46b5903989b8 100644
@@ -263,6 +263,9 @@ void percpu_up_write(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);
 
+	if (trace_contended_release_enabled() && wq_has_sleeper(&sem->waiters))
+		trace_contended_release(sem);
+
 	/*
 	 * Signal the writer is done, no fast path yet.
 	 *
@@ -292,6 +295,14 @@ EXPORT_SYMBOL_GPL(percpu_up_write);
 void __percpu_up_read(struct percpu_rw_semaphore *sem)
 {
 	lockdep_assert_preemption_disabled();
+	/*
+	 * After percpu_up_write() completes, rcu_sync_is_idle() can still
+	 * return false during the grace period, forcing readers into this
+	 * slowpath. Only trace when a writer is actually waiting for
+	 * readers to drain.
+	 */
+	if (trace_contended_release_enabled() && rcuwait_active(&sem->writer))
+		trace_contended_release(sem);
 	/*
 	 * slowpath; reader will only ever wake a single blocked
 	 * writer.
kernel/locking/rtmutex.c +1 -0
index ccaba6148b61..3db8a840b4e8 100644
@@ -1466,6 +1466,7 @@ static void __sched rt_mutex_slowunlock(struct rt_mutex_base *lock)
 		raw_spin_lock_irqsave(&lock->wait_lock, flags);
 	}
 
+	trace_contended_release(lock);
 	/*
 	 * The wakeup next waiter path does not suffer from the above
 	 * race. See the comments there.
kernel/locking/rwbase_rt.c +6 -0
index 82e078c0665a..74da5601018f 100644
@@ -174,6 +174,8 @@ static void __sched __rwbase_read_unlock(struct rwbase_rt *rwb,
 static __always_inline void rwbase_read_unlock(struct rwbase_rt *rwb,
 					       unsigned int state)
 {
+	if (trace_contended_release_enabled() && rt_mutex_owner(&rwb->rtmutex))
+		trace_contended_release(rwb);
 	/*
 	 * rwb->readers can only hit 0 when a writer is waiting for the
 	 * active readers to leave the critical section.
@@ -205,6 +207,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);
 	__rwbase_write_unlock(rwb, WRITER_BIAS, flags);
 }
 
@@ -214,6 +218,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);
 	/* Release it and account current as reader */
 	__rwbase_write_unlock(rwb, WRITER_BIAS - 1, flags);
 }
kernel/locking/rwsem.c +8 -2
index bf647097369c..602d5fd3c91a 100644
@@ -1387,6 +1387,8 @@ static inline void __up_read(struct rw_semaphore *sem)
 	rwsem_clear_reader_owned(sem);
 	tmp = atomic_long_add_return_release(-RWSEM_READER_BIAS, &sem->count);
 	DEBUG_RWSEMS_WARN_ON(tmp < 0, sem);
+	if (trace_contended_release_enabled() && (tmp & RWSEM_FLAG_WAITERS))
+		trace_contended_release(sem);
 	if (unlikely((tmp & (RWSEM_LOCK_MASK|RWSEM_FLAG_WAITERS)) ==
 		      RWSEM_FLAG_WAITERS)) {
 		clear_nonspinnable(sem);
@@ -1413,8 +1415,10 @@ static inline void __up_write(struct rw_semaphore *sem)
 	preempt_disable();
 	rwsem_clear_owner(sem);
 	tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
-	if (unlikely(tmp & RWSEM_FLAG_WAITERS))
+	if (unlikely(tmp & RWSEM_FLAG_WAITERS)) {
+		trace_contended_release(sem);
 		rwsem_wake(sem);
+	}
 	preempt_enable();
 }
 
@@ -1437,8 +1441,10 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 	tmp = atomic_long_fetch_add_release(
 		-RWSEM_WRITER_LOCKED+RWSEM_READER_BIAS, &sem->count);
 	rwsem_set_reader_owned(sem);
-	if (tmp & RWSEM_FLAG_WAITERS)
+	if (tmp & RWSEM_FLAG_WAITERS) {
+		trace_contended_release(sem);
 		rwsem_downgrade_wake(sem);
+	}
 	preempt_enable();
 }
 
kernel/locking/semaphore.c +4 -0
index 74d41433ba13..35ac3498dca5 100644
@@ -230,6 +230,10 @@ void __sched up(struct semaphore *sem)
 		sem->count++;
 	else
 		__up(sem, &wake_q);
+
+	if (trace_contended_release_enabled() && !wake_q_empty(&wake_q))
+		trace_contended_release(sem);
+
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
 	if (!wake_q_empty(&wake_q))
 		wake_up_q(&wake_q);
Signature · 1 line
2.52.0
#5 lore raw

Re: [PATCH v4 3/5] locking: Add contended_release tracepoint to sleepable locks

Quoted 65 lines · On Thu, 26 Mar 2026 15:10:02 +0000 Dmitry Ilvokhin <[email protected]> wrote:

Add the contended_release trace event. This tracepoint fires on the holder side when a contended lock is released, complementing the existing contention_begin/contention_end tracepoints which fire on the waiter side.

This enables correlating lock hold time under contention with waiter events by lock address.

Add trace_contended_release() calls to the slowpath unlock paths of sleepable locks: mutex, rtmutex, semaphore, rwsem, percpu-rwsem, and RT-specific rwbase locks.

Where possible, trace_contended_release() fires before the lock is released and before the waiter is woken. For some lock types, the tracepoint fires after the release but before the wake. Making the placement consistent across all lock types is not worth the added complexity.

For reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

---
 include/trace/events/lock.h   | 17 +++++++++++++++++
 kernel/locking/mutex.c        |  4 ++++
 kernel/locking/percpu-rwsem.c | 11 +++++++++++
 kernel/locking/rtmutex.c      |  1 +
 kernel/locking/rwbase_rt.c    |  6 ++++++
 kernel/locking/rwsem.c        | 10 ++++++++--
 kernel/locking/semaphore.c    |  4 ++++
 7 files changed, 51 insertions(+), 2 deletions(-)
include/trace/events/lock.h +17 -0
index da978f2afb45..1ded869cd619 100644
@@ -137,6 +137,23 @@ TRACE_EVENT(contention_end,
 	TP_printk("%p (ret=%d)", __entry->lock_addr, __entry->ret)
 );
 
+TRACE_EVENT(contended_release,
+
+	TP_PROTO(void *lock),
+
+	TP_ARGS(lock),
+
+	TP_STRUCT__entry(
+		__field(void *, lock_addr)
+	),
+
+	TP_fast_assign(
+		__entry->lock_addr = lock;
+	),
+
+	TP_printk("%p", __entry->lock_addr)
+);
+
 #endif /* _TRACE_LOCK_H */
 
 /* This part must be outside protection */
kernel/locking/mutex.c +0 -0
index 427187ff02db..6c2c9312eb8f 100644

kernel/locking/mutex.c static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1002

Quoted 6 lines
kernel/locking/mutex.c +3 -0
@@ -997,6 +997,9 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 		wake_q_add(&wake_q, next);
 	}
 
+	if (trace_contended_release_enabled() && waiter)
+		trace_contended_release(lock);
+

This won't compile? waiter is declared in the if block, so you are using it outside scope here.

#6 lore raw

Re: [PATCH v4 3/5] locking: Add contended_release tracepoint to sleepable locks

Quoted 70 lines · Usama Arif
Quoted 68 lines · On Thu, 26 Mar 2026 15:10:02 +0000 Dmitry Ilvokhin <[email protected]> wrote:

Add the contended_release trace event. This tracepoint fires on the holder side when a contended lock is released, complementing the existing contention_begin/contention_end tracepoints which fire on the waiter side.

This enables correlating lock hold time under contention with waiter events by lock address.

Add trace_contended_release() calls to the slowpath unlock paths of sleepable locks: mutex, rtmutex, semaphore, rwsem, percpu-rwsem, and RT-specific rwbase locks.

Where possible, trace_contended_release() fires before the lock is released and before the waiter is woken. For some lock types, the tracepoint fires after the release but before the wake. Making the placement consistent across all lock types is not worth the added complexity.

For reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

---
 include/trace/events/lock.h   | 17 +++++++++++++++++
 kernel/locking/mutex.c        |  4 ++++
 kernel/locking/percpu-rwsem.c | 11 +++++++++++
 kernel/locking/rtmutex.c      |  1 +
 kernel/locking/rwbase_rt.c    |  6 ++++++
 kernel/locking/rwsem.c        | 10 ++++++++--
 kernel/locking/semaphore.c    |  4 ++++
 7 files changed, 51 insertions(+), 2 deletions(-)
include/trace/events/lock.h +17 -0
index da978f2afb45..1ded869cd619 100644
@@ -137,6 +137,23 @@ TRACE_EVENT(contention_end,
 	TP_printk("%p (ret=%d)", __entry->lock_addr, __entry->ret)
 );
 
+TRACE_EVENT(contended_release,
+
+	TP_PROTO(void *lock),
+
+	TP_ARGS(lock),
+
+	TP_STRUCT__entry(
+		__field(void *, lock_addr)
+	),
+
+	TP_fast_assign(
+		__entry->lock_addr = lock;
+	),
+
+	TP_printk("%p", __entry->lock_addr)
+);
+
 #endif /* _TRACE_LOCK_H */
 
 /* This part must be outside protection */
kernel/locking/mutex.c +0 -0
index 427187ff02db..6c2c9312eb8f 100644
@@ -997,6 +997,9 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 		wake_q_add(&wake_q, next);
 	}
 
Quoted 7 lines

kernel/locking/mutex.c static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1002

Quoted 3 lines
kernel/locking/mutex.c +3 -0
+	if (trace_contended_release_enabled() && waiter)
+		trace_contended_release(lock);
+

This won't compile? waiter is declared in the if block, so you are using it outside scope here.

Thanks for the feedback, Usama.

waiter is declared at function scope, right on top. It's also assigned before the if block, so it's still in scope at the tracepoint.

#7 lore raw

Re: [PATCH v4 3/5] locking: Add contended_release tracepoint to sleepable locks

Quoted 79 lines · Dmitry Ilvokhin
Quoted 70 lines · Usama Arif
Quoted 68 lines · On Thu, 26 Mar 2026 15:10:02 +0000 Dmitry Ilvokhin <[email protected]> wrote:

Add the contended_release trace event. This tracepoint fires on the holder side when a contended lock is released, complementing the existing contention_begin/contention_end tracepoints which fire on the waiter side.

This enables correlating lock hold time under contention with waiter events by lock address.

Add trace_contended_release() calls to the slowpath unlock paths of sleepable locks: mutex, rtmutex, semaphore, rwsem, percpu-rwsem, and RT-specific rwbase locks.

Where possible, trace_contended_release() fires before the lock is released and before the waiter is woken. For some lock types, the tracepoint fires after the release but before the wake. Making the placement consistent across all lock types is not worth the added complexity.

For reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

---
 include/trace/events/lock.h   | 17 +++++++++++++++++
 kernel/locking/mutex.c        |  4 ++++
 kernel/locking/percpu-rwsem.c | 11 +++++++++++
 kernel/locking/rtmutex.c      |  1 +
 kernel/locking/rwbase_rt.c    |  6 ++++++
 kernel/locking/rwsem.c        | 10 ++++++++--
 kernel/locking/semaphore.c    |  4 ++++
 7 files changed, 51 insertions(+), 2 deletions(-)
include/trace/events/lock.h +17 -0
index da978f2afb45..1ded869cd619 100644
@@ -137,6 +137,23 @@ TRACE_EVENT(contention_end,
 	TP_printk("%p (ret=%d)", __entry->lock_addr, __entry->ret)
 );
 
+TRACE_EVENT(contended_release,
+
+	TP_PROTO(void *lock),
+
+	TP_ARGS(lock),
+
+	TP_STRUCT__entry(
+		__field(void *, lock_addr)
+	),
+
+	TP_fast_assign(
+		__entry->lock_addr = lock;
+	),
+
+	TP_printk("%p", __entry->lock_addr)
+);
+
 #endif /* _TRACE_LOCK_H */
 
 /* This part must be outside protection */
kernel/locking/mutex.c +0 -0
index 427187ff02db..6c2c9312eb8f 100644
@@ -997,6 +997,9 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 		wake_q_add(&wake_q, next);
 	}
 
Quoted 7 lines

kernel/locking/mutex.c static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1002

Quoted 3 lines
kernel/locking/mutex.c +3 -0
+	if (trace_contended_release_enabled() && waiter)
+		trace_contended_release(lock);
+

This won't compile? waiter is declared in the if block, so you are using it outside scope here.

Quoted 4 lines

Thanks for the feedback, Usama.

waiter is declared at function scope, right on top. It's also assigned before the if block, so it's still in scope at the tracepoint.

Ah ok, I was reviewing on top of mm-new branch from today where waiter is declared in the if block. Probably something changed related to locking/tracing and its not in mm-new yet.

Acked-by on v4 3/5
#8 lore raw

Re: [PATCH v4 3/5] locking: Add contended_release tracepoint to sleepable locks

In reply to Dmitry Ilvokhin
Quoted 18 lines · Dmitry Ilvokhin

Add the contended_release trace event. This tracepoint fires on the holder side when a contended lock is released, complementing the existing contention_begin/contention_end tracepoints which fire on the waiter side.

This enables correlating lock hold time under contention with waiter events by lock address.

Add trace_contended_release() calls to the slowpath unlock paths of sleepable locks: mutex, rtmutex, semaphore, rwsem, percpu-rwsem, and RT-specific rwbase locks.

Where possible, trace_contended_release() fires before the lock is released and before the waiter is woken. For some lock types, the tracepoint fires after the release but before the wake. Making the placement consistent across all lock types is not worth the added complexity.

Quoted 4 lines

For reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

Looks plausible:

Quoted 187 lines
---
 include/trace/events/lock.h   | 17 +++++++++++++++++
 kernel/locking/mutex.c        |  4 ++++
 kernel/locking/percpu-rwsem.c | 11 +++++++++++
 kernel/locking/rtmutex.c      |  1 +
 kernel/locking/rwbase_rt.c    |  6 ++++++
 kernel/locking/rwsem.c        | 10 ++++++++--
 kernel/locking/semaphore.c    |  4 ++++
 7 files changed, 51 insertions(+), 2 deletions(-)
include/trace/events/lock.h +17 -0
index da978f2afb45..1ded869cd619 100644
@@ -137,6 +137,23 @@ TRACE_EVENT(contention_end,
 	TP_printk("%p (ret=%d)", __entry->lock_addr, __entry->ret)
 );
 
+TRACE_EVENT(contended_release,
+
+	TP_PROTO(void *lock),
+
+	TP_ARGS(lock),
+
+	TP_STRUCT__entry(
+		__field(void *, lock_addr)
+	),
+
+	TP_fast_assign(
+		__entry->lock_addr = lock;
+	),
+
+	TP_printk("%p", __entry->lock_addr)
+);
+
 #endif /* _TRACE_LOCK_H */
 
 /* This part must be outside protection */
kernel/locking/mutex.c +4 -0
index 427187ff02db..6c2c9312eb8f 100644
@@ -997,6 +997,9 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 		wake_q_add(&wake_q, next);
 	}
 
+	if (trace_contended_release_enabled() && waiter)
+		trace_contended_release(lock);
+
 	if (owner & MUTEX_FLAG_HANDOFF)
 		__mutex_handoff(lock, next);
 
@@ -1194,6 +1197,7 @@ EXPORT_SYMBOL(ww_mutex_lock_interruptible);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(contention_begin);
 EXPORT_TRACEPOINT_SYMBOL_GPL(contention_end);
+EXPORT_TRACEPOINT_SYMBOL_GPL(contended_release);
 
 /**
  * atomic_dec_and_mutex_lock - return holding mutex if we dec to 0
kernel/locking/percpu-rwsem.c +11 -0
index f3ee7a0d6047..46b5903989b8 100644
@@ -263,6 +263,9 @@ void percpu_up_write(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);
 
+	if (trace_contended_release_enabled() && wq_has_sleeper(&sem->waiters))
+		trace_contended_release(sem);
+
 	/*
 	 * Signal the writer is done, no fast path yet.
 	 *
@@ -292,6 +295,14 @@ EXPORT_SYMBOL_GPL(percpu_up_write);
 void __percpu_up_read(struct percpu_rw_semaphore *sem)
 {
 	lockdep_assert_preemption_disabled();
+	/*
+	 * After percpu_up_write() completes, rcu_sync_is_idle() can still
+	 * return false during the grace period, forcing readers into this
+	 * slowpath. Only trace when a writer is actually waiting for
+	 * readers to drain.
+	 */
+	if (trace_contended_release_enabled() && rcuwait_active(&sem->writer))
+		trace_contended_release(sem);
 	/*
 	 * slowpath; reader will only ever wake a single blocked
 	 * writer.
kernel/locking/rtmutex.c +1 -0
index ccaba6148b61..3db8a840b4e8 100644
@@ -1466,6 +1466,7 @@ static void __sched rt_mutex_slowunlock(struct rt_mutex_base *lock)
 		raw_spin_lock_irqsave(&lock->wait_lock, flags);
 	}
 
+	trace_contended_release(lock);
 	/*
 	 * The wakeup next waiter path does not suffer from the above
 	 * race. See the comments there.
kernel/locking/rwbase_rt.c +6 -0
index 82e078c0665a..74da5601018f 100644
@@ -174,6 +174,8 @@ static void __sched __rwbase_read_unlock(struct rwbase_rt *rwb,
 static __always_inline void rwbase_read_unlock(struct rwbase_rt *rwb,
 					       unsigned int state)
 {
+	if (trace_contended_release_enabled() && rt_mutex_owner(&rwb->rtmutex))
+		trace_contended_release(rwb);
 	/*
 	 * rwb->readers can only hit 0 when a writer is waiting for the
 	 * active readers to leave the critical section.
@@ -205,6 +207,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);
 	__rwbase_write_unlock(rwb, WRITER_BIAS, flags);
 }
 
@@ -214,6 +218,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);
 	/* Release it and account current as reader */
 	__rwbase_write_unlock(rwb, WRITER_BIAS - 1, flags);
 }
kernel/locking/rwsem.c +8 -2
index bf647097369c..602d5fd3c91a 100644
@@ -1387,6 +1387,8 @@ static inline void __up_read(struct rw_semaphore *sem)
 	rwsem_clear_reader_owned(sem);
 	tmp = atomic_long_add_return_release(-RWSEM_READER_BIAS, &sem->count);
 	DEBUG_RWSEMS_WARN_ON(tmp < 0, sem);
+	if (trace_contended_release_enabled() && (tmp & RWSEM_FLAG_WAITERS))
+		trace_contended_release(sem);
 	if (unlikely((tmp & (RWSEM_LOCK_MASK|RWSEM_FLAG_WAITERS)) ==
 		      RWSEM_FLAG_WAITERS)) {
 		clear_nonspinnable(sem);
@@ -1413,8 +1415,10 @@ static inline void __up_write(struct rw_semaphore *sem)
 	preempt_disable();
 	rwsem_clear_owner(sem);
 	tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
-	if (unlikely(tmp & RWSEM_FLAG_WAITERS))
+	if (unlikely(tmp & RWSEM_FLAG_WAITERS)) {
+		trace_contended_release(sem);
 		rwsem_wake(sem);
+	}
 	preempt_enable();
 }
 
@@ -1437,8 +1441,10 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 	tmp = atomic_long_fetch_add_release(
 		-RWSEM_WRITER_LOCKED+RWSEM_READER_BIAS, &sem->count);
 	rwsem_set_reader_owned(sem);
-	if (tmp & RWSEM_FLAG_WAITERS)
+	if (tmp & RWSEM_FLAG_WAITERS) {
+		trace_contended_release(sem);
 		rwsem_downgrade_wake(sem);
+	}
 	preempt_enable();
 }
 
kernel/locking/semaphore.c +4 -0
index 74d41433ba13..35ac3498dca5 100644
@@ -230,6 +230,10 @@ void __sched up(struct semaphore *sem)
 		sem->count++;
 	else
 		__up(sem, &wake_q);
+
+	if (trace_contended_release_enabled() && !wake_q_empty(&wake_q))
+		trace_contended_release(sem);
+
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
 	if (!wake_q_empty(&wake_q))
 		wake_up_q(&wake_q);
Signature · 1 line
2.52.0
#9 lore raw

PATCH v4 4/5 locking: Factor out queued_spin_release()

In reply to Dmitry Ilvokhin

Introduce queued_spin_release() as an arch-overridable unlock primitive, and make queued_spin_unlock() a generic wrapper around it. This is a preparatory refactoring for the next commit, which adds contended_release tracepoint instrumentation to queued_spin_unlock().

Rename the existing arch-specific queued_spin_unlock() overrides on x86 (paravirt) and MIPS to queued_spin_release().

No functional change.

---
 arch/mips/include/asm/spinlock.h         |  6 +++---
 arch/x86/include/asm/paravirt-spinlock.h |  6 +++---
 include/asm-generic/qspinlock.h          | 15 ++++++++++++---
 3 files changed, 18 insertions(+), 9 deletions(-)
arch/mips/include/asm/spinlock.h +3 -3
index 6ce2117e49f6..c349162f15eb 100644
@@ -13,12 +13,12 @@
 
 #include <asm-generic/qspinlock_types.h>
 
-#define	queued_spin_unlock queued_spin_unlock
+#define	queued_spin_release queued_spin_release
 /**
- * queued_spin_unlock - release a queued spinlock
+ * queued_spin_release - release a queued spinlock
  * @lock : Pointer to queued spinlock structure
  */
-static inline void queued_spin_unlock(struct qspinlock *lock)
+static inline void queued_spin_release(struct qspinlock *lock)
 {
 	/* This could be optimised with ARCH_HAS_MMIOWB */
 	mmiowb();
arch/x86/include/asm/paravirt-spinlock.h +3 -3
index 7beffcb08ed6..ac75e0736198 100644
@@ -49,9 +49,9 @@ static __always_inline bool pv_vcpu_is_preempted(long cpu)
 				ALT_NOT(X86_FEATURE_VCPUPREEMPT));
 }
 
-#define queued_spin_unlock queued_spin_unlock
+#define queued_spin_release queued_spin_release
 /**
- * queued_spin_unlock - release a queued spinlock
+ * queued_spin_release - release a queued spinlock
  * @lock : Pointer to queued spinlock structure
  *
  * A smp_store_release() on the least-significant byte.
@@ -66,7 +66,7 @@ static inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
 	pv_queued_spin_lock_slowpath(lock, val);
 }
 
-static inline void queued_spin_unlock(struct qspinlock *lock)
+static inline void queued_spin_release(struct qspinlock *lock)
 {
 	kcsan_release();
 	pv_queued_spin_unlock(lock);
include/asm-generic/qspinlock.h +12 -3
index bf47cca2c375..df76f34645a0 100644
@@ -115,12 +115,12 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
 }
 #endif
 
-#ifndef queued_spin_unlock
+#ifndef queued_spin_release
 /**
- * queued_spin_unlock - release a queued spinlock
+ * queued_spin_release - release a queued spinlock
  * @lock : Pointer to queued spinlock structure
  */
-static __always_inline void queued_spin_unlock(struct qspinlock *lock)
+static __always_inline void queued_spin_release(struct qspinlock *lock)
 {
 	/*
 	 * unlock() needs release semantics:
@@ -129,6 +129,15 @@ static __always_inline void queued_spin_unlock(struct qspinlock *lock)
 }
 #endif
 
+/**
+ * queued_spin_unlock - unlock a queued spinlock
+ * @lock : Pointer to queued spinlock structure
+ */
+static __always_inline void queued_spin_unlock(struct qspinlock *lock)
+{
+	queued_spin_release(lock);
+}
+
 #ifndef virt_spin_lock
 static __always_inline bool virt_spin_lock(struct qspinlock *lock)
 {
Signature · 1 line
2.52.0
Reviewed-by on v4 4/5
#10 lore raw

Re: [PATCH v4 4/5] locking: Factor out queued_spin_release()

Quoted 11 lines · Dmitry Ilvokhin

Introduce queued_spin_release() as an arch-overridable unlock primitive, and make queued_spin_unlock() a generic wrapper around it. This is a preparatory refactoring for the next commit, which adds contended_release tracepoint instrumentation to queued_spin_unlock().

Rename the existing arch-specific queued_spin_unlock() overrides on x86 (paravirt) and MIPS to queued_spin_release().

No functional change.

Quoted 90 lines
---
 arch/mips/include/asm/spinlock.h         |  6 +++---
 arch/x86/include/asm/paravirt-spinlock.h |  6 +++---
 include/asm-generic/qspinlock.h          | 15 ++++++++++++---
 3 files changed, 18 insertions(+), 9 deletions(-)
arch/mips/include/asm/spinlock.h +3 -3
index 6ce2117e49f6..c349162f15eb 100644
@@ -13,12 +13,12 @@
 
 #include <asm-generic/qspinlock_types.h>
 
-#define	queued_spin_unlock queued_spin_unlock
+#define	queued_spin_release queued_spin_release
 /**
- * queued_spin_unlock - release a queued spinlock
+ * queued_spin_release - release a queued spinlock
  * @lock : Pointer to queued spinlock structure
  */
-static inline void queued_spin_unlock(struct qspinlock *lock)
+static inline void queued_spin_release(struct qspinlock *lock)
 {
 	/* This could be optimised with ARCH_HAS_MMIOWB */
 	mmiowb();
arch/x86/include/asm/paravirt-spinlock.h +3 -3
index 7beffcb08ed6..ac75e0736198 100644
@@ -49,9 +49,9 @@ static __always_inline bool pv_vcpu_is_preempted(long cpu)
 				ALT_NOT(X86_FEATURE_VCPUPREEMPT));
 }
 
-#define queued_spin_unlock queued_spin_unlock
+#define queued_spin_release queued_spin_release
 /**
- * queued_spin_unlock - release a queued spinlock
+ * queued_spin_release - release a queued spinlock
  * @lock : Pointer to queued spinlock structure
  *
  * A smp_store_release() on the least-significant byte.
@@ -66,7 +66,7 @@ static inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
 	pv_queued_spin_lock_slowpath(lock, val);
 }
 
-static inline void queued_spin_unlock(struct qspinlock *lock)
+static inline void queued_spin_release(struct qspinlock *lock)
 {
 	kcsan_release();
 	pv_queued_spin_unlock(lock);
include/asm-generic/qspinlock.h +12 -3
index bf47cca2c375..df76f34645a0 100644
@@ -115,12 +115,12 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
 }
 #endif
 
-#ifndef queued_spin_unlock
+#ifndef queued_spin_release
 /**
- * queued_spin_unlock - release a queued spinlock
+ * queued_spin_release - release a queued spinlock
  * @lock : Pointer to queued spinlock structure
  */
-static __always_inline void queued_spin_unlock(struct qspinlock *lock)
+static __always_inline void queued_spin_release(struct qspinlock *lock)
 {
 	/*
 	 * unlock() needs release semantics:
@@ -129,6 +129,15 @@ static __always_inline void queued_spin_unlock(struct qspinlock *lock)
 }
 #endif
 
+/**
+ * queued_spin_unlock - unlock a queued spinlock
+ * @lock : Pointer to queued spinlock structure
+ */
+static __always_inline void queued_spin_unlock(struct qspinlock *lock)
+{
+	queued_spin_release(lock);
+}
+
 #ifndef virt_spin_lock
 static __always_inline bool virt_spin_lock(struct qspinlock *lock)
 {
Signature · 1 line
2.52.0
#11 lore raw

PATCH v4 5/5 locking: Add contended_release tracepoint to spinning locks

In reply to Dmitry Ilvokhin

Extend the contended_release tracepoint to queued spinlocks and queued rwlocks.

Use the arch-overridable queued_spin_release(), introduced in the previous commit, to ensure the tracepoint works correctly across all architectures, including those with custom unlock implementations (e.g. x86 paravirt).

When the tracepoint is disabled, the only addition to the hot path is a single NOP instruction (the static branch). When enabled, the contention check, trace call, and unlock are combined in an out-of-line function to minimize hot path impact, avoiding the compiler needing to preserve the lock pointer in a callee-saved register across the trace call.

Binary size impact (x86_64, defconfig):
uninlined unlock (common case): +983 bytes (+0.00%)
inlined unlock (worst case): +58165 bytes (+0.24%)

The inlined unlock case could not be achieved through Kconfig options on x86_64 as PREEMPT_BUILD unconditionally selects UNINLINE_SPIN_UNLOCK on x86_64. The UNINLINE_SPIN_UNLOCK guards were manually inverted to force inline the unlock path and estimate the worst case binary size increase.

Architectures with fully custom qspinlock implementations (e.g. PowerPC) are not covered by this change.

---
 include/asm-generic/qrwlock.h   | 48 +++++++++++++++++++++++++++------
 include/asm-generic/qspinlock.h | 18 +++++++++++++
 kernel/locking/qrwlock.c        | 16 +++++++++++
 kernel/locking/qspinlock.c      |  8 ++++++
 4 files changed, 82 insertions(+), 8 deletions(-)
include/asm-generic/qrwlock.h +40 -8
index 75b8f4601b28..e24dc537fd66 100644
@@ -14,6 +14,7 @@
 #define __ASM_GENERIC_QRWLOCK_H
 
 #include <linux/atomic.h>
+#include <linux/tracepoint-defs.h>
 #include <asm/barrier.h>
 #include <asm/processor.h>
 
@@ -35,6 +36,10 @@
  */
 extern void queued_read_lock_slowpath(struct qrwlock *lock);
 extern void queued_write_lock_slowpath(struct qrwlock *lock);
+extern void queued_read_unlock_traced(struct qrwlock *lock);
+extern void queued_write_unlock_traced(struct qrwlock *lock);
+
+DECLARE_TRACEPOINT(contended_release);
 
 /**
  * queued_read_trylock - try to acquire read lock of a queued rwlock
@@ -102,10 +107,16 @@ static inline void queued_write_lock(struct qrwlock *lock)
 }
 
 /**
- * queued_read_unlock - release read lock of a queued rwlock
+ * queued_rwlock_is_contended - check if the lock is contended
  * @lock : Pointer to queued rwlock structure
+ * Return: 1 if lock contended, 0 otherwise
  */
-static inline void queued_read_unlock(struct qrwlock *lock)
+static inline int queued_rwlock_is_contended(struct qrwlock *lock)
+{
+	return arch_spin_is_locked(&lock->wait_lock);
+}
+
+static __always_inline void __queued_read_unlock(struct qrwlock *lock)
 {
 	/*
 	 * Atomically decrement the reader count
@@ -114,22 +125,43 @@ static inline void queued_read_unlock(struct qrwlock *lock)
 }
 
 /**
- * queued_write_unlock - release write lock of a queued rwlock
+ * queued_read_unlock - release read lock of a queued rwlock
  * @lock : Pointer to queued rwlock structure
  */
-static inline void queued_write_unlock(struct qrwlock *lock)
+static inline void queued_read_unlock(struct qrwlock *lock)
+{
+	/*
+	 * Trace and unlock are combined in the traced unlock variant so
+	 * the compiler does not need to preserve the lock pointer across
+	 * the function call, avoiding callee-saved register save/restore
+	 * on the hot path.
+	 */
+	if (tracepoint_enabled(contended_release)) {
+		queued_read_unlock_traced(lock);
+		return;
+	}
+
+	__queued_read_unlock(lock);
+}
+
+static __always_inline void __queued_write_unlock(struct qrwlock *lock)
 {
 	smp_store_release(&lock->wlocked, 0);
 }
 
 /**
- * queued_rwlock_is_contended - check if the lock is contended
+ * queued_write_unlock - release write lock of a queued rwlock
  * @lock : Pointer to queued rwlock structure
- * Return: 1 if lock contended, 0 otherwise
  */
-static inline int queued_rwlock_is_contended(struct qrwlock *lock)
+static inline void queued_write_unlock(struct qrwlock *lock)
 {
-	return arch_spin_is_locked(&lock->wait_lock);
+	/* See comment in queued_read_unlock(). */
+	if (tracepoint_enabled(contended_release)) {
+		queued_write_unlock_traced(lock);
+		return;
+	}
+
+	__queued_write_unlock(lock);
 }
 
 /*
include/asm-generic/qspinlock.h +18 -0
index df76f34645a0..915a4c2777f6 100644
@@ -41,6 +41,7 @@
 
 #include <asm-generic/qspinlock_types.h>
 #include <linux/atomic.h>
+#include <linux/tracepoint-defs.h>
 
 #ifndef queued_spin_is_locked
 /**
@@ -129,12 +130,29 @@ static __always_inline void queued_spin_release(struct qspinlock *lock)
 }
 #endif
 
+DECLARE_TRACEPOINT(contended_release);
+
+extern void queued_spin_release_traced(struct qspinlock *lock);
+
 /**
  * queued_spin_unlock - unlock a queued spinlock
  * @lock : Pointer to queued spinlock structure
+ *
+ * Generic tracing wrapper around the arch-overridable
+ * queued_spin_release().
  */
 static __always_inline void queued_spin_unlock(struct qspinlock *lock)
 {
+	/*
+	 * Trace and release are combined in queued_spin_release_traced() so
+	 * the compiler does not need to preserve the lock pointer across the
+	 * function call, avoiding callee-saved register save/restore on the
+	 * hot path.
+	 */
+	if (tracepoint_enabled(contended_release)) {
+		queued_spin_release_traced(lock);
+		return;
+	}
 	queued_spin_release(lock);
 }
 
kernel/locking/qrwlock.c +16 -0
index d2ef312a8611..5f7a0fc2b27a 100644
@@ -90,3 +90,19 @@ void __lockfunc queued_write_lock_slowpath(struct qrwlock *lock)
 	trace_contention_end(lock, 0);
 }
 EXPORT_SYMBOL(queued_write_lock_slowpath);
+
+void __lockfunc queued_read_unlock_traced(struct qrwlock *lock)
+{
+	if (queued_rwlock_is_contended(lock))
+		trace_contended_release(lock);
+	__queued_read_unlock(lock);
+}
+EXPORT_SYMBOL(queued_read_unlock_traced);
+
+void __lockfunc queued_write_unlock_traced(struct qrwlock *lock)
+{
+	if (queued_rwlock_is_contended(lock))
+		trace_contended_release(lock);
+	__queued_write_unlock(lock);
+}
+EXPORT_SYMBOL(queued_write_unlock_traced);
kernel/locking/qspinlock.c +8 -0
index af8d122bb649..c72610980ec7 100644
@@ -104,6 +104,14 @@ static __always_inline u32  __pv_wait_head_or_lock(struct qspinlock *lock,
 #define queued_spin_lock_slowpath	native_queued_spin_lock_slowpath
 #endif
 
+void __lockfunc queued_spin_release_traced(struct qspinlock *lock)
+{
+	if (queued_spin_is_contended(lock))
+		trace_contended_release(lock);
+	queued_spin_release(lock);
+}
+EXPORT_SYMBOL(queued_spin_release_traced);
+
 #endif /* _GEN_PV_LOCK_SLOWPATH */
 
 /**
Signature · 1 line
2.52.0
#12 lore raw

Re: [PATCH v4 5/5] locking: Add contended_release tracepoint to spinning locks

Quoted 90 lines · Dmitry Ilvokhin

Extend the contended_release tracepoint to queued spinlocks and queued rwlocks.

Use the arch-overridable queued_spin_release(), introduced in the previous commit, to ensure the tracepoint works correctly across all architectures, including those with custom unlock implementations (e.g. x86 paravirt).

When the tracepoint is disabled, the only addition to the hot path is a single NOP instruction (the static branch). When enabled, the contention check, trace call, and unlock are combined in an out-of-line function to minimize hot path impact, avoiding the compiler needing to preserve the lock pointer in a callee-saved register across the trace call.

Binary size impact (x86_64, defconfig):
uninlined unlock (common case): +983 bytes (+0.00%)
inlined unlock (worst case): +58165 bytes (+0.24%)

The inlined unlock case could not be achieved through Kconfig options on x86_64 as PREEMPT_BUILD unconditionally selects UNINLINE_SPIN_UNLOCK on x86_64. The UNINLINE_SPIN_UNLOCK guards were manually inverted to force inline the unlock path and estimate the worst case binary size increase.

Architectures with fully custom qspinlock implementations (e.g. PowerPC) are not covered by this change.

---
 include/asm-generic/qrwlock.h   | 48 +++++++++++++++++++++++++++------
 include/asm-generic/qspinlock.h | 18 +++++++++++++
 kernel/locking/qrwlock.c        | 16 +++++++++++
 kernel/locking/qspinlock.c      |  8 ++++++
 4 files changed, 82 insertions(+), 8 deletions(-)
include/asm-generic/qrwlock.h +19 -4
index 75b8f4601b28..e24dc537fd66 100644
@@ -14,6 +14,7 @@
 #define __ASM_GENERIC_QRWLOCK_H
 
 #include <linux/atomic.h>
+#include <linux/tracepoint-defs.h>
 #include <asm/barrier.h>
 #include <asm/processor.h>
 
@@ -35,6 +36,10 @@
  */
 extern void queued_read_lock_slowpath(struct qrwlock *lock);
 extern void queued_write_lock_slowpath(struct qrwlock *lock);
+extern void queued_read_unlock_traced(struct qrwlock *lock);
+extern void queued_write_unlock_traced(struct qrwlock *lock);
+
+DECLARE_TRACEPOINT(contended_release);
 
 /**
  * queued_read_trylock - try to acquire read lock of a queued rwlock
@@ -102,10 +107,16 @@ static inline void queued_write_lock(struct qrwlock *lock)
 }
 
 /**
- * queued_read_unlock - release read lock of a queued rwlock
+ * queued_rwlock_is_contended - check if the lock is contended
  * @lock : Pointer to queued rwlock structure
+ * Return: 1 if lock contended, 0 otherwise
  */
-static inline void queued_read_unlock(struct qrwlock *lock)
+static inline int queued_rwlock_is_contended(struct qrwlock *lock)
+{
+	return arch_spin_is_locked(&lock->wait_lock);
+}
+
+static __always_inline void __queued_read_unlock(struct qrwlock *lock)
 {
 	/*
 	 * Atomically decrement the reader count
@@ -114,22 +125,43 @@ static inline void queued_read_unlock(struct qrwlock *lock)
 }
 
 /**
- * queued_write_unlock - release write lock of a queued rwlock
+ * queued_read_unlock - release read lock of a queued rwlock
  * @lock : Pointer to queued rwlock structure
  */
-static inline void queued_write_unlock(struct qrwlock *lock)
+static inline void queued_read_unlock(struct qrwlock *lock)
+{
+	/*
+	 * Trace and unlock are combined in the traced unlock variant so
+	 * the compiler does not need to preserve the lock pointer across

include/asm-generic/qrwlock.h static inline void queued_read_unlock(struct qrwlock *lock) · line 145

Quoted 10 lines
include/asm-generic/qrwlock.h +10 -0
+	 * the function call, avoiding callee-saved register save/restore
+	 * on the hot path.
+	 */
+	if (tracepoint_enabled(contended_release)) {
+		queued_read_unlock_traced(lock);
+		return;
+	}
+
+	__queued_read_unlock(lock);
+}

Shouldn't this refactoring be its own separate patch, similar to 4/5?

That would probably clean up this diff a bit.

Quoted 13 lines
include/asm-generic/qrwlock.h +3 -3
+
+static __always_inline void __queued_write_unlock(struct qrwlock *lock)
 {
 	smp_store_release(&lock->wlocked, 0);
 }
 
 /**
- * queued_rwlock_is_contended - check if the lock is contended
+ * queued_write_unlock - release write lock of a queued rwlock
  * @lock : Pointer to queued rwlock structure
- * Return: 1 if lock contended, 0 otherwise
  */
-static inline int queued_rwlock_is_contended(struct qrwlock *lock)

include/asm-generic/qrwlock.h static inline void queued_read_unlock(struct qrwlock *lock) · line 164

Quoted 10 lines
include/asm-generic/qrwlock.h +8 -1
+static inline void queued_write_unlock(struct qrwlock *lock)
 {
-	return arch_spin_is_locked(&lock->wait_lock);
+	/* See comment in queued_read_unlock(). */
+	if (tracepoint_enabled(contended_release)) {
+		queued_write_unlock_traced(lock);
+		return;
+	}
+
+	__queued_write_unlock(lock);

And the same here, so one patch for interposing __queued_read_unlock() and another for interposing __queued_write_unlock().

kernel/locking/qrwlock.c

Quoted 49 lines
include/asm-generic/qrwlock.h +0 -0
 }
 
 /*
include/asm-generic/qspinlock.h +18 -0
index df76f34645a0..915a4c2777f6 100644
@@ -41,6 +41,7 @@
 
 #include <asm-generic/qspinlock_types.h>
 #include <linux/atomic.h>
+#include <linux/tracepoint-defs.h>
 
 #ifndef queued_spin_is_locked
 /**
@@ -129,12 +130,29 @@ static __always_inline void queued_spin_release(struct qspinlock *lock)
 }
 #endif
 
+DECLARE_TRACEPOINT(contended_release);
+
+extern void queued_spin_release_traced(struct qspinlock *lock);
+
 /**
  * queued_spin_unlock - unlock a queued spinlock
  * @lock : Pointer to queued spinlock structure
+ *
+ * Generic tracing wrapper around the arch-overridable
+ * queued_spin_release().
  */
 static __always_inline void queued_spin_unlock(struct qspinlock *lock)
 {
+	/*
+	 * Trace and release are combined in queued_spin_release_traced() so
+	 * the compiler does not need to preserve the lock pointer across the
+	 * function call, avoiding callee-saved register save/restore on the
+	 * hot path.
+	 */
+	if (tracepoint_enabled(contended_release)) {
+		queued_spin_release_traced(lock);
+		return;
+	}
 	queued_spin_release(lock);
 }
 
kernel/locking/qrwlock.c +0 -0
index d2ef312a8611..5f7a0fc2b27a 100644

And is it possible to have one patch for qspinlock and another for qrwlock? It *looks* like it should be.

Thanx, Paul

Quoted 42 lines
kernel/locking/qrwlock.c +16 -0
@@ -90,3 +90,19 @@ void __lockfunc queued_write_lock_slowpath(struct qrwlock *lock)
 	trace_contention_end(lock, 0);
 }
 EXPORT_SYMBOL(queued_write_lock_slowpath);
+
+void __lockfunc queued_read_unlock_traced(struct qrwlock *lock)
+{
+	if (queued_rwlock_is_contended(lock))
+		trace_contended_release(lock);
+	__queued_read_unlock(lock);
+}
+EXPORT_SYMBOL(queued_read_unlock_traced);
+
+void __lockfunc queued_write_unlock_traced(struct qrwlock *lock)
+{
+	if (queued_rwlock_is_contended(lock))
+		trace_contended_release(lock);
+	__queued_write_unlock(lock);
+}
+EXPORT_SYMBOL(queued_write_unlock_traced);
kernel/locking/qspinlock.c +8 -0
index af8d122bb649..c72610980ec7 100644
@@ -104,6 +104,14 @@ static __always_inline u32  __pv_wait_head_or_lock(struct qspinlock *lock,
 #define queued_spin_lock_slowpath	native_queued_spin_lock_slowpath
 #endif
 
+void __lockfunc queued_spin_release_traced(struct qspinlock *lock)
+{
+	if (queued_spin_is_contended(lock))
+		trace_contended_release(lock);
+	queued_spin_release(lock);
+}
+EXPORT_SYMBOL(queued_spin_release_traced);
+
 #endif /* _GEN_PV_LOCK_SLOWPATH */
 
 /**
Signature · 1 line
2.52.0
#13 lore raw

Re: [PATCH v4 5/5] locking: Add contended_release tracepoint to spinning locks

On Tue, Apr 14, 2026 at 04:20:26PM -0700, Paul E. McKenney wrote:

[...]

Quoted 48 lines
Quoted 5 lines
include/asm-generic/qrwlock.h +5 -0
+static inline void queued_read_unlock(struct qrwlock *lock)
+{
+	/*
+	 * Trace and unlock are combined in the traced unlock variant so
+	 * the compiler does not need to preserve the lock pointer across

include/asm-generic/qrwlock.h static inline void queued_read_unlock(struct qrwlock *lock) · line 145

Quoted 10 lines
include/asm-generic/qrwlock.h +10 -0
+	 * the function call, avoiding callee-saved register save/restore
+	 * on the hot path.
+	 */
+	if (tracepoint_enabled(contended_release)) {
+		queued_read_unlock_traced(lock);
+		return;
+	}
+
+	__queued_read_unlock(lock);
+}

Shouldn't this refactoring be its own separate patch, similar to 4/5?

That would probably clean up this diff a bit.

Quoted 13 lines
include/asm-generic/qrwlock.h +3 -3
+
+static __always_inline void __queued_write_unlock(struct qrwlock *lock)
 {
 	smp_store_release(&lock->wlocked, 0);
 }
 
 /**
- * queued_rwlock_is_contended - check if the lock is contended
+ * queued_write_unlock - release write lock of a queued rwlock
  * @lock : Pointer to queued rwlock structure
- * Return: 1 if lock contended, 0 otherwise
  */
-static inline int queued_rwlock_is_contended(struct qrwlock *lock)

include/asm-generic/qrwlock.h static inline void queued_read_unlock(struct qrwlock *lock) · line 164

Quoted 10 lines
include/asm-generic/qrwlock.h +8 -1
+static inline void queued_write_unlock(struct qrwlock *lock)
 {
-	return arch_spin_is_locked(&lock->wait_lock);
+	/* See comment in queued_read_unlock(). */
+	if (tracepoint_enabled(contended_release)) {
+		queued_write_unlock_traced(lock);
+		return;
+	}
+
+	__queued_write_unlock(lock);

And the same here, so one patch for interposing __queued_read_unlock() and another for interposing __queued_write_unlock().

[...]

Quoted 5 lines

And is it possible to have one patch for qspinlock and another for qrwlock? It *looks* like it should be.

Thanx, Paul

Thanks for the suggestion, Paul.

I think separate commits for the read and write paths of qrwlock is a bit too fine-grained, but I like the point about mixing refactoring with instrumentation and keeping different lock types separate.

I'll split this commit into four.

locking: Factor out __queued_read_unlock()/__queued_write_unlock()
locking: Add contended_release tracepoint to qrwlock
locking: Factor out queued_spin_release()
locking: Add contended_release tracepoint to qspinlock

#14 lore raw
In reply to Dmitry Ilvokhin
Quoted 9 lines · Dmitry Ilvokhin

The existing contention_begin/contention_end tracepoints fire on the waiter side. The lock holder's identity and stack can be captured at contention_begin time (e.g. perf lock contention --lock-owner), but this reflects the holder's state when a waiter arrives, not when the lock is actually released.

This series adds a contended_release tracepoint that fires on the holder side when a lock with waiters is released. This provides:

Quoted 8 lines

- Hold time estimation: when the holder's own acquisition was contended, its contention_end (acquisition) and contended_release can be correlated to measure how long the lock was held under contention.

- The holder's stack at release time, which may differ from what perf lock contention --lock-owner captures if the holder does significant work between the waiter's arrival and the unlock.

As someone who's not an expert in this area (so please use short words to explain it to me), why do we want to know how long this holder took to release the lock from when it became contended?

I understand why we want to know how long any given waiter had to wait to gain the lock (but we already have tracepoints which show that).

I also don't understand why we want to know the holder's stack at release time. The stack at contention-begin time will include the point at which the lock was acquired which should be correlated with where the lock was released.

Perhaps examples might help me understand why we want this?

maintainer
#15 lore raw
Quoted 11 lines · On Thu, 26 Mar 2026 15:55:21 +0000 Matthew Wilcox <[email protected]> wrote:
Quoted 3 lines

- The holder's stack at release time, which may differ from what perf lock contention --lock-owner captures if the holder does significant work between the waiter's arrival and the unlock.

As someone who's not an expert in this area (so please use short words to explain it to me), why do we want to know how long this holder took to release the lock from when it became contended?

I understand why we want to know how long any given waiter had to wait to gain the lock (but we already have tracepoints which show that).

Quoted 6 lines

I also don't understand why we want to know the holder's stack at release time. The stack at contention-begin time will include the point at which the lock was acquired which should be correlated with where the lock was released.

Perhaps examples might help me understand why we want this?

Dmitry could give his own rationale for this, but I have my only use case.

This would be useful to find out how long the critical section is. If a lock is highly contended by many tasks, you could get a high contention time simply because other tasks are causing the delay for the waiter. Seeing the release time and location would let you also know how long the critical section was held, and if the length of the critical section is causing the contention.

Having a stack trace of the release would differentiate the path that released the lock, as there can be many places that release them. Although, I have to admit, I'm not sure there are many different places locks are released. Especially now that we have guard(), which will make all the releases in a function at the same location.

-- Steve

#16 lore raw
In reply to Matthew Wilcox
Quoted 19 lines · Matthew Wilcox
Quoted 9 lines · Dmitry Ilvokhin

The existing contention_begin/contention_end tracepoints fire on the waiter side. The lock holder's identity and stack can be captured at contention_begin time (e.g. perf lock contention --lock-owner), but this reflects the holder's state when a waiter arrives, not when the lock is actually released.

This series adds a contended_release tracepoint that fires on the holder side when a lock with waiters is released. This provides:

Quoted 8 lines

- Hold time estimation: when the holder's own acquisition was contended, its contention_end (acquisition) and contended_release can be correlated to measure how long the lock was held under contention.

- The holder's stack at release time, which may differ from what perf lock contention --lock-owner captures if the holder does significant work between the waiter's arrival and the unlock.

Quoted 6 lines

As someone who's not an expert in this area (so please use short words to explain it to me), why do we want to know how long this holder took to release the lock from when it became contended?

I understand why we want to know how long any given waiter had to wait to gain the lock (but we already have tracepoints which show that).

I think the simplest way to think about it is the following. Waiter time is the symptom, while holder time is the cause.

The waiter-side contention_begin/contention_end tells us how long a waiter waited, but that time can span multiple holders.

If a waiter waited 10 ms, we can not tell whether one holder held the lock for 10 ms or five holders held it for 2 ms each. These need different treatments: the first means shrink the critical section, the second means reduce lock frequency or split the lock. Today we can not distinguish between these cases from waiter-side data alone.

Quoted 7 lines

I also don't understand why we want to know the holder's stack at release time. The stack at contention-begin time will include the point at which the lock was acquired which should be correlated with where the lock was released.

Perhaps examples might help me understand why we want this?

Holder's stack allows us to understand who exactly waiters were waiting for to release the lock.

The stack at contention_begin time does not always include the holder's stack. The --lock-owner feature works by reading the owner field from the lock struct, but it only supports mutex and rwsem. For spinlocks, queued rwlocks, semaphores, and several others, the waiter has no visibility into the holder whatsoever.

contended_release fires in the holder's context, so we get the holder's stack at release time. For spinlocks, this is the only way to get any holder-side information.

Original motivation was zone lock contention (a spinlock) in Meta production workloads. We could see waiters were blocked, but had no way to identify the holders or what they were doing.

#17 lore raw
In reply to Dmitry Ilvokhin
Quoted 14 lines · On Thu, 26 Mar 2026 15:09:59 +0000 Dmitry Ilvokhin <[email protected]> wrote:

The existing contention_begin/contention_end tracepoints fire on the waiter side. The lock holder's identity and stack can be captured at contention_begin time (e.g. perf lock contention --lock-owner), but this reflects the holder's state when a waiter arrives, not when the lock is actually released.

This series adds a contended_release tracepoint that fires on the holder side when a lock with waiters is released. This provides:

- Hold time estimation: when the holder's own acquisition was contended, its contention_end (acquisition) and contended_release can be correlated to measure how long the lock was held under contention.

Quoted 7 lines

- The holder's stack at release time, which may differ from what perf lock contention --lock-owner captures if the holder does significant work between the waiter's arrival and the unlock.

Note: for reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

Would it be better to reorder the patches? It would help with git bisectability as well. Move the refractoring work in patch 4 and 5 (excluding adding the tracepoints ofcourse) earlier, and then add all the tracepoints in the same commit at the end? It would help in the future with git blame to see where all the tracepoints were added as well.

#18 lore raw
Quoted 24 lines · Usama Arif
Quoted 14 lines · On Thu, 26 Mar 2026 15:09:59 +0000 Dmitry Ilvokhin <[email protected]> wrote:

The existing contention_begin/contention_end tracepoints fire on the waiter side. The lock holder's identity and stack can be captured at contention_begin time (e.g. perf lock contention --lock-owner), but this reflects the holder's state when a waiter arrives, not when the lock is actually released.

This series adds a contended_release tracepoint that fires on the holder side when a lock with waiters is released. This provides:

- Hold time estimation: when the holder's own acquisition was contended, its contention_end (acquisition) and contended_release can be correlated to measure how long the lock was held under contention.

Quoted 7 lines

- The holder's stack at release time, which may differ from what perf lock contention --lock-owner captures if the holder does significant work between the waiter's arrival and the unlock.

Note: for reader/writer locks, the tracepoint fires for every reader releasing while a writer is waiting, not only for the last reader.

Quoted 6 lines

Would it be better to reorder the patches? It would help with git bisectability as well. Move the refractoring work in patch 4 and 5 (excluding adding the tracepoints ofcourse) earlier, and then add all the tracepoints in the same commit at the end? It would help in the future with git blame to see where all the tracepoints were added as well.

Thanks for the suggestion, Usama.

I'd prefer to keep the current ordering: each refactoring commit is immediately followed by the commit that uses it. For example, queued_spin_release() is factored out right before the commit that adds the tracepoint to spinning locks. This makes the motivation for the refactoring clear and should also ease the review since the context is still fresh.

Bisectability should be fine as-is, each commit compiles and works independently, since the refactoring patches do not introduce behavioral changes on their own.

#19 lore raw
In reply to Dmitry Ilvokhin

Hi,

Just a gentle ping on this series.

I'd appreciate any feedback. The spinning locks part (patch 5) would particularly benefit from review.

Peter, Steven, any thoughts on that part would be greatly appreciated.