[PATCH v2 00/19] tracepoint: Avoid double static_branch evaluation at guarded call sites
When a caller already guards a tracepoint with an explicit enabled check:
if (trace_foo_enabled() && cond) trace_foo(args);
trace_foo() internally re-evaluates the static_branch_unlikely() key. Since static branches are patched binary instructions the compiler cannot fold the two evaluations, so every such site pays the cost twice.
This series introduces trace_call__##name() as a companion to trace_##name(). It calls __do_trace_##name() directly, bypassing the redundant static-branch re-check, while preserving all other correctness properties of the normal path (RCU-watching assertion, might_fault() for syscall tracepoints). The internal __do_trace_##name() symbol is not leaked to call sites; trace_call__##name() is the only new public API.
if (trace_foo_enabled() && cond) trace_call__foo(args); /* calls __do_trace_foo() directly */
The first patch adds the three-location change to include/linux/tracepoint.h (__DECLARE_TRACE, __DECLARE_TRACE_SYSCALL, and the !TRACEPOINTS_ENABLED stub). The remaining 18 patches mechanically convert all guarded call sites found in the tree: kernel/, io_uring/, net/, accel/habanalabs, cpufreq/, devfreq/, dma-buf/, fsi/, drm/, HID, i2c/, spi/, scsi/ufs/, btrfs/, net/devlink/, kernel/time/, kernel/trace/, mm/damon/, and arch/x86/.
This series is motivated by Peter Zijlstra's observation in the discussion around Dmitry Ilvokhin's locking tracepoint instrumentation series, where he noted that compilers cannot optimize static branches and that guarded call sites end up evaluating the static branch twice for no reason, and by Steven Rostedt's suggestion to add a proper API instead of exposing internal implementation details like __do_trace_##name() directly to call sites:
https://lore.kernel.org/linux-trace-kernel/8298e098d3418cb446ef396f119edac58...
Suggested-by: Steven Rostedt rostedt@goodmis.org Suggested-by: Peter Zijlstra peterz@infradead.org
Changes in v2: - Renamed trace_invoke_##name() to trace_call__##name() (double underscore) per review comments. - Added 4 new patches covering sites missed in v1, found using coccinelle to scan the tree (Keith Busch): * net/devlink: guarded tracepoint_enabled() block in trap.c * kernel/time: early-return guard in tick-sched.c (tick_stop) * kernel/trace: early-return guard in trace_benchmark.c * mm/damon: early-return guard in core.c * arch/x86: do_trace_*() wrapper functions in lib/msr.c, which are called exclusively from tracepoint_enabled()-guarded sites in asm/msr.h
v1: https://lore.kernel.org/linux-trace-kernel/abSqrJ1J59RQC47U@kbusch-mbp/
Vineeth Pillai (Google) (19): tracepoint: Add trace_call__##name() API kernel: Use trace_call__##name() at guarded tracepoint call sites io_uring: Use trace_call__##name() at guarded tracepoint call sites net: Use trace_call__##name() at guarded tracepoint call sites accel/habanalabs: Use trace_call__##name() at guarded tracepoint call sites cpufreq: Use trace_call__##name() at guarded tracepoint call sites devfreq: Use trace_call__##name() at guarded tracepoint call sites dma-buf: Use trace_call__##name() at guarded tracepoint call sites fsi: Use trace_call__##name() at guarded tracepoint call sites drm: Use trace_call__##name() at guarded tracepoint call sites HID: Use trace_call__##name() at guarded tracepoint call sites i2c: Use trace_call__##name() at guarded tracepoint call sites spi: Use trace_call__##name() at guarded tracepoint call sites scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites btrfs: Use trace_call__##name() at guarded tracepoint call sites net: devlink: Use trace_call__##name() at guarded tracepoint call sites kernel: time, trace: Use trace_call__##name() at guarded tracepoint call sites mm: damon: Use trace_call__##name() at guarded tracepoint call sites x86: msr: Use trace_call__##name() at guarded tracepoint call sites
arch/x86/lib/msr.c | 6 +++--- drivers/accel/habanalabs/common/device.c | 12 ++++++------ drivers/accel/habanalabs/common/mmu/mmu.c | 3 ++- drivers/accel/habanalabs/common/pci/pci.c | 4 ++-- drivers/cpufreq/amd-pstate.c | 10 +++++----- drivers/cpufreq/cpufreq.c | 2 +- drivers/cpufreq/intel_pstate.c | 2 +- drivers/devfreq/devfreq.c | 2 +- drivers/dma-buf/dma-fence.c | 4 ++-- drivers/fsi/fsi-master-aspeed.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++-- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/scheduler/sched_entity.c | 4 ++-- drivers/hid/intel-ish-hid/ipc/pci-ish.c | 2 +- drivers/i2c/i2c-core-slave.c | 2 +- drivers/spi/spi-axi-spi-engine.c | 4 ++-- drivers/ufs/core/ufshcd.c | 12 ++++++------ fs/btrfs/extent_map.c | 4 ++-- fs/btrfs/raid56.c | 4 ++-- include/linux/tracepoint.h | 11 +++++++++++ io_uring/io_uring.h | 2 +- kernel/irq_work.c | 2 +- kernel/sched/ext.c | 2 +- kernel/smp.c | 2 +- kernel/time/tick-sched.c | 12 ++++++------ kernel/trace/trace_benchmark.c | 2 +- mm/damon/core.c | 2 +- net/core/dev.c | 2 +- net/core/xdp.c | 2 +- net/devlink/trap.c | 2 +- net/openvswitch/actions.c | 2 +- net/openvswitch/datapath.c | 2 +- net/sctp/outqueue.c | 2 +- net/tipc/node.c | 2 +- 35 files changed, 74 insertions(+), 62 deletions(-)
Replace trace_foo() with the new trace_call__foo() at sites already guarded by trace_foo_enabled(), avoiding a redundant static_branch_unlikely() re-evaluation inside the tracepoint. trace_call__foo() calls the tracepoint callbacks directly without utilizing the static branch again.
Suggested-by: Steven Rostedt rostedt@goodmis.org Suggested-by: Peter Zijlstra peterz@infradead.org Signed-off-by: Vineeth Pillai (Google) vineeth@bitbyteword.org Assisted-by: Claude:claude-sonnet-4-6 --- drivers/dma-buf/dma-fence.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 35afcfcac5910..232e92196da43 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -535,7 +535,7 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
if (trace_dma_fence_wait_start_enabled()) { rcu_read_lock(); - trace_dma_fence_wait_start(fence); + trace_call__dma_fence_wait_start(fence); rcu_read_unlock(); } if (fence->ops->wait) @@ -544,7 +544,7 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) ret = dma_fence_default_wait(fence, intr, timeout); if (trace_dma_fence_wait_end_enabled()) { rcu_read_lock(); - trace_dma_fence_wait_end(fence); + trace_call__dma_fence_wait_end(fence); rcu_read_unlock(); } return ret;
Add trace_call__##name() as a companion to trace_##name(). When a caller already guards a tracepoint with an explicit enabled check:
if (trace_foo_enabled() && cond) trace_foo(args);
trace_foo() internally repeats the static_branch_unlikely() test, which the compiler cannot fold since static branches are patched binary instructions. This results in two static-branch evaluations for every guarded call site.
trace_call__##name() calls __do_trace_##name() directly, skipping the redundant static-branch re-check. This avoids leaking the internal __do_trace_##name() symbol into call sites while still eliminating the double evaluation:
if (trace_foo_enabled() && cond) trace_invoke_foo(args); /* calls __do_trace_foo() directly */
Three locations are updated: - __DECLARE_TRACE: invoke form omits static_branch_unlikely, retains the LOCKDEP RCU-watching assertion. - __DECLARE_TRACE_SYSCALL: same, plus retains might_fault(). - !TRACEPOINTS_ENABLED stub: empty no-op so callers compile cleanly when tracepoints are compiled out.
Suggested-by: Steven Rostedt rostedt@goodmis.org Suggested-by: Peter Zijlstra peterz@infradead.org Signed-off-by: Vineeth Pillai (Google) vineeth@bitbyteword.org Assisted-by: Claude:claude-sonnet-4-6 --- include/linux/tracepoint.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index 22ca1c8b54f32..ed969705341f1 100644 --- a/include/linux/tracepoint.h +++ b/include/linux/tracepoint.h @@ -294,6 +294,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) WARN_ONCE(!rcu_is_watching(), \ "RCU not watching for tracepoint"); \ } \ + } \ + static inline void trace_call__##name(proto) \ + { \ + __do_trace_##name(args); \ }
#define __DECLARE_TRACE_SYSCALL(name, proto, args, data_proto) \ @@ -313,6 +317,11 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) WARN_ONCE(!rcu_is_watching(), \ "RCU not watching for tracepoint"); \ } \ + } \ + static inline void trace_call__##name(proto) \ + { \ + might_fault(); \ + __do_trace_##name(args); \ }
/* @@ -398,6 +407,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) #define __DECLARE_TRACE_COMMON(name, proto, args, data_proto) \ static inline void trace_##name(proto) \ { } \ + static inline void trace_call__##name(proto) \ + { } \ static inline int \ register_trace_##name(void (*probe)(data_proto), \ void *data) \
On Mon, 23 Mar 2026 12:00:20 -0400 "Vineeth Pillai (Google)" vineeth@bitbyteword.org wrote:
Add trace_call__##name() as a companion to trace_##name(). When a caller already guards a tracepoint with an explicit enabled check:
if (trace_foo_enabled() && cond) trace_foo(args);
trace_foo() internally repeats the static_branch_unlikely() test, which the compiler cannot fold since static branches are patched binary instructions. This results in two static-branch evaluations for every guarded call site.
trace_call__##name() calls __do_trace_##name() directly, skipping the redundant static-branch re-check. This avoids leaking the internal __do_trace_##name() symbol into call sites while still eliminating the double evaluation:
if (trace_foo_enabled() && cond) trace_invoke_foo(args); /* calls __do_trace_foo() directly */
nit: trace_call_foo() instead of trace_invoke_foo()?
Anyway looks good to me.
Acked-by: Masami Hiramatsu (Google) mhiramat@kernel.org
Three locations are updated:
- __DECLARE_TRACE: invoke form omits static_branch_unlikely, retains the LOCKDEP RCU-watching assertion.
- __DECLARE_TRACE_SYSCALL: same, plus retains might_fault().
- !TRACEPOINTS_ENABLED stub: empty no-op so callers compile cleanly when tracepoints are compiled out.
Suggested-by: Steven Rostedt rostedt@goodmis.org Suggested-by: Peter Zijlstra peterz@infradead.org Signed-off-by: Vineeth Pillai (Google) vineeth@bitbyteword.org Assisted-by: Claude:claude-sonnet-4-6
include/linux/tracepoint.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index 22ca1c8b54f32..ed969705341f1 100644 --- a/include/linux/tracepoint.h +++ b/include/linux/tracepoint.h @@ -294,6 +294,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) WARN_ONCE(!rcu_is_watching(), \ "RCU not watching for tracepoint"); \ } \
- } \
- static inline void trace_call__##name(proto) \
- { \
__do_trace_##name(args); \#define __DECLARE_TRACE_SYSCALL(name, proto, args, data_proto) \ @@ -313,6 +317,11 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) WARN_ONCE(!rcu_is_watching(), \ "RCU not watching for tracepoint"); \ } \
- } \
- static inline void trace_call__##name(proto) \
- { \
might_fault(); \__do_trace_##name(args); \/* @@ -398,6 +407,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) #define __DECLARE_TRACE_COMMON(name, proto, args, data_proto) \ static inline void trace_##name(proto) \ { } \
- static inline void trace_call__##name(proto) \
- { } \ static inline int \ register_trace_##name(void (*probe)(data_proto), \ void *data) \
-- 2.53.0
On Mon, 23 Mar 2026 12:00:19 -0400 "Vineeth Pillai (Google)" vineeth@bitbyteword.org wrote:
When a caller already guards a tracepoint with an explicit enabled check:
if (trace_foo_enabled() && cond) trace_foo(args);
Thanks Vineeth!
I'm going to start pulling in this series. I'll take the first patch, and then any patch that has an Acked-by or Reviewed-by from the maintainer.
For patches without acks, I'll leave alone and then after the first patch gets merged into mainline, the maintainers could pull in their own patches at their own convenience. Unless of course they speak up now if they want me to take them ;-)
-- Steve
On Mon, 23 Mar 2026 12:00:19 -0400 "Vineeth Pillai (Google)" vineeth@bitbyteword.org wrote:
if (trace_foo_enabled() && cond) trace_call__foo(args); /* calls __do_trace_foo() directly */
Hi Vineeth,
Could you rebase this series on top of 7.1-rc1 when it comes out? Several of these patches were accepted already. Obviously drop those. They were the patches that added the feature, and any where the maintainer acked the patch.
Now that the feature has been accepted, if you post the patch series again after 7.1-rc1 with all the patches that haven't been accepted yet, then the maintainers can simply take them directly. As the feature is now accepted, there's no dependency on it, and they don't need to go through the tracing tree.
Thanks,
-- Steve
On Sat, Apr 18, 2026 at 7:05 PM Steven Rostedt rostedt@goodmis.org wrote:
On Mon, 23 Mar 2026 12:00:19 -0400 "Vineeth Pillai (Google)" vineeth@bitbyteword.org wrote:
if (trace_foo_enabled() && cond) trace_call__foo(args); /* calls __do_trace_foo() directly */
Hi Vineeth,
Could you rebase this series on top of 7.1-rc1 when it comes out? Several of these patches were accepted already. Obviously drop those. They were the patches that added the feature, and any where the maintainer acked the patch.
Now that the feature has been accepted, if you post the patch series again after 7.1-rc1 with all the patches that haven't been accepted yet, then the maintainers can simply take them directly. As the feature is now accepted, there's no dependency on it, and they don't need to go through the tracing tree.
Sure, will do. Thanks for merging this feature.
Thanks, Vineeth
linaro-mm-sig@lists.linaro.org
-
Masami Hiramatsu -
Steven Rostedt -
Vineeth Pillai (Google) -
Vineeth Remanan Pillai