locking/qspinlock: Add contended_release tracepoint
cover locking/qspinlock: Add contended_release tracepoint
The contended_release tracepoint landed in v7.2-rc2 for sleeping locks (4f070ccb4dc4 "locking: Add contended_release tracepoint to sleepable locks"). Spinlock support was dropped from that series. This one adds it for queued spinlocks.
The existing contention_begin/contention_end tracepoints fire on the waiter side. The holder's identity and stack can be captured at contention_begin time (e.g. perf lock contention --lock-owner), but only for locks with an owner field to read: mutex and rwsem. qspinlock has none, so a contended spinlock cannot be attributed to its holder at all. Even where the owner can be read, it reflects the holder's state when a waiter arrives, not when the lock is released.
This series adds a contended_release tracepoint to qspinlock 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 for spinlocks is not available by any other means.
The unlock path might be quite hot, so the tracepoint is made as cheap as possible, to keep it usable in production:
- x86 with PARAVIRT_SPINLOCKS=y, which is what distributions ship, swaps the unlock implementation via static_call() when the tracepoint is enabled. The disabled path is byte-identical to today's: the same inline movb, no NOP and no call.
- Everywhere else a static-branch check is compiled into queued_spin_unlock(). On x86_64 that is a single NOP on the executed path, with the call to the traced helper emitted out of line and unreachable while the tracepoint is off. On other architectures a few more instructions to manage a stack frame land on the executed path too, so the generic path sits behind CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE (default n).
Costs and measurements are in the individual changelogs. Briefly, no throughput or latency change is measurable on either x86_64 or arm64 with QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE=y.
Tested: x86_64 with PARAVIRT_SPINLOCKS=y and =n, arm64, tracepoint on and off, disassembly checked in both states, locktorture with tracepoint on and off.
Not covered: qrwlock, and architectures with fully custom qspinlock implementations (e.g. PowerPC). The stack frame managing instructions on arm64 should be avoidable, but that is not done in this patchset.
Patch 1 is Peter's draft from [1] and is missing his Signed-off-by. Peter, please add it if you are happy with the patch.
[1]: https://lore.kernel.org/all/[email protected]/
Dmitry Ilvokhin (4):
locking: Factor out queued_spin_release() locking/qspinlock: Add contended_release tracepoint tracing/lock: Use TRACE_EVENT_FN() for contended_release x86/paravirt: Trace contended_release on unlock
Peter Zijlstra (1):
x86/paravirt: Use static_call() for the paravirt spinlock ops
arch/mips/include/asm/spinlock.h | 6 +-- arch/x86/hyperv/hv_spinlock.c | 4 +- arch/x86/include/asm/cpufeatures.h | 1 - arch/x86/include/asm/paravirt-spinlock.h | 21 +++++--- arch/x86/kernel/kvm.c | 5 +- arch/x86/kernel/paravirt-spinlocks.c | 63 +++++++++++++++++++++--- arch/x86/kernel/static_call.c | 27 ++++++++++ arch/x86/xen/spinlock.c | 5 +- include/asm-generic/qspinlock.h | 38 ++++++++++++-- include/trace/events/lock.h | 10 +++- kernel/Kconfig.locks | 20 ++++++++ kernel/locking/mutex.c | 4 ++ kernel/locking/qspinlock.c | 22 +++++++++ tools/arch/x86/include/asm/cpufeatures.h | 1 - 14 files changed, 195 insertions(+), 32 deletions(-)
base-commit: 5e601ab3615c86be7c4068ce992f94654693a032
Signature · 1 line
2.53.0-Meta
PATCH 1/5 x86/paravirt: Use static_call() for the paravirt spinlock ops
From: Peter Zijlstra <[email protected]>
queued_spin_lock_slowpath() and queued_spin_unlock() are dispatched through pv_ops_lock via the paravirt-ops ALTERNATIVE machinery, which picks the target (native inline store / hypervisor call) once at boot and cannot change at runtime.
Convert both to static_call(). The site becomes a direct call patched in place (one byte smaller), and on native the unlock still collapses to the inline "movb $0, (%rdi)" store, so the fast path is unchanged.
Unlike the ALTERNATIVE mechanism, a static_call() target can also be updated at runtime via static_call_update(). This is a prerequisite for the contended_release tracepoint, which has to swap in a traced unlock while the system is running.
[ ilvokhin: commit message; fix PARAVIRT_SPINLOCKS=n build; teach
__static_call_validate() about the inline unlock insn; make the slowpath site module-safe: static_call_mod() + EXPORT_STATIC_CALL_TRAMP(); pass @lock to the callee-save unlock, fixing a boot hang under CALL_DEPTH_TRACKING. Boot tested native + KVM PV guest. ]
--- arch/x86/hyperv/hv_spinlock.c | 4 ++-- arch/x86/include/asm/cpufeatures.h | 1 - arch/x86/include/asm/paravirt-spinlock.h | 19 +++++++++++------ arch/x86/kernel/kvm.c | 5 ++--- arch/x86/kernel/paravirt-spinlocks.c | 12 +++++------ arch/x86/kernel/static_call.c | 27 ++++++++++++++++++++++++ arch/x86/xen/spinlock.c | 5 ++--- tools/arch/x86/include/asm/cpufeatures.h | 1 - 8 files changed, 51 insertions(+), 23 deletions(-)
arch/x86/hyperv/hv_spinlock.c +2 -2
@@ -78,8 +78,8 @@ void __init hv_init_spinlocks(void)pr_info ("PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock =PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = hv_qlock_wait; pv_ops_lock.kick = hv_qlock_kick; pv_ops_lock.vcpu_is_preempted =PV_CALLEE_SAVE (hv_vcpu_is_preempted);
arch/x86/include/asm/cpufeatures.h +0 -1
@@ -225,7 +225,6 @@#define X86_FEATURE_EPT_AD ( 8*32+17)/* "ept_ad" Intel Extended Page Table access-dirty bit */ #define X86_FEATURE_VMCALL ( 8*32+18)/* Hypervisor supports the VMCALL instruction */ #define X86_FEATURE_VMW_VMMCALL ( 8*32+19)/* VMware prefers VMMCALL hypercall instruction */ -#define X86_FEATURE_PVUNLOCK ( 8*32+20)/* PV unlock function */ #define X86_FEATURE_VCPUPREEMPT ( 8*32+21)/* PV vcpu_is_preempted function */ #define X86_FEATURE_TDX_GUEST ( 8*32+22)/* "tdx_guest" Intel Trust Domain Extensions Guest */
arch/x86/include/asm/paravirt-spinlock.h +12 -7
@@ -3,6 +3,7 @@#define _ASM_X86_PARAVIRT_SPINLOCK_H#include <asm/paravirt_types.h> +#include <linux/static_call_types.h> #ifdef CONFIG_SMP #include <asm/spinlock_types.h>
@@ -11,9 +12,6 @@struct qspinlock ;struct pv_lock_ops { -void (*queued_spin_lock_slowpath)(struct qspinlock *lock,u32 val); -struct paravirt_callee_save queued_spin_unlock; -void (*wait)(u8 *ptr,u8 val);void (*kick)(int cpu);
@@ -26,20 +24,27 @@ extern struct pv_lock_ops pv_ops_lock;extern void native_queued_spin_lock_slowpath (struct qspinlock *lock,u32 val);extern void __pv_init_lock_hash (void );extern void __pv_queued_spin_lock_slowpath (struct qspinlock *lock,u32 val); +extern void __raw_callee_save___native_queued_spin_unlock (struct qspinlock *lock);extern void __raw_callee_save___pv_queued_spin_unlock (struct qspinlock *lock);extern bool nopvspin; +DECLARE_STATIC_CALL (queued_spin_lock_slowpath, native_queued_spin_lock_slowpath); +DECLARE_STATIC_CALL (queued_spin_unlock, __raw_callee_save___native_queued_spin_unlock); + static __always_inline void pv_queued_spin_lock_slowpath(struct qspinlock *lock,u32 val) { -PVOP_VCALL2 (pv_ops_lock, queued_spin_lock_slowpath, lock, val); +static_call_mod (queued_spin_lock_slowpath)(lock, val); } static __always_inline void pv_queued_spin_unlock(struct qspinlock *lock) { -PVOP_ALT_VCALLEE1 (pv_ops_lock, queued_spin_unlock, lock, -"movb $0, (%%" _ASM_ARG1")" , -ALT_NOT (X86_FEATURE_PVUNLOCK )); +PVOP_CALL_ARGS ; +__STATIC_CALL_MOD_ADDRESSABLE (queued_spin_unlock); + asm volatile ("call " STATIC_CALL_TRAMP_STR(queued_spin_unlock) + : PVOP_VCALLEE_CLOBBERS, ASM_CALL_CONSTRAINT + : PVOP_CALL_ARG1(lock) + : "memory", "cc"); } static __always_inline bool pv_vcpu_is_preempted(long cpu)
arch/x86/kernel/kvm.c +2 -3
@@ -1136,9 +1136,8 @@ void __init kvm_spinlock_init(void)pr_info ("PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock = -PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = kvm_wait; pv_ops_lock.kick = kvm_kick_cpu;
arch/x86/kernel/paravirt-spinlocks.c +6 -6
@@ -25,9 +25,14 @@ __visible void __native_queued_spin_unlock(struct qspinlock *lock) }PV_CALLEE_SAVE_REGS_THUNK (__native_queued_spin_unlock); +DEFINE_STATIC_CALL (queued_spin_lock_slowpath, native_queued_spin_lock_slowpath); +EXPORT_STATIC_CALL_TRAMP (queued_spin_lock_slowpath); +DEFINE_STATIC_CALL (queued_spin_unlock, __raw_callee_save___native_queued_spin_unlock); +EXPORT_STATIC_CALL_TRAMP (queued_spin_unlock); +bool pv_is_native_spin_unlock (void ) { -return pv_ops_lock.queued_spin_unlock.func == +return static_call_query (queued_spin_unlock) == __raw_callee_save___native_queued_spin_unlock; }
@@ -45,16 +50,11 @@ bool pv_is_native_vcpu_is_preempted(void)void __init paravirt_set_cap (void ) { -if (!pv_is_native_spin_unlock ()) -setup_force_cpu_cap (X86_FEATURE_PVUNLOCK ); -if (!pv_is_native_vcpu_is_preempted ())setup_force_cpu_cap (X86_FEATURE_VCPUPREEMPT ); }struct pv_lock_ops pv_ops_lock = { - .queued_spin_lock_slowpath = native_queued_spin_lock_slowpath, - .queued_spin_unlock =PV_CALLEE_SAVE (__native_queued_spin_unlock), .wait = paravirt_nop, .kick = paravirt_nop, .vcpu_is_preempted =PV_CALLEE_SAVE (__native_vcpu_is_preempted),
arch/x86/kernel/static_call.c +27 -0
@@ -4,6 +4,12 @@#include <linux/bug.h> #include <asm/text-patching.h> +/* Declared locally to avoid pulling asm/paravirt-spinlock.h header. */ +#ifdef CONFIG_PARAVIRT_SPINLOCKS +struct qspinlock ; +void __raw_callee_save___native_queued_spin_unlock (struct qspinlock *lock); +#endif +enum insn_type {CALL =0 ,/* site call */ NOP =1 ,/* site cond-call */
@@ -31,6 +37,17 @@ static const u8 retinsn[] = { RET_INSN_OPCODE, 0xcc, 0xcc, 0xcc, 0xcc };*/ static const u8 warninsn[] = {0x67 ,0x48 ,0x0f ,0xb9 ,0x3a }; +#ifdef CONFIG_PARAVIRT_SPINLOCKS +/* +* ds ds movb $0, (_ASM_ARG1) +*/ +#ifdef CONFIG_64BIT +static const u8 unlockinsn[] = {0x3e ,0x3e ,0xc6 ,0x07 ,0x00 }; +#else +static const u8 unlockinsn[] = {0x3e ,0x3e ,0xc6 ,0x00 ,0x00 }; +#endif +#endif +static u8 __is_Jcc (u8 *insn)/* Jcc.d32 */ {u8 ret =0 ;
@@ -78,6 +95,12 @@ static void __ref __static_call_transform(void *insn, enum insn_type type, emulate = code; code = &warninsn; } +#ifdef CONFIG_PARAVIRT_SPINLOCKS +if (func == &__raw_callee_save___native_queued_spin_unlock) { + emulate = code; + code = &unlockinsn; + } +#endif break ;case NOP :
@@ -139,6 +162,10 @@ static void __static_call_validate(u8 *insn, bool tail, bool tramp) !memcmp (insn, xor5rax,5 ) || !memcmp(insn, warninsn, 5)) return; +#ifdef CONFIG_PARAVIRT_SPINLOCKS +if (!memcmp (insn, unlockinsn,5 )) +return ; +#endif }/*
arch/x86/xen/spinlock.c +2 -3
@@ -134,9 +134,8 @@ void __init xen_init_spinlocks(void)printk (KERN_DEBUG "xen: PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock = -PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = xen_qlock_wait; pv_ops_lock.kick = xen_qlock_kick; pv_ops_lock.vcpu_is_preempted =PV_CALLEE_SAVE (xen_vcpu_stolen);
tools/arch/x86/include/asm/cpufeatures.h +0 -1
@@ -225,7 +225,6 @@#define X86_FEATURE_EPT_AD ( 8*32+17)/* "ept_ad" Intel Extended Page Table access-dirty bit */ #define X86_FEATURE_VMCALL ( 8*32+18)/* Hypervisor supports the VMCALL instruction */ #define X86_FEATURE_VMW_VMMCALL ( 8*32+19)/* VMware prefers VMMCALL hypercall instruction */ -#define X86_FEATURE_PVUNLOCK ( 8*32+20)/* PV unlock function */ #define X86_FEATURE_VCPUPREEMPT ( 8*32+21)/* PV vcpu_is_preempted function */ #define X86_FEATURE_TDX_GUEST ( 8*32+22)/* "tdx_guest" Intel Trust Domain Extensions Guest */
Signature · 1 line
2.53.0-Meta
Borislav Petkov
maintainerRe: [PATCH 1/5] x86/paravirt: Use static_call() for the paravirt spinlock ops
Quoted 23 lines · Dmitry Ilvokhin
From: Peter Zijlstra <[email protected]>
queued_spin_lock_slowpath()andqueued_spin_unlock()are dispatched throughpv_ops_lockvia the paravirt-ops ALTERNATIVE machinery, which picks the target (native inline store / hypervisor call) once at boot and cannot change at runtime.Convert both to
static_call(). The site becomes a direct call patched in place (one byte smaller), and on native the unlock still collapses to the inline "movb $0, (%rdi)" store, so the fast path is unchanged.Unlike the ALTERNATIVE mechanism, a
static_call()target can also be updated at runtime viastatic_call_update(). This is a prerequisite for thecontended_releasetracepoint, which has to swap in a traced unlock while the system is running.[ ilvokhin: commit message; fix PARAVIRT_SPINLOCKS=n build; teach
__static_call_validate()about the inline unlock insn; make the slowpath site module-safe:static_call_mod()+EXPORT_STATIC_CALL_TRAMP(); pass @lock to the callee-save unlock, fixing a boot hang underCALL_DEPTH_TRACKING. Boot tested native + KVM PV guest. ]
Quoted 3 lines
Co-developed-by Dmitry Ilvokhin <[email protected]>Signed-off-by Dmitry Ilvokhin <[email protected]>
This needs Peter's SOB.
Quoted 31 lines
--- arch/x86/hyperv/hv_spinlock.c | 4 ++-- arch/x86/include/asm/cpufeatures.h | 1 - arch/x86/include/asm/paravirt-spinlock.h | 19 +++++++++++------ arch/x86/kernel/kvm.c | 5 ++--- arch/x86/kernel/paravirt-spinlocks.c | 12 +++++------ arch/x86/kernel/static_call.c | 27 ++++++++++++++++++++++++ arch/x86/xen/spinlock.c | 5 ++--- tools/arch/x86/include/asm/cpufeatures.h | 1 - 8 files changed, 51 insertions(+), 23 deletions(-)arch/x86/hyperv/hv_spinlock.c +2
-2@@ -78,8 +78,8 @@ void __init hv_init_spinlocks(void)pr_info ("PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock =PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = hv_qlock_wait; pv_ops_lock.kick = hv_qlock_kick; pv_ops_lock.vcpu_is_preempted =PV_CALLEE_SAVE (hv_vcpu_is_preempted);arch/x86/include/asm/cpufeatures.h +0
-0
arch/x86/include/asm/cpufeatures.h line 228
Quoted 4 lines
arch/x86/include/asm/cpufeatures.h +0
-1@@ -225,7 +225,6 @@#define X86_FEATURE_EPT_AD ( 8*32+17)/* "ept_ad" Intel Extended Page Table access-dirty bit */ #define X86_FEATURE_VMCALL ( 8*32+18)/* Hypervisor supports the VMCALL instruction */ #define X86_FEATURE_VMW_VMMCALL ( 8*32+19)/* VMware prefers VMMCALL hypercall instruction */ -#define X86_FEATURE_PVUNLOCK ( 8*32+20)/* PV unlock function */
No, do:
/* free: was #define X86_FEATURE_PVUNLOCK ( 8*32+20) /* PV unlock function */
so that we can reuse it by finding it easier.
Quoted 2 lines
arch/x86/include/asm/cpufeatures.h +0
-0#define X86_FEATURE_VCPUPREEMPT ( 8*32+21)/* PV vcpu_is_preempted function */ #define X86_FEATURE_TDX_GUEST ( 8*32+22)/* "tdx_guest" Intel Trust Domain Extensions Guest */
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquettePeter Zijlstra
maintainerRe: [PATCH 1/5] x86/paravirt: Use static_call() for the paravirt spinlock ops
Quoted 24 lines · Borislav Petkov
Quoted 23 lines · Dmitry Ilvokhin
From: Peter Zijlstra <[email protected]>
queued_spin_lock_slowpath()andqueued_spin_unlock()are dispatched throughpv_ops_lockvia the paravirt-ops ALTERNATIVE machinery, which picks the target (native inline store / hypervisor call) once at boot and cannot change at runtime.Convert both to
static_call(). The site becomes a direct call patched in place (one byte smaller), and on native the unlock still collapses to the inline "movb $0, (%rdi)" store, so the fast path is unchanged.Unlike the ALTERNATIVE mechanism, a
static_call()target can also be updated at runtime viastatic_call_update(). This is a prerequisite for thecontended_releasetracepoint, which has to swap in a traced unlock while the system is running.[ ilvokhin: commit message; fix PARAVIRT_SPINLOCKS=n build; teach
__static_call_validate()about the inline unlock insn; make the slowpath site module-safe:static_call_mod()+EXPORT_STATIC_CALL_TRAMP(); pass @lock to the callee-save unlock, fixing a boot hang underCALL_DEPTH_TRACKING. Boot tested native + KVM PV guest. ]
Quoted 5 lines
Quoted 3 lines
Co-developed-by Dmitry Ilvokhin <[email protected]>Signed-off-by Dmitry Ilvokhin <[email protected]>This needs Peter's SOB.
Yeah, that got fixed when I applied it ;-)
Quoted 38 lines
Quoted 31 lines
--- arch/x86/hyperv/hv_spinlock.c | 4 ++-- arch/x86/include/asm/cpufeatures.h | 1 - arch/x86/include/asm/paravirt-spinlock.h | 19 +++++++++++------ arch/x86/kernel/kvm.c | 5 ++--- arch/x86/kernel/paravirt-spinlocks.c | 12 +++++------ arch/x86/kernel/static_call.c | 27 ++++++++++++++++++++++++ arch/x86/xen/spinlock.c | 5 ++--- tools/arch/x86/include/asm/cpufeatures.h | 1 - 8 files changed, 51 insertions(+), 23 deletions(-)arch/x86/hyperv/hv_spinlock.c +2
-2@@ -78,8 +78,8 @@ void __init hv_init_spinlocks(void)pr_info ("PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock =PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = hv_qlock_wait; pv_ops_lock.kick = hv_qlock_kick; pv_ops_lock.vcpu_is_preempted =PV_CALLEE_SAVE (hv_vcpu_is_preempted);arch/x86/include/asm/cpufeatures.h +0
-0arch/x86/include/asm/cpufeatures.h line 228
Quoted 4 lines
arch/x86/include/asm/cpufeatures.h +0
-1@@ -225,7 +225,6 @@#define X86_FEATURE_EPT_AD ( 8*32+17)/* "ept_ad" Intel Extended Page Table access-dirty bit */ #define X86_FEATURE_VMCALL ( 8*32+18)/* Hypervisor supports the VMCALL instruction */ #define X86_FEATURE_VMW_VMMCALL ( 8*32+19)/* VMware prefers VMMCALL hypercall instruction */ -#define X86_FEATURE_PVUNLOCK ( 8*32+20)/* PV unlock function */ No, do:
Quoted 3 lines
/* free: was #define
X86_FEATURE_PVUNLOCK( 8*32+20) /* PV unlock function */so that we can reuse it by finding it easier.
Sure, I can do that.
[tip: locking/core] x86/paravirt: Use static_call() for the paravirt spinlock ops
In reply to Dmitry IlvokhinThe following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: fc6ad5eadcd1694d2f1ee2717c8024efd8152e20 Gitweb: https://git.kernel.org/tip/fc6ad5eadcd1694d2f1ee2717c8024efd8152e20 Author: Peter Zijlstra <[email protected]> AuthorDate: Tue, 04 Aug 2026 07:15:41 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 07 Aug 2026 17:58:09 +02:00
x86/paravirt: Use static_call() for the paravirt spinlock ops
queued_spin_lock_slowpath() and queued_spin_unlock() are dispatched through pv_ops_lock via the paravirt-ops ALTERNATIVE machinery, which picks the target (native inline store / hypervisor call) once at boot and cannot change at runtime.
Convert both to static_call(). The site becomes a direct call patched in place (one byte smaller), and on native the unlock still collapses to the inline "movb $0, (%rdi)" store, so the fast path is unchanged.
Unlike the ALTERNATIVE mechanism, a static_call() target can also be updated at runtime via static_call_update(). This is a prerequisite for the contended_release tracepoint, which has to swap in a traced unlock while the system is running.
[ ilvokhin: commit message; fix PARAVIRT_SPINLOCKS=n build; teach
__static_call_validate() about the inline unlock insn; make the slowpath site module-safe: static_call_mod() + EXPORT_STATIC_CALL_TRAMP(); pass @lock to the callee-save unlock, fixing a boot hang under CALL_DEPTH_TRACKING. Boot tested native + KVM PV guest. ]
--- arch/x86/hyperv/hv_spinlock.c | 4 +-- arch/x86/include/asm/cpufeatures.h | 2 +- arch/x86/include/asm/paravirt-spinlock.h | 19 ++++++++++------ arch/x86/kernel/kvm.c | 5 +--- arch/x86/kernel/paravirt-spinlocks.c | 12 +++++----- arch/x86/kernel/static_call.c | 27 +++++++++++++++++++++++- arch/x86/xen/spinlock.c | 5 +--- tools/arch/x86/include/asm/cpufeatures.h | 2 +- 8 files changed, 53 insertions(+), 23 deletions(-)
arch/x86/hyperv/hv_spinlock.c +2 -2
@@ -78,8 +78,8 @@ void __init hv_init_spinlocks(void)pr_info ("PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock =PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = hv_qlock_wait; pv_ops_lock.kick = hv_qlock_kick; pv_ops_lock.vcpu_is_preempted =PV_CALLEE_SAVE (hv_vcpu_is_preempted);
arch/x86/include/asm/cpufeatures.h +1 -1
@@ -225,7 +225,7 @@#define X86_FEATURE_EPT_AD ( 8*32+17)/* "ept_ad" Intel Extended Page Table access-dirty bit */ #define X86_FEATURE_VMCALL ( 8*32+18)/* Hypervisor supports the VMCALL instruction */ #define X86_FEATURE_VMW_VMMCALL ( 8*32+19)/* VMware prefers VMMCALL hypercall instruction */ -#define X86_FEATURE_PVUNLOCK ( 8*32+20)/* PV unlock function */ +// free: was #define X86_FEATURE_PVUNLOCK ( 8*32+20) /* PV unlock function */ #define X86_FEATURE_VCPUPREEMPT ( 8*32+21)/* PV vcpu_is_preempted function */ #define X86_FEATURE_TDX_GUEST ( 8*32+22)/* "tdx_guest" Intel Trust Domain Extensions Guest */
arch/x86/include/asm/paravirt-spinlock.h +12 -7
@@ -3,6 +3,7 @@#define _ASM_X86_PARAVIRT_SPINLOCK_H#include <asm/paravirt_types.h> +#include <linux/static_call_types.h> #ifdef CONFIG_SMP #include <asm/spinlock_types.h>
@@ -11,9 +12,6 @@struct qspinlock ;struct pv_lock_ops { -void (*queued_spin_lock_slowpath)(struct qspinlock *lock,u32 val); -struct paravirt_callee_save queued_spin_unlock; -void (*wait)(u8 *ptr,u8 val);void (*kick)(int cpu);
@@ -26,20 +24,27 @@ extern struct pv_lock_ops pv_ops_lock;extern void native_queued_spin_lock_slowpath (struct qspinlock *lock,u32 val);extern void __pv_init_lock_hash (void );extern void __pv_queued_spin_lock_slowpath (struct qspinlock *lock,u32 val); +extern void __raw_callee_save___native_queued_spin_unlock (struct qspinlock *lock);extern void __raw_callee_save___pv_queued_spin_unlock (struct qspinlock *lock);extern bool nopvspin; +DECLARE_STATIC_CALL (queued_spin_lock_slowpath, native_queued_spin_lock_slowpath); +DECLARE_STATIC_CALL (queued_spin_unlock, __raw_callee_save___native_queued_spin_unlock); + static __always_inline void pv_queued_spin_lock_slowpath(struct qspinlock *lock,u32 val) { -PVOP_VCALL2 (pv_ops_lock, queued_spin_lock_slowpath, lock, val); +static_call_mod (queued_spin_lock_slowpath)(lock, val); } static __always_inline void pv_queued_spin_unlock(struct qspinlock *lock) { -PVOP_ALT_VCALLEE1 (pv_ops_lock, queued_spin_unlock, lock, -"movb $0, (%%" _ASM_ARG1")" , -ALT_NOT (X86_FEATURE_PVUNLOCK )); +PVOP_CALL_ARGS ; +__STATIC_CALL_MOD_ADDRESSABLE (queued_spin_unlock); + asm volatile ("call " STATIC_CALL_TRAMP_STR(queued_spin_unlock) + : PVOP_VCALLEE_CLOBBERS, ASM_CALL_CONSTRAINT + : PVOP_CALL_ARG1(lock) + : "memory", "cc"); } static __always_inline bool pv_vcpu_is_preempted(long cpu)
arch/x86/kernel/kvm.c +2 -3
@@ -1136,9 +1136,8 @@ void __init kvm_spinlock_init(void)pr_info ("PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock = -PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = kvm_wait; pv_ops_lock.kick = kvm_kick_cpu;
arch/x86/kernel/paravirt-spinlocks.c +6 -6
@@ -25,9 +25,14 @@ __visible void __native_queued_spin_unlock(struct qspinlock *lock) }PV_CALLEE_SAVE_REGS_THUNK (__native_queued_spin_unlock); +DEFINE_STATIC_CALL (queued_spin_lock_slowpath, native_queued_spin_lock_slowpath); +EXPORT_STATIC_CALL_TRAMP (queued_spin_lock_slowpath); +DEFINE_STATIC_CALL (queued_spin_unlock, __raw_callee_save___native_queued_spin_unlock); +EXPORT_STATIC_CALL_TRAMP (queued_spin_unlock); +bool pv_is_native_spin_unlock (void ) { -return pv_ops_lock.queued_spin_unlock.func == +return static_call_query (queued_spin_unlock) == __raw_callee_save___native_queued_spin_unlock; }
@@ -45,16 +50,11 @@ bool pv_is_native_vcpu_is_preempted(void)void __init paravirt_set_cap (void ) { -if (!pv_is_native_spin_unlock ()) -setup_force_cpu_cap (X86_FEATURE_PVUNLOCK ); -if (!pv_is_native_vcpu_is_preempted ())setup_force_cpu_cap (X86_FEATURE_VCPUPREEMPT ); }struct pv_lock_ops pv_ops_lock = { - .queued_spin_lock_slowpath = native_queued_spin_lock_slowpath, - .queued_spin_unlock =PV_CALLEE_SAVE (__native_queued_spin_unlock), .wait = paravirt_nop, .kick = paravirt_nop, .vcpu_is_preempted =PV_CALLEE_SAVE (__native_vcpu_is_preempted),
arch/x86/kernel/static_call.c +27 -0
@@ -4,6 +4,12 @@#include <linux/bug.h> #include <asm/text-patching.h> +/* Declared locally to avoid pulling asm/paravirt-spinlock.h header. */ +#ifdef CONFIG_PARAVIRT_SPINLOCKS +struct qspinlock ; +void __raw_callee_save___native_queued_spin_unlock (struct qspinlock *lock); +#endif +enum insn_type {CALL =0 ,/* site call */ NOP =1 ,/* site cond-call */
@@ -31,6 +37,17 @@ static const u8 retinsn[] = { RET_INSN_OPCODE, 0xcc, 0xcc, 0xcc, 0xcc };*/ static const u8 warninsn[] = {0x67 ,0x48 ,0x0f ,0xb9 ,0x3a }; +#ifdef CONFIG_PARAVIRT_SPINLOCKS +/* +* ds ds movb $0, (_ASM_ARG1) +*/ +#ifdef CONFIG_64BIT +static const u8 unlockinsn[] = {0x3e ,0x3e ,0xc6 ,0x07 ,0x00 }; +#else +static const u8 unlockinsn[] = {0x3e ,0x3e ,0xc6 ,0x00 ,0x00 }; +#endif +#endif +static u8 __is_Jcc (u8 *insn)/* Jcc.d32 */ {u8 ret =0 ;
@@ -78,6 +95,12 @@ static void __ref __static_call_transform(void *insn, enum insn_type type, emulate = code; code = &warninsn; } +#ifdef CONFIG_PARAVIRT_SPINLOCKS +if (func == &__raw_callee_save___native_queued_spin_unlock) { + emulate = code; + code = &unlockinsn; + } +#endif break ;case NOP :
@@ -139,6 +162,10 @@ static void __static_call_validate(u8 *insn, bool tail, bool tramp) !memcmp (insn, xor5rax,5 ) || !memcmp(insn, warninsn, 5)) return; +#ifdef CONFIG_PARAVIRT_SPINLOCKS +if (!memcmp (insn, unlockinsn,5 )) +return ; +#endif }/*
arch/x86/xen/spinlock.c +2 -3
@@ -134,9 +134,8 @@ void __init xen_init_spinlocks(void)printk (KERN_DEBUG "xen: PV spinlocks enabled\n" );__pv_init_lock_hash (); - pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; - pv_ops_lock.queued_spin_unlock = -PV_CALLEE_SAVE (__pv_queued_spin_unlock); +static_call_update (queued_spin_lock_slowpath, __pv_queued_spin_lock_slowpath); +static_call_update (queued_spin_unlock, __raw_callee_save___pv_queued_spin_unlock); pv_ops_lock.wait = xen_qlock_wait; pv_ops_lock.kick = xen_qlock_kick; pv_ops_lock.vcpu_is_preempted =PV_CALLEE_SAVE (xen_vcpu_stolen);
tools/arch/x86/include/asm/cpufeatures.h +1 -1
@@ -225,7 +225,7 @@#define X86_FEATURE_EPT_AD ( 8*32+17)/* "ept_ad" Intel Extended Page Table access-dirty bit */ #define X86_FEATURE_VMCALL ( 8*32+18)/* Hypervisor supports the VMCALL instruction */ #define X86_FEATURE_VMW_VMMCALL ( 8*32+19)/* VMware prefers VMMCALL hypercall instruction */ -#define X86_FEATURE_PVUNLOCK ( 8*32+20)/* PV unlock function */ +// free: was #define X86_FEATURE_PVUNLOCK ( 8*32+20) /* PV unlock function */ #define X86_FEATURE_VCPUPREEMPT ( 8*32+21)/* PV vcpu_is_preempted function */ #define X86_FEATURE_TDX_GUEST ( 8*32+22)/* "tdx_guest" Intel Trust Domain Extensions Guest */
PATCH 2/5 locking: Factor out queued_spin_release()
In reply to Dmitry IlvokhinThe contended_release tracepoint needs to hook queued_spin_unlock(), but architectures with a custom unlock define queued_spin_unlock() directly, leaving no single generic place to add the tracing.
Introduce queued_spin_release() as the arch-overridable release primitive and make queued_spin_unlock() a generic wrapper around it. An architecture that only customizes the release can then override queued_spin_release() and inherit the generic wrapper.
Rename the MIPS override to queued_spin_release() accordingly. x86 paravirt overrides queued_spin_unlock() directly and is left unchanged.
No functional change intended.
--- arch/mips/include/asm/spinlock.h | 6 +++--- include/asm-generic/qspinlock.h | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-)
arch/mips/include/asm/spinlock.h +3 -3
@@ -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 ();
include/asm-generic/qspinlock.h +14 -3
@@ -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,17 @@ static __always_inline void queued_spin_unlock(struct qspinlock *lock) }#endif +#ifndef queued_spin_unlock +/** +* 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); +} +#endif +#ifndef virt_spin_lock static __always_inline bool virt_spin_lock(struct qspinlock *lock) {
Signature · 1 line
2.53.0-Meta
[tip: locking/core] locking: Factor out queued_spin_release()
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 216c6c67f7f1292474de1a3d1ae7d1cb95514264 Gitweb: https://git.kernel.org/tip/216c6c67f7f1292474de1a3d1ae7d1cb95514264 Author: Dmitry Ilvokhin <[email protected]> AuthorDate: Tue, 04 Aug 2026 07:15:42 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 07 Aug 2026 17:58:09 +02:00
locking: Factor out queued_spin_release()
The contended_release tracepoint needs to hook queued_spin_unlock(), but architectures with a custom unlock define queued_spin_unlock() directly, leaving no single generic place to add the tracing.
Introduce queued_spin_release() as the arch-overridable release primitive and make queued_spin_unlock() a generic wrapper around it. An architecture that only customizes the release can then override queued_spin_release() and inherit the generic wrapper.
Rename the MIPS override to queued_spin_release() accordingly. x86 paravirt overrides queued_spin_unlock() directly and is left unchanged.
No functional change intended.
--- arch/mips/include/asm/spinlock.h | 6 +++--- include/asm-generic/qspinlock.h | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-)
arch/mips/include/asm/spinlock.h +3 -3
@@ -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 ();
include/asm-generic/qspinlock.h +14 -3
@@ -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,17 @@ static __always_inline void queued_spin_unlock(struct qspinlock *lock) }#endif +#ifndef queued_spin_unlock +/** +* 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); +} +#endif +#ifndef virt_spin_lock static __always_inline bool virt_spin_lock(struct qspinlock *lock) {
PATCH 3/5 locking/qspinlock: Add contended_release tracepoint
In reply to Dmitry IlvokhinUnlike mutex and rw_semaphore, qspinlock has no owner field, so "perf lock contention --lock-owner" cannot attribute a contended spinlock to its holder. The waiter-side contention_begin event records that a spinlock is contended, but not by whom. Firing contended_release in the holder's context at unlock is the only way to capture the holder of a contended spinlock.
Combine the contention check, trace call and release in an out-of-line queued_spin_release_traced() so the compiler need not preserve the lock pointer in a callee-saved register across the call.
The check in queued_spin_unlock() is paid on every unlock, even while the tracepoint is disabled: a static-branch NOP on x86_64, and a few more instructions to manage a stack frame elsewhere. Gate it behind CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE (default n) so nobody pays for a tracepoint they do not use. Sleeping locks fire contended_release regardless.
On x86 this generic path is used only with PARAVIRT_SPINLOCKS=n (e.g. defconfig). PARAVIRT_SPINLOCKS=y kernels keep the paravirt static_call unlock and are wired up separately.
All below are with the QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE option enabled.
_raw_spin_unlock(), x86_64 defconfig, GCC 11, tracepoint compiled in but disabled. The unlock is the single 'movb'. The only instruction added to the executed path is the 2-byte static-branch NOP. The CALL to the traced helper and the JMP back are emitted out of line and are reached only once the static branch is patched on:
endbr64 ; 4 bytes
xchg %ax,%ax ; 2 static-branch NOP
; (added)
movb $0x0,(%rdi) ; 3 unlock (single store)
A: decl %gs:__preempt_count ; 7
je B ; 2
jmp __x86_return_thunk ; 5
call queued_spin_release_traced ; 5 out of line, reached
; only when the
; tracepoint is on
jmp A ; 2 (added)
B: call __SCT__preempt_schedule ; 5
jmp __x86_return_thunk ; 5
Baseline is the same stream without the NOP and the out-of-line CALL/JMP: 31 bytes vs 40 (+9 bytes).
Binary size impact on x86_64, defconfig: +680 bytes (+0.00%), since all standard configs out-of-line unlock. Architectures with inlined unlock (s390 (always), csky and loongarch (both when !PREEMPTION)) will see a bigger increase in binary size.
On the same path (x86_64, PARAVIRT_SPINLOCKS=n) with the tracepoint disabled, a _raw_spin_unlock()-heavy nginx workload [1] shows no measurable difference between baseline and patched kernels in throughput, latency, cycles, instructions, IPC, or L1 instruction-cache misses (kernel and total): all deltas stay within run-to-run noise.
Unlike x86, on arm64 the frame setup code (STP, MOV and LDP) lands on the executed path in addition to static-branch NOP. Binary size impact on arm64, defconfig: +932 bytes (+0.00%).
The _raw_spin_unlock()-heavy nginx workload reflects the larger hot
path: L1 instruction-cache misses rise ~1.4% (kernel and total) and instruction count ~0.4%, consistent with the per-unlock frame. cpu_cycles, throughput and latency show no measurable change and are within run-to-run noise.
Architectures with fully custom qspinlock implementations (e.g. PowerPC) are not covered by this change.
[1]: https://lore.kernel.org/all/[email protected]/
--- include/asm-generic/qspinlock.h | 21 +++++++++++++++++++++ kernel/Kconfig.locks | 20 ++++++++++++++++++++ kernel/locking/qspinlock.c | 22 ++++++++++++++++++++++ 3 files changed, 63 insertions(+)
include/asm-generic/qspinlock.h +21 -0
@@ -41,6 +41,7 @@#include <asm-generic/qspinlock_types.h> #include <linux/atomic.h> +#include <linux/tracepoint-defs.h> #ifndef queued_spin_is_locked/**
@@ -130,12 +131,32 @@ static __always_inline void queued_spin_release(struct qspinlock *lock)#endif #ifndef queued_spin_unlock + +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. queued_spin_release() is therefore called both here and in +* queued_spin_release_traced(). Keep the two in sync. +*/ +if (IS_ENABLED (CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE ) && +tracepoint_enabled (contended_release)) { +queued_spin_release_traced (lock); +return ; + }queued_spin_release (lock); }#endif
kernel/Kconfig.locks +20 -0
@@ -243,6 +243,26 @@ config QUEUED_SPINLOCKS def_bool y if ARCH_USE_QUEUED_SPINLOCKS depends on SMP +config QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE + bool "Trace contended_release on queued spinlocks" + depends on QUEUED_SPINLOCKS && TRACEPOINTS + help + Fire the lock:contended_release tracepoint when a contended queued + spinlock is released, so it is possible to attribute a contended + spinlock to its holder. + + Architectures that can patch the unlock site do this at no cost and + do not need this option. + + Everywhere else the check is compiled into queued_spin_unlock() and + a small cost is paid on every unlock even when the tracepoint is + disabled: a static-branch NOP and possibly a few more instructions + to manage a stack frame. + + Sleeping locks fire lock:contended_release regardless of this option. + + If unsure, say N. + config BPF_ARCH_SPINLOCK bool
kernel/locking/qspinlock.c +22 -0
@@ -104,6 +104,28 @@ static __always_inline u32 __pv_wait_head_or_lock(struct qspinlock *lock,#define queued_spin_lock_slowpath native_queued_spin_lock_slowpath#endif +#if !defined(queued_spin_unlock) && \ +IS_ENABLED (CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE ) +/* +* Out-of-line trace-and-release path for queued_spin_unlock(), used when +* the contended_release tracepoint is enabled. +* +* queued_spin_release() is duplicated here on purpose: doing the release +* in this function (rather than tracing here and releasing in the caller) +* lets queued_spin_unlock() return right after the call, so the +* tracepoint-disabled hot path never has to keep lock live across a call +* in a callee-saved register. Keep this release in sync with the one in +* queued_spin_unlock(). +*/ +void __lockfunc queued_spin_release_traced(struct qspinlock *lock) +{ +if (queued_spin_is_contended (lock)) +trace_call__contended_release (lock); +queued_spin_release (lock); +} +EXPORT_SYMBOL (queued_spin_release_traced); +#endif +#endif /* _GEN_PV_LOCK_SLOWPATH */ /**
Signature · 1 line
2.53.0-Meta
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The inline function `queued_spin_unlock()` references a GPL-only tracepoint symbol, inadvertently…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The inline function queued_spin_unlock() references a GPL-only tracepoint symbol, inadvertently turning spin_unlock() into a GPL-only API and breaking non-GPL out-of-tree modules.
- [High] The lock release tracepoint bypasses the RCU watching check, allowing trace probes to execute unsafely in idle/offline contexts.
--
--- Patch [3]: [PATCH 3/5] locking/qspinlock: Add contended_release tracepoint --- commit afc21b7e9fbc296817ffabe3684c2756069801e5
Author: Dmitry Ilvokhin <[email protected]>
locking/qspinlock: Add contended_release tracepoint
This commit adds a contended_release tracepoint to qspinlock to allow attributing a contended spinlock to its holder. It combines the contention check and trace call in an out-of-line function to minimize overhead on the hot path. The feature is gated behind a new configuration option.
Quoted 4 lines
include/asm-generic/qspinlock.h +0
-0
[ ... ]
Quoted 17 lines
include/asm-generic/qspinlock.h +8
-0@@ -130,12 +131,32 @@ static __always_inline void queued_spin_release(struct qspinlock *lock)#endif #ifndef queued_spin_unlock + +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)
include/asm-generic/qspinlock.h static __always_inline void queued_spin_release(struct qspinlock *lock) · line 156
Quoted 10 lines
include/asm-generic/qspinlock.h +9
-0{ +/* +* 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. queued_spin_release() is therefore called both here and in +* queued_spin_release_traced(). Keep the two in sync. +*/ +if (IS_ENABLED (CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE ) && +tracepoint_enabled (contended_release)) {
[Severity: High]
Does this inadvertently make spin_unlock() a GPL-only API?
The contended_release tracepoint is exported via EXPORT_TRACEPOINT_SYMBOL_GPL in mutex.c.
When spin_unlock() is inlined, this injects the GPL-only symbol into every module calling spin_unlock().
Would this break the compilation or loading of non-GPL out-of-tree modules due to referencing a GPL-only symbol?
Quoted 10 lines
include/asm-generic/qspinlock.h +3
-0+queued_spin_release_traced (lock); + return; + }queued_spin_release (lock); }#endif kernel/locking/qspinlock.c +0
-0
[ ... ]
Quoted 11 lines
kernel/locking/qspinlock.c +7
-0@@ -104,6 +104,28 @@ static __always_inline u32 __pv_wait_head_or_lock(struct qspinlock *lock,#define queued_spin_lock_slowpath native_queued_spin_lock_slowpath#endif +#if !defined(queued_spin_unlock) && \ +IS_ENABLED (CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE ) +/* +* Out-of-line trace-and-release path for queued_spin_unlock(), used when +* the contended_release tracepoint is enabled. +* +* queued_spin_release() is duplicated here on purpose: doing the release
kernel/locking/qspinlock.c static __always_inline u32 __pv_wait_head_or_lock(struct qspinlock *lock, · line 123
Quoted 10 lines
kernel/locking/qspinlock.c +10
-0+* in this function (rather than tracing here and releasing in the caller) +* lets queued_spin_unlock() return right after the call, so the +* tracepoint-disabled hot path never has to keep lock live across a call +* in a callee-saved register. Keep this release in sync with the one in +* queued_spin_unlock(). +*/ +void __lockfunc queued_spin_release_traced(struct qspinlock *lock) +{ +if (queued_spin_is_contended (lock)) +trace_call__contended_release (lock);
[Severity: High]
Does calling trace_call__contended_release() directly bypass the RCU watching checks?
Raw spinlocks are often used in the scheduler idle loop or CPU hotplug paths where RCU is explicitly not watching (for example, after rcu_idle_enter() is called).
The standard trace_contended_release() wrapper includes a lockdep safety check to warn if RCU is not watching.
Could this direct call allow trace probes to execute unsafely in idle or offline contexts, potentially causing RCU splats or use-after-free bugs if attached BPF programs or ftrace plugins dereference RCU-protected data?
Quoted 5 lines
kernel/locking/qspinlock.c +5
-0+ queued_spin_release(lock); +} +EXPORT_SYMBOL (queued_spin_release_traced); +#endif +
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: f7e2cb6d495aa1a88368650970a230b45870859b Gitweb: https://git.kernel.org/tip/f7e2cb6d495aa1a88368650970a230b45870859b Author: Dmitry Ilvokhin <[email protected]> AuthorDate: Tue, 04 Aug 2026 07:15:43 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 07 Aug 2026 17:58:10 +02:00
locking/qspinlock: Add contended_release tracepoint
Unlike mutex and rw_semaphore, qspinlock has no owner field, so "perf lock contention --lock-owner" cannot attribute a contended spinlock to its holder. The waiter-side contention_begin event records that a spinlock is contended, but not by whom. Firing contended_release in the holder's context at unlock is the only way to capture the holder of a contended spinlock.
Combine the contention check, trace call and release in an out-of-line queued_spin_release_traced() so the compiler need not preserve the lock pointer in a callee-saved register across the call.
The check in queued_spin_unlock() is paid on every unlock, even while the tracepoint is disabled: a static-branch NOP on x86_64, and a few more instructions to manage a stack frame elsewhere. Gate it behind CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE (default n) so nobody pays for a tracepoint they do not use. Sleeping locks fire contended_release regardless.
On x86 this generic path is used only with PARAVIRT_SPINLOCKS=n (e.g. defconfig). PARAVIRT_SPINLOCKS=y kernels keep the paravirt static_call unlock and are wired up separately.
All below are with the QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE option enabled.
_raw_spin_unlock(), x86_64 defconfig, GCC 11, tracepoint compiled in but disabled. The unlock is the single 'movb'. The only instruction added to the executed path is the 2-byte static-branch NOP. The CALL to the traced helper and the JMP back are emitted out of line and are reached only once the static branch is patched on:
endbr64 ; 4 bytes
xchg %ax,%ax ; 2 static-branch NOP
; (added)
movb $0x0,(%rdi) ; 3 unlock (single store)
A: decl %gs:__preempt_count ; 7
je B ; 2
jmp __x86_return_thunk ; 5
call queued_spin_release_traced ; 5 out of line, reached
; only when the
; tracepoint is on
jmp A ; 2 (added)
B: call __SCT__preempt_schedule ; 5
jmp __x86_return_thunk ; 5
Baseline is the same stream without the NOP and the out-of-line CALL/JMP: 31 bytes vs 40 (+9 bytes).
Binary size impact on x86_64, defconfig: +680 bytes (+0.00%), since all standard configs out-of-line unlock. Architectures with inlined unlock (s390 (always), csky and loongarch (both when !PREEMPTION)) will see a bigger increase in binary size.
On the same path (x86_64, PARAVIRT_SPINLOCKS=n) with the tracepoint disabled, a _raw_spin_unlock()-heavy nginx workload [1] shows no measurable difference between baseline and patched kernels in throughput, latency, cycles, instructions, IPC, or L1 instruction-cache misses (kernel and total): all deltas stay within run-to-run noise.
Unlike x86, on arm64 the frame setup code (STP, MOV and LDP) lands on the executed path in addition to static-branch NOP. Binary size impact on arm64, defconfig: +932 bytes (+0.00%).
The _raw_spin_unlock()-heavy nginx workload reflects the larger hot
path: L1 instruction-cache misses rise ~1.4% (kernel and total) and instruction count ~0.4%, consistent with the per-unlock frame. cpu_cycles, throughput and latency show no measurable change and are within run-to-run noise.
Architectures with fully custom qspinlock implementations (e.g. PowerPC) are not covered by this change.
[1]: https://lore.kernel.org/all/[email protected]/
--- include/asm-generic/qspinlock.h | 21 +++++++++++++++++++++ kernel/Kconfig.locks | 20 ++++++++++++++++++++ kernel/locking/qspinlock.c | 22 ++++++++++++++++++++++ 3 files changed, 63 insertions(+)
include/asm-generic/qspinlock.h +21 -0
@@ -41,6 +41,7 @@#include <asm-generic/qspinlock_types.h> #include <linux/atomic.h> +#include <linux/tracepoint-defs.h> #ifndef queued_spin_is_locked/**
@@ -130,12 +131,32 @@ static __always_inline void queued_spin_release(struct qspinlock *lock)#endif #ifndef queued_spin_unlock + +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. queued_spin_release() is therefore called both here and in +* queued_spin_release_traced(). Keep the two in sync. +*/ +if (IS_ENABLED (CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE ) && +tracepoint_enabled (contended_release)) { +queued_spin_release_traced (lock); +return ; + }queued_spin_release (lock); }#endif
kernel/Kconfig.locks +20 -0
@@ -243,6 +243,26 @@ config QUEUED_SPINLOCKS def_bool y if ARCH_USE_QUEUED_SPINLOCKS depends on SMP +config QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE + bool "Trace contended_release on queued spinlocks" + depends on QUEUED_SPINLOCKS && TRACEPOINTS + help + Fire the lock:contended_release tracepoint when a contended queued + spinlock is released, so it is possible to attribute a contended + spinlock to its holder. + + Architectures that can patch the unlock site do this at no cost and + do not need this option. + + Everywhere else the check is compiled into queued_spin_unlock() and + a small cost is paid on every unlock even when the tracepoint is + disabled: a static-branch NOP and possibly a few more instructions + to manage a stack frame. + + Sleeping locks fire lock:contended_release regardless of this option. + + If unsure, say N. + config BPF_ARCH_SPINLOCK bool
kernel/locking/qspinlock.c +22 -0
@@ -104,6 +104,28 @@ static __always_inline u32 __pv_wait_head_or_lock(struct qspinlock *lock,#define queued_spin_lock_slowpath native_queued_spin_lock_slowpath#endif +#if !defined(queued_spin_unlock) && \ +IS_ENABLED (CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE ) +/* +* Out-of-line trace-and-release path for queued_spin_unlock(), used when +* the contended_release tracepoint is enabled. +* +* queued_spin_release() is duplicated here on purpose: doing the release +* in this function (rather than tracing here and releasing in the caller) +* lets queued_spin_unlock() return right after the call, so the +* tracepoint-disabled hot path never has to keep lock live across a call +* in a callee-saved register. Keep this release in sync with the one in +* queued_spin_unlock(). +*/ +void __lockfunc queued_spin_release_traced(struct qspinlock *lock) +{ +if (queued_spin_is_contended (lock)) +trace_call__contended_release (lock); +queued_spin_release (lock); +} +EXPORT_SYMBOL (queued_spin_release_traced); +#endif +#endif /* _GEN_PV_LOCK_SLOWPATH */ /**
PATCH 4/5 tracing/lock: Use TRACE_EVENT_FN() for contended_release
In reply to Dmitry Ilvokhinqueued_spin_unlock() gates its contended_release trace call behind a static branch, so a NOP sits on the unlock path even while the tracepoint is disabled. Removing that requires replacing the unlock implementation only while contended_release is enabled, which needs a callback when the tracepoint is toggled.
Convert contended_release to TRACE_EVENT_FN() and add weak no-op arch_contended_release_trace_reg()/arch_contended_release_trace_unreg() hooks.
The default hooks are empty, so this is a no-op until an architecture overrides them.
No functional change intended.
--- include/trace/events/lock.h | 10 ++++++++-- kernel/locking/mutex.c | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
include/trace/events/lock.h +8 -2
@@ -137,7 +137,11 @@ TRACE_EVENT(contention_end,TP_printk ("%p (ret=%d)" , __entry->lock_addr, __entry->ret) ); -TRACE_EVENT(contended_release, +/* kernel/locking/mutex.c */ +int arch_contended_release_trace_reg (void ); +void arch_contended_release_trace_unreg (void ); + +TRACE_EVENT_FN(contended_release, TP_PROTO(void *lock),
@@ -151,7 +155,9 @@ TRACE_EVENT(contended_release, __entry->lock_addr = lock; ), -TP_printk ("%p" , __entry->lock_addr) +TP_printk ("%p" , __entry->lock_addr), + + arch_contended_release_trace_reg, arch_contended_release_trace_unreg );#endif /* _TRACE_LOCK_H */
kernel/locking/mutex.c +4 -0
@@ -1272,6 +1272,10 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(contention_begin);EXPORT_TRACEPOINT_SYMBOL_GPL (contention_end);EXPORT_TRACEPOINT_SYMBOL_GPL (contended_release); +__weak int arch_contended_release_trace_reg (void ) {return 0 ; } + +__weak void arch_contended_release_trace_unreg (void ) { } +/** * atomic_dec_and_mutex_lock - return holding mutex if we dec to 0 * @cnt: the atomic which we are to dec
Signature · 1 line
2.53.0-Meta
[tip: locking/core] tracing/lock: Use TRACE_EVENT_FN() for contended_release
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: b359800c6970cb653d41ed2af18fd8e95dbb822f Gitweb: https://git.kernel.org/tip/b359800c6970cb653d41ed2af18fd8e95dbb822f Author: Dmitry Ilvokhin <[email protected]> AuthorDate: Tue, 04 Aug 2026 07:15:44 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 07 Aug 2026 17:58:10 +02:00
tracing/lock: Use TRACE_EVENT_FN() for contended_release
queued_spin_unlock() gates its contended_release trace call behind a static branch, so a NOP sits on the unlock path even while the tracepoint is disabled. Removing that requires replacing the unlock implementation only while contended_release is enabled, which needs a callback when the tracepoint is toggled.
Convert contended_release to TRACE_EVENT_FN() and add weak no-op arch_contended_release_trace_reg()/arch_contended_release_trace_unreg() hooks.
The default hooks are empty, so this is a no-op until an architecture overrides them.
No functional change intended.
--- include/trace/events/lock.h | 10 ++++++++-- kernel/locking/mutex.c | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
include/trace/events/lock.h +8 -2
@@ -137,7 +137,11 @@ TRACE_EVENT(contention_end,TP_printk ("%p (ret=%d)" , __entry->lock_addr, __entry->ret) ); -TRACE_EVENT(contended_release, +/* kernel/locking/mutex.c */ +int arch_contended_release_trace_reg (void ); +void arch_contended_release_trace_unreg (void ); + +TRACE_EVENT_FN(contended_release, TP_PROTO(void *lock),
@@ -151,7 +155,9 @@ TRACE_EVENT(contended_release, __entry->lock_addr = lock; ), -TP_printk ("%p" , __entry->lock_addr) +TP_printk ("%p" , __entry->lock_addr), + + arch_contended_release_trace_reg, arch_contended_release_trace_unreg );#endif /* _TRACE_LOCK_H */
kernel/locking/mutex.c +4 -0
@@ -1272,6 +1272,10 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(contention_begin);EXPORT_TRACEPOINT_SYMBOL_GPL (contention_end);EXPORT_TRACEPOINT_SYMBOL_GPL (contended_release); +__weak int arch_contended_release_trace_reg (void ) {return 0 ; } + +__weak void arch_contended_release_trace_unreg (void ) { } +/** * atomic_dec_and_mutex_lock - return holding mutex if we dec to 0 * @cnt: the atomic which we are to dec
PATCH 5/5 x86/paravirt: Trace contended_release on unlock
In reply to Dmitry IlvokhinOn PARAVIRT_SPINLOCKS=y kernels queued_spin_unlock() is dispatched through a static_call(). Those PARAVIRT_SPINLOCKS=y kernels are quite popular. Gating contended_release behind a static branch would leave a NOP on the unlock hot path even, when the tracepoint is disabled.
Since the static_call() is already present, swap its target to a traced unlock, when the tracepoint is enabled instead. When contended_release tracepoint is disabled the target is the plain unlock (an inline store on native x86_64), so the unlock path is unchanged and the tracepoint is truly zero-cost.
Provide two traced variants, native_queued_spin_unlock_traced() and pv_queued_spin_unlock_traced(), so each tail-calls its own base unlock directly rather than recursing through the now-traced static_call().
Teach pv_is_native_spin_unlock() that the traced native variant still counts as native.
Only PARAVIRT_SPINLOCKS=y is affected. PARAVIRT_SPINLOCKS=n keeps the generic static-branch path.
--- arch/x86/include/asm/paravirt-spinlock.h | 2 + arch/x86/kernel/paravirt-spinlocks.c | 53 +++++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-)
arch/x86/include/asm/paravirt-spinlock.h +2 -0
@@ -99,6 +99,8 @@ bool __raw_callee_save___native_vcpu_is_preempted(long cpu);void __init native_pv_lock_init (void ); __visible void __native_queued_spin_unlock(struct qspinlock *lock); +__visible void native_queued_spin_unlock_traced(struct qspinlock *lock); +__visible void pv_queued_spin_unlock_traced(struct qspinlock *lock);bool pv_is_native_spin_unlock (void ); __visible bool __native_vcpu_is_preempted(long cpu);bool pv_is_native_vcpu_is_preempted (void );
arch/x86/kernel/paravirt-spinlocks.c +51 -2
67 lines not on this page · raw message
Signature · 1 line
2.53.0-Meta
[email protected]
AI reviewRe: [PATCH 5/5] x86/paravirt: Trace contended_release on unlock
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Paravirt queued spinlocks unconditionally trace 'contended_release' when the tracepoint is…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Paravirt queued spinlocks unconditionally trace 'contended_release' when the tracepoint is enabled globally, ignoring the CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE opt-out. --
--- Patch [5]: [PATCH 5/5] x86/paravirt: Trace contended_release on unlock --- commit 1a2ad2e72bd14437a21c720ae7476112bc7cb8d5
Author: Dmitry Ilvokhin <[email protected]>
x86/paravirt: Trace contended_release on unlock
This commit introduces traced variants of paravirt spinlock unlock functions to trace the contended_release event. It dynamically swaps the static call target to point to these traced versions when the tracepoint is enabled, keeping the disabled path zero-cost.
Quoted 4 lines
arch/x86/kernel/paravirt-spinlocks.c +0
-0
[ ... ]
arch/x86/kernel/paravirt-spinlocks.c EXPORT_STATIC_CALL_TRAMP(queued_spin_lock_slowpath); · line 74
Quoted 12 lines
arch/x86/kernel/paravirt-spinlocks.c +12
-0+int arch_contended_release_trace_reg (void ) +{ +void *cur =static_call_query (queued_spin_unlock); + +if (cur == __raw_callee_save___native_queued_spin_unlock) +static_call_update (queued_spin_unlock, + __raw_callee_save_native_queued_spin_unlock_traced); +else if (cur == __raw_callee_save___pv_queued_spin_unlock) +static_call_update (queued_spin_unlock, + __raw_callee_save_pv_queued_spin_unlock_traced); +return 0 ; +}
[Severity: Medium]
Will arch_contended_release_trace_reg() unconditionally trace the paravirt queued spinlocks when the tracepoint is enabled globally, even if the kernel was built with CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE=n?
It looks like this registration callback swaps the static calls to the traced variants without checking if spinlock tracing was explicitly opted out in the kernel configuration.
arch/x86/kernel/paravirt-spinlocks.c EXPORT_STATIC_CALL_TRAMP(queued_spin_lock_slowpath); · line 86
Quoted 12 lines
arch/x86/kernel/paravirt-spinlocks.c +11
-0+ +void arch_contended_release_trace_unreg (void ) +{ +void *cur =static_call_query (queued_spin_unlock); + +if (cur == __raw_callee_save_native_queued_spin_unlock_traced) +static_call_update (queued_spin_unlock, + __raw_callee_save___native_queued_spin_unlock); +else if (cur == __raw_callee_save_pv_queued_spin_unlock_traced) +static_call_update (queued_spin_unlock, + __raw_callee_save___pv_queued_spin_unlock); }
[Severity: Medium]
Similarly, does arch_contended_release_trace_unreg() also need to check the CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE configuration?
Without a config guard, this manipulates the static calls when the tracepoint is disabled globally, even if spinlock tracing was intentionally excluded during the build.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
[tip: locking/core] x86/paravirt: Trace contended_release on unlock
In reply to Dmitry IlvokhinThe following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 087116fefbf343ce63b8911cf494e059e9679432 Gitweb: https://git.kernel.org/tip/087116fefbf343ce63b8911cf494e059e9679432 Author: Dmitry Ilvokhin <[email protected]> AuthorDate: Tue, 04 Aug 2026 07:15:45 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 07 Aug 2026 17:58:10 +02:00
x86/paravirt: Trace contended_release on unlock
On PARAVIRT_SPINLOCKS=y kernels queued_spin_unlock() is dispatched through a static_call(). Those PARAVIRT_SPINLOCKS=y kernels are quite popular. Gating contended_release behind a static branch would leave a NOP on the unlock hot path even, when the tracepoint is disabled.
Since the static_call() is already present, swap its target to a traced unlock, when the tracepoint is enabled instead. When contended_release tracepoint is disabled the target is the plain unlock (an inline store on native x86_64), so the unlock path is unchanged and the tracepoint is truly zero-cost.
Provide two traced variants, native_queued_spin_unlock_traced() and pv_queued_spin_unlock_traced(), so each tail-calls its own base unlock directly rather than recursing through the now-traced static_call().
Teach pv_is_native_spin_unlock() that the traced native variant still counts as native.
Only PARAVIRT_SPINLOCKS=y is affected. PARAVIRT_SPINLOCKS=n keeps the generic static-branch path.
--- arch/x86/include/asm/paravirt-spinlock.h | 2 +- arch/x86/kernel/paravirt-spinlocks.c | 53 ++++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-)
arch/x86/include/asm/paravirt-spinlock.h +2 -0
@@ -99,6 +99,8 @@ bool __raw_callee_save___native_vcpu_is_preempted(long cpu);void __init native_pv_lock_init (void ); __visible void __native_queued_spin_unlock(struct qspinlock *lock); +__visible void native_queued_spin_unlock_traced(struct qspinlock *lock); +__visible void pv_queued_spin_unlock_traced(struct qspinlock *lock);bool pv_is_native_spin_unlock (void ); __visible bool __native_vcpu_is_preempted(long cpu);bool pv_is_native_vcpu_is_preempted (void );
arch/x86/kernel/paravirt-spinlocks.c +51 -2
67 lines not on this page · raw message
Quoted 83 lines · Dmitry Ilvokhin
The
contended_releasetracepoint landed in v7.2-rc2 for sleeping locks (4f070ccb4dc4"locking: Addcontended_releasetracepoint to sleepable locks"). Spinlock support was dropped from that series. This one adds it for queued spinlocks.The existing contention_begin/contention_end tracepoints fire on the waiter side. The holder's identity and stack can be captured at
contention_begintime (e.g. perf lock contention --lock-owner), but only for locks with an owner field to read: mutex and rwsem. qspinlock has none, so a contended spinlock cannot be attributed to its holder at all. Even where the owner can be read, it reflects the holder's state when a waiter arrives, not when the lock is released.This series adds a
contended_releasetracepoint to qspinlock 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) andcontended_releasecan be correlated to measure how long the lock was held under contention.- The holder's stack at release time, which for spinlocks is not available by any other means.
The unlock path might be quite hot, so the tracepoint is made as cheap as possible, to keep it usable in production:
- x86 with PARAVIRT_SPINLOCKS=y, which is what distributions ship, swaps the unlock implementation via
static_call()when the tracepoint is enabled. The disabled path is byte-identical to today's: the same inline movb, no NOP and no call.- Everywhere else a static-branch check is compiled into
queued_spin_unlock(). Onx86_64that is a single NOP on the executed path, with the call to the traced helper emitted out of line and unreachable while the tracepoint is off. On other architectures a few more instructions to manage a stack frame land on the executed path too, so the generic path sits behindCONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE(default n).Costs and measurements are in the individual changelogs. Briefly, no throughput or latency change is measurable on either
x86_64or arm64 with QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE=y.Tested:
x86_64with PARAVIRT_SPINLOCKS=y and =n, arm64, tracepoint on and off, disassembly checked in both states, locktorture with tracepoint on and off.Not covered: qrwlock, and architectures with fully custom qspinlock implementations (e.g. PowerPC). The stack frame managing instructions on arm64 should be avoidable, but that is not done in this patchset.
Patch 1 is Peter's draft from [1] and is missing his Signed-off-by. Peter, please add it if you are happy with the patch.
[1]: https://lore.kernel.org/all/[email protected]/
Dmitry Ilvokhin (4):
locking: Factor outqueued_spin_release()locking/qspinlock: Addcontended_releasetracepoint tracing/lock: UseTRACE_EVENT_FN()forcontended_releasex86/paravirt: Tracecontended_releaseon unlockPeter Zijlstra (1):
x86/paravirt: Usestatic_call()for the paravirt spinlock opsarch/mips/include/asm/spinlock.h | 6 +-- arch/x86/hyperv/hv_spinlock.c | 4 +- arch/x86/include/asm/cpufeatures.h | 1 - arch/x86/include/asm/paravirt-spinlock.h | 21 +++++--- arch/x86/kernel/kvm.c | 5 +- arch/x86/kernel/paravirt-spinlocks.c | 63 +++++++++++++++++++++--- arch/x86/kernel/static_call.c | 27 ++++++++++ arch/x86/xen/spinlock.c | 5 +- include/asm-generic/qspinlock.h | 38 ++++++++++++-- include/trace/events/lock.h | 10 +++- kernel/Kconfig.locks | 20 ++++++++ kernel/locking/mutex.c | 4 ++ kernel/locking/qspinlock.c | 22 +++++++++ tools/arch/x86/include/asm/cpufeatures.h | 1 - 14 files changed, 195 insertions(+), 32 deletions(-)
Quoted 1 line
base-commit:
5e601ab3615c86be7c4068ce992f94654693a032
For the whole series:
I'm considering some followup patches replacing the remaining paravirt cases not covered by CONFIG_PARAVIRT_XXL with static_call(), too.
This will allow to drop the 32-bit paravirt patching completely. :-)
The queued_spin_unlock() hook was the main reason I didn't do that yet.
Juergen
Peter Zijlstra
maintainerQuoted 11 lines · Dmitry Ilvokhin
Patch 1 is Peter's draft from [1] and is missing his Signed-off-by. Peter, please add it if you are happy with the patch.
Dmitry Ilvokhin (4):
locking: Factor outqueued_spin_release()locking/qspinlock: Addcontended_releasetracepoint tracing/lock: UseTRACE_EVENT_FN()forcontended_releasex86/paravirt: Tracecontended_releaseon unlockPeter Zijlstra (1):
x86/paravirt: Usestatic_call()for the paravirt spinlock ops
Right, this all looks nice. Let me go queue this for the robots.
Thanks!