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/cover.1773858853.git.d@ilvokhin.com/
v2: https://lore.kernel.org/all/cover.1773164180.git.d@ilvokhin.com/
RFC: https://lore.kernel.org/all/cover.1772642407.git.d@ilvokhin.com/
[1]: https://lore.kernel.org/all/aa7G1nD7Rd9F4eBH@casper.infradead.org/
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(-)
--
2.52.0
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
Acked-by: Usama Arif <usama.arif@linux.dev>
---
include/linux/percpu-rwsem.h | 15 +++------------
kernel/locking/percpu-rwsem.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+), 12 deletions(-)
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
@@ -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-releaseaqueuedspinlock+*queued_spin_release-releaseaqueuedspinlock*@lock:Pointertoqueuedspinlockstructure*/-staticinlinevoidqueued_spin_unlock(structqspinlock*lock)+staticinlinevoidqueued_spin_release(structqspinlock*lock){/* This could be optimised with ARCH_HAS_MMIOWB */mmiowb();
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
@@ -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 */
@@ -214,6 +218,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)unsignedlongflags;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);}
From: Matthew Wilcox <willy@infradead.org> Date: 2026-03-26 15:55:47
On Thu, Mar 26, 2026 at 03:09:59PM +0000, Dmitry Ilvokhin 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.
- 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?
From: Steven Rostedt <rostedt@goodmis.org> Date: 2026-03-26 16:46:22
On Thu, 26 Mar 2026 15:55:21 +0000
Matthew Wilcox [off-list ref] wrote:
quoted
- 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?
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
On Thu, Mar 26, 2026 at 03:55:21PM +0000, Matthew Wilcox wrote:
On Thu, Mar 26, 2026 at 03:09:59PM +0000, Dmitry Ilvokhin wrote:
quoted
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.
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.
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.
From: Usama Arif <usama.arif@linux.dev> Date: 2026-03-31 10:27:24
On Thu, 26 Mar 2026 15:09:59 +0000 Dmitry Ilvokhin [off-list ref] 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.
- 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.
From: Usama Arif <usama.arif@linux.dev> Date: 2026-03-31 10:35:13
On Thu, 26 Mar 2026 15:10:02 +0000 Dmitry Ilvokhin [off-list ref] wrote:
quoted hunk
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
@@ -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 */
On Tue, Mar 31, 2026 at 03:34:50AM -0700, Usama Arif wrote:
On Thu, 26 Mar 2026 15:10:02 +0000 Dmitry Ilvokhin [off-list ref] wrote:
quoted
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
@@ -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 */
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.
On Tue, Mar 31, 2026 at 03:27:03AM -0700, Usama Arif wrote:
On Thu, 26 Mar 2026 15:09:59 +0000 Dmitry Ilvokhin [off-list ref] wrote:
quoted
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.
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.
From: Usama Arif <usama.arif@linux.dev> Date: 2026-03-31 14:12:01
On 31/03/2026 15:16, Dmitry Ilvokhin wrote:
On Tue, Mar 31, 2026 at 03:34:50AM -0700, Usama Arif wrote:
quoted
On Thu, 26 Mar 2026 15:10:02 +0000 Dmitry Ilvokhin [off-list ref] wrote:
quoted
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
@@ -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 */
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.
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.
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.
From: "Paul E. McKenney" <paulmck@kernel.org> Date: 2026-04-14 23:09:30
On Thu, Mar 26, 2026 at 03:10:02PM +0000, Dmitry Ilvokhin 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.
Signed-off-by: Dmitry Ilvokhin <redacted>
Looks plausible:
Acked-by: Paul E. McKenney <paulmck@kernel.org>
@@ -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 */
@@ -214,6 +218,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)unsignedlongflags;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);}
From: "Paul E. McKenney" <paulmck@kernel.org> Date: 2026-04-14 23:11:51
On Thu, Mar 26, 2026 at 03:10:03PM +0000, Dmitry Ilvokhin wrote:
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
@@ -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-releaseaqueuedspinlock+*queued_spin_release-releaseaqueuedspinlock*@lock:Pointertoqueuedspinlockstructure*/-staticinlinevoidqueued_spin_unlock(structqspinlock*lock)+staticinlinevoidqueued_spin_release(structqspinlock*lock){/* This could be optimised with ARCH_HAS_MMIOWB */mmiowb();
From: "Paul E. McKenney" <paulmck@kernel.org> Date: 2026-04-14 23:20:27
On Thu, Mar 26, 2026 at 03:10:04PM +0000, Dmitry Ilvokhin wrote:
quoted hunk
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.
Signed-off-by: Dmitry Ilvokhin <redacted>
---
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(-)
On Tue, Apr 14, 2026 at 04:20:26PM -0700, Paul E. McKenney wrote:
[...]
quoted
+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);
+}
Shouldn't this refactoring be its own separate patch, similar to 4/5?
That would probably clean up this diff a bit.
quoted
+
+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);
And the same here, so one patch for interposing __queued_read_unlock()
and another for interposing __queued_write_unlock().
[...]
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