Skip to contentSkip to conversation

locking: contended_release tracepoint instrumentation

RFC PATCH Superseded Dmitry Ilvokhin 4 participants 14 messages Mar 4 – Mar 10

Subsystems (4)

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

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.

The tracepoint is placed exclusively in slowpath unlock paths, so there is no performance impact on the uncontended fast path and expected minimal impact on binary size.

Dmitry Ilvokhin (3):
locking: Add contended_release tracepoint locking/percpu-rwsem: Extract __percpu_up_read_slowpath()
locking: Wire up contended_release tracepoint

 include/linux/percpu-rwsem.h  | 15 +++------------
 include/trace/events/lock.h   | 17 +++++++++++++++++
 kernel/locking/mutex.c        |  1 +
 kernel/locking/percpu-rwsem.c | 21 +++++++++++++++++++++
 kernel/locking/rtmutex.c      |  1 +
 kernel/locking/rwbase_rt.c    |  8 +++++++-
 kernel/locking/rwsem.c        |  9 +++++++--
 kernel/locking/semaphore.c    |  4 +++-
 8 files changed, 60 insertions(+), 16 deletions(-)
Signature · 1 line
2.47.3
#1 lore raw

RFC PATCH 1/3 locking: Add contended_release tracepoint

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. Subsequent patches wire this tracepoint into the individual lock implementations.

---
 include/trace/events/lock.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
include/trace/events/lock.h +17 -0
index 8e89baa3775f..4f28e41977ec 100644
@@ -138,6 +138,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 */
Signature · 1 line
2.47.3
#2 lore raw

Re: [PATCH RFC 1/3] locking: Add contended_release tracepoint

Quoted 9 lines · On Wed, 4 Mar 2026 16:56:15 +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. Subsequent patches wire this tracepoint into the individual lock implementations.

OK, but I don't recommend you to split the traceevent definition and its usage. Could you combine [1/3] and [3/3], so that we can understand why this event is introduced and where it is recorded by checking one commit?

Thank you,

Quoted 36 lines
---
 include/trace/events/lock.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
include/trace/events/lock.h +17 -0
index 8e89baa3775f..4f28e41977ec 100644
@@ -138,6 +138,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 */
Signature · 1 line
2.47.3
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
#3 lore raw

Re: [PATCH RFC 1/3] locking: Add contended_release tracepoint

Quoted 13 lines · Masami Hiramatsu
Quoted 9 lines · On Wed, 4 Mar 2026 16:56:15 +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. Subsequent patches wire this tracepoint into the individual lock implementations.

Quoted 4 lines

OK, but I don't recommend you to split the traceevent definition and its usage. Could you combine [1/3] and [3/3], so that we can understand why this event is introduced and where it is recorded by checking one commit?

Thanks, Masami, absolutely. I'll squash these commits.

#4 lore raw

RFC PATCH 2/3 locking/percpu-rwsem: Extract __percpu_up_read_slowpath()

In reply to Dmitry Ilvokhin

Move the percpu_up_read() slowpath out of the inline function into a new __percpu_up_read_slowpath() 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..89506895365c 100644
@@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 	return ret;
 }
 
+void __percpu_up_read_slowpath(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_slowpath(sem);
 	}
 	preempt_enable();
 }
kernel/locking/percpu-rwsem.c +18 -0
index ef234469baac..4190635458da 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_slowpath(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_slowpath);
Signature · 1 line
2.47.3
maintainer
#5 lore raw

Re: [PATCH RFC 2/3] locking/percpu-rwsem: Extract __percpu_up_read_slowpath()

Quoted 15 lines · Dmitry Ilvokhin

Move the percpu_up_read() slowpath out of the inline function into a new __percpu_up_read_slowpath() 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 +0 -0
index c8cb010d655e..89506895365c 100644

include/linux/percpu-rwsem.h static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem) · line 111

Quoted 5 lines
include/linux/percpu-rwsem.h +2 -0
@@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 	return ret;
 }
 
+void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem);
+

extern for consistency with all the other declarations in this header.

s/_slowpath//, the corresponding down function also doesn't have _slowpath on.

Quoted 3 lines
include/linux/percpu-rwsem.h +0 -0
 static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);
maintainer
#6 lore raw

Re: [PATCH RFC 2/3] locking/percpu-rwsem: Extract __percpu_up_read_slowpath()

Quoted 8 lines · On Wed, 4 Mar 2026 23:02:23 +0100 Peter Zijlstra <[email protected]> wrote:
Quoted 8 lines
include/linux/percpu-rwsem.h +0 -0
index c8cb010d655e..89506895365c 100644
@@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 	return ret;
 }
 
Quoted 4 lines

include/linux/percpu-rwsem.h static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem) · line 111

Quoted 2 lines
include/linux/percpu-rwsem.h +2 -0
+void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem);
+  

extern for consistency with all the other declarations in this header.

I wonder if a cleanup patch should be added to remove the "extern" from the other functions, as that tends to be the way things are going (hch just recommended it elsewhere).

Quoted 7 lines

s/_slowpath//, the corresponding down function also doesn't have _slowpath on.

Quoted 3 lines
include/linux/percpu-rwsem.h +0 -0
 static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);  

And since "slowpath" is more descriptive (and used in the rtmutex code), should that be added too?

-- Steve

maintainer
#7 lore raw

Re: [PATCH RFC 2/3] locking/percpu-rwsem: Extract __percpu_up_read_slowpath()

Quoted 11 lines · Steven Rostedt
Quoted 8 lines · On Wed, 4 Mar 2026 23:02:23 +0100 Peter Zijlstra <[email protected]> wrote:
Quoted 8 lines
include/linux/percpu-rwsem.h +0 -0
index c8cb010d655e..89506895365c 100644
@@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 	return ret;
 }
 
Quoted 8 lines
Quoted 4 lines

include/linux/percpu-rwsem.h static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem) · line 111

Quoted 2 lines
include/linux/percpu-rwsem.h +2 -0
+void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem);
+  

extern for consistency with all the other declarations in this header.

I wonder if a cleanup patch should be added to remove the "extern" from the other functions, as that tends to be the way things are going (hch just recommended it elsewhere).

Well, I rather like the extern. But yeah, I know hch does not agree.

Quoted 10 lines
Quoted 7 lines

s/_slowpath//, the corresponding down function also doesn't have _slowpath on.

Quoted 3 lines
include/linux/percpu-rwsem.h +0 -0
 static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);  

And since "slowpath" is more descriptive (and used in the rtmutex code), should that be added too?

It already has __ prefix, no point in making the name even longer for no real benefit.

#8 lore raw

Re: [PATCH RFC 2/3] locking/percpu-rwsem: Extract __percpu_up_read_slowpath()

In reply to Peter Zijlstra
Quoted 19 lines · Peter Zijlstra
Quoted 18 lines · Dmitry Ilvokhin

Move the percpu_up_read() slowpath out of the inline function into a new __percpu_up_read_slowpath() 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 +0 -0
index c8cb010d655e..89506895365c 100644
@@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 	return ret;
 }
 
Quoted 7 lines

include/linux/percpu-rwsem.h static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem) · line 111

Quoted 2 lines
include/linux/percpu-rwsem.h +2 -0
+void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem);
+

extern for consistency with all the other declarations in this header.

s/_slowpath//, the corresponding down function also doesn't have _slowpath on.

Thanks for the feedback, Peter. Applied both suggestions locally.

#9 lore raw

RFC PATCH 3/3 locking: Wire up contended_release tracepoint

In reply to Dmitry Ilvokhin

Add trace_contended_release() calls to the slowpath unlock paths of sleepable locks: mutex, rtmutex, semaphore, rwsem, percpu-rwsem, and RT-specific rwbase locks. Each call site fires only when there are blocked waiters being woken, except percpu_up_write() which always wakes via __wake_up().

---
 kernel/locking/mutex.c        | 1 +
 kernel/locking/percpu-rwsem.c | 3 +++
 kernel/locking/rtmutex.c      | 1 +
 kernel/locking/rwbase_rt.c    | 8 +++++++-
 kernel/locking/rwsem.c        | 9 +++++++--
 kernel/locking/semaphore.c    | 4 +++-
 6 files changed, 22 insertions(+), 4 deletions(-)
kernel/locking/mutex.c +1 -0
index c867f6c15530..54ca045987a2 100644
@@ -970,6 +970,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 
 		next = waiter->task;
 
+		trace_contended_release(lock);
 		debug_mutex_wake_waiter(lock, waiter);
 		__clear_task_blocked_on(next, lock);
 		wake_q_add(&wake_q, next);
kernel/locking/percpu-rwsem.c +3 -0
index 4190635458da..0f2e8e63d252 100644
@@ -263,6 +263,8 @@ void percpu_up_write(struct percpu_rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, _RET_IP_);
 
+	trace_contended_release(sem);
+
 	/*
 	 * Signal the writer is done, no fast path yet.
 	 *
@@ -297,6 +299,7 @@ void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem)
 	 * writer.
 	 */
 	smp_mb(); /* B matches C */
+	trace_contended_release(sem);
 	/*
 	 * In other words, if they see our decrement (presumably to
 	 * aggregate zero, as that is the only time it matters) they
kernel/locking/rtmutex.c +1 -0
index c80902eacd79..e0873f0ed982 100644
@@ -1457,6 +1457,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 +7 -1
index 9f4322c07486..42f3658c0059 100644
@@ -162,8 +162,10 @@ static void __sched __rwbase_read_unlock(struct rwbase_rt *rwb,
 	 * worst case which can happen is a spurious wakeup.
 	 */
 	owner = rt_mutex_owner(rtm);
-	if (owner)
+	if (owner) {
+		trace_contended_release(rwb);
 		rt_mutex_wake_q_add_task(&wqh, owner, state);
+	}
 
 	/* Pairs with the preempt_enable in rt_mutex_wake_up_q() */
 	preempt_disable();
@@ -204,6 +206,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);
 	__rwbase_write_unlock(rwb, WRITER_BIAS, flags);
 }
 
@@ -213,6 +217,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (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 +7 -2
index 24df4d98f7d2..4e61dc0bb045 100644
@@ -1360,6 +1360,7 @@ static inline void __up_read(struct rw_semaphore *sem)
 	if (unlikely((tmp & (RWSEM_LOCK_MASK|RWSEM_FLAG_WAITERS)) ==
 		      RWSEM_FLAG_WAITERS)) {
 		clear_nonspinnable(sem);
+		trace_contended_release(sem);
 		rwsem_wake(sem);
 	}
 	preempt_enable();
@@ -1383,8 +1384,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();
 }
 
@@ -1407,8 +1410,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 +3 -1
index 3ef032e22f7e..3cef5ba88f7e 100644
@@ -231,8 +231,10 @@ void __sched up(struct semaphore *sem)
 	else
 		__up(sem, &wake_q);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
-	if (!wake_q_empty(&wake_q))
+	if (!wake_q_empty(&wake_q)) {
+		trace_contended_release(sem);
 		wake_up_q(&wake_q);
+	}
 }
 EXPORT_SYMBOL(up);
 
Signature · 1 line
2.47.3
maintainer
#10 lore raw

Re: [PATCH RFC 3/3] locking: Wire up contended_release tracepoint

kernel/locking/rwbase_rt.c static inline void rwbase_write_unlock(struct rwbase_rt *rwb) · line 210

Quoted 6 lines · On Wed, 4 Mar 2026 16:56:17 +0000 Dmitry Ilvokhin <[email protected]> wrote:
kernel/locking/rwbase_rt.c +2 -0
@@ -204,6 +206,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);

Hmm, if statements should never be used just for tracepoints without a static branch. The above should be:

	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
		trace_contended_release(rwb);

The above "trace_contened_release_enabled()" is a static_branch where it turns the if statement into a nop when the tracepoint is not enabled, and a jmp when it is.

kernel/locking/rwbase_rt.c static inline void rwbase_write_downgrade(struct rwbase_rt *rwb) · line 221

Quoted 9 lines
kernel/locking/rwbase_rt.c +2 -0
 	__rwbase_write_unlock(rwb, WRITER_BIAS, flags);
 }
 
@@ -213,6 +217,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);

Same here.

-- Steve

Quoted 55 lines
kernel/locking/rwbase_rt.c +0 -0
 	/* Release it and account current as reader */
 	__rwbase_write_unlock(rwb, WRITER_BIAS - 1, flags);
 }
kernel/locking/rwsem.c +7 -2
index 24df4d98f7d2..4e61dc0bb045 100644
@@ -1360,6 +1360,7 @@ static inline void __up_read(struct rw_semaphore *sem)
 	if (unlikely((tmp & (RWSEM_LOCK_MASK|RWSEM_FLAG_WAITERS)) ==
 		      RWSEM_FLAG_WAITERS)) {
 		clear_nonspinnable(sem);
+		trace_contended_release(sem);
 		rwsem_wake(sem);
 	}
 	preempt_enable();
@@ -1383,8 +1384,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();
 }
 
@@ -1407,8 +1410,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 +3 -1
index 3ef032e22f7e..3cef5ba88f7e 100644
@@ -231,8 +231,10 @@ void __sched up(struct semaphore *sem)
 	else
 		__up(sem, &wake_q);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
-	if (!wake_q_empty(&wake_q))
+	if (!wake_q_empty(&wake_q)) {
+		trace_contended_release(sem);
 		wake_up_q(&wake_q);
+	}
 }
 EXPORT_SYMBOL(up);
 
maintainer
#11 lore raw

Re: [PATCH RFC 3/3] locking: Wire up contended_release tracepoint

Quoted 13 lines · Steven Rostedt

kernel/locking/rwbase_rt.c static inline void rwbase_write_unlock(struct rwbase_rt *rwb) · line 210

Quoted 6 lines · On Wed, 4 Mar 2026 16:56:17 +0000 Dmitry Ilvokhin <[email protected]> wrote:
kernel/locking/rwbase_rt.c +2 -0
@@ -204,6 +206,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);

Hmm, if statements should never be used just for tracepoints without a static branch. The above should be:

Quoted 3 lines
	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
		trace_contended_release(rwb);

I still wish you would accept:

	if (trace_foo_enabled() && foo)
		__do_trace_foo();

The compilers can't optimize the static branches and thus you'll get it twice for no reason.

I really wish they would just accept __pure, but alas.

maintainer
#12 lore raw

Re: [PATCH RFC 3/3] locking: Wire up contended_release tracepoint

Quoted 9 lines · On Thu, 5 Mar 2026 18:42:23 +0100 Peter Zijlstra <[email protected]> wrote:

I still wish you would accept:

	if (trace_foo_enabled() && foo)
		__do_trace_foo();

The compilers can't optimize the static branches and thus you'll get it twice for no reason.

I really wish they would just accept __pure, but alas.

Makes sense, and that could probably be done. It shouldn't be too hard to do. If I find some time I could look at it, or perhaps someone lurking on this thread could possibly give it a try! (I may even Cc some people that want to learn this code).

-- Steve

#13 lore raw

Re: [PATCH RFC 3/3] locking: Wire up contended_release tracepoint

In reply to Steven Rostedt
Quoted 13 lines · Steven Rostedt

kernel/locking/rwbase_rt.c static inline void rwbase_write_unlock(struct rwbase_rt *rwb) · line 210

Quoted 6 lines · On Wed, 4 Mar 2026 16:56:17 +0000 Dmitry Ilvokhin <[email protected]> wrote:
kernel/locking/rwbase_rt.c +2 -0
@@ -204,6 +206,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
+	if (rt_mutex_has_waiters(rtm))
+		trace_contended_release(rwb);

Hmm, if statements should never be used just for tracepoints without a static branch. The above should be:

Quoted 6 lines
	if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
		trace_contended_release(rwb);

The above "trace_contened_release_enabled()" is a static_branch where it turns the if statement into a nop when the tracepoint is not enabled, and a jmp when it is.

Thanks for catching this, Steven. Fixed locally.