* Re: [PATCH 06/15] cpufreq: Use trace_invoke_##name() at guarded tracepoint call sites
From: Rafael J. Wysocki @ 2026-03-12 18:58 UTC (permalink / raw)
To: vineeth
Cc: Steven Rostedt, Peter Zijlstra, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki, Viresh Kumar,
Srinivas Pandruvada, Len Brown, linux-pm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260312150523.2054552-7-vineeth@bitbyteword.org>
On Thu, Mar 12, 2026 at 4:05 PM Vineeth Pillai (Google)
<vineeth@bitbyteword.org> wrote:
>
> Replace trace_foo() with the new trace_invoke_foo() at sites already
> guarded by trace_foo_enabled(), avoiding a redundant
> static_branch_unlikely() re-evaluation inside the tracepoint.
> trace_invoke_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
Fine with me, so
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> # cpufreq
> ---
> drivers/cpufreq/amd-pstate.c | 10 +++++-----
> drivers/cpufreq/cpufreq.c | 2 +-
> drivers/cpufreq/intel_pstate.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5aa9fcd80cf51..3fa40a32ef6b5 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -247,7 +247,7 @@ static int msr_update_perf(struct cpufreq_policy *policy, u8 min_perf,
> if (trace_amd_pstate_epp_perf_enabled()) {
> union perf_cached perf = READ_ONCE(cpudata->perf);
>
> - trace_amd_pstate_epp_perf(cpudata->cpu,
> + trace_invoke_amd_pstate_epp_perf(cpudata->cpu,
> perf.highest_perf,
> epp,
> min_perf,
> @@ -298,7 +298,7 @@ static int msr_set_epp(struct cpufreq_policy *policy, u8 epp)
> if (trace_amd_pstate_epp_perf_enabled()) {
> union perf_cached perf = cpudata->perf;
>
> - trace_amd_pstate_epp_perf(cpudata->cpu, perf.highest_perf,
> + trace_invoke_amd_pstate_epp_perf(cpudata->cpu, perf.highest_perf,
> epp,
> FIELD_GET(AMD_CPPC_MIN_PERF_MASK,
> cpudata->cppc_req_cached),
> @@ -343,7 +343,7 @@ static int shmem_set_epp(struct cpufreq_policy *policy, u8 epp)
> if (trace_amd_pstate_epp_perf_enabled()) {
> union perf_cached perf = cpudata->perf;
>
> - trace_amd_pstate_epp_perf(cpudata->cpu, perf.highest_perf,
> + trace_invoke_amd_pstate_epp_perf(cpudata->cpu, perf.highest_perf,
> epp,
> FIELD_GET(AMD_CPPC_MIN_PERF_MASK,
> cpudata->cppc_req_cached),
> @@ -507,7 +507,7 @@ static int shmem_update_perf(struct cpufreq_policy *policy, u8 min_perf,
> if (trace_amd_pstate_epp_perf_enabled()) {
> union perf_cached perf = READ_ONCE(cpudata->perf);
>
> - trace_amd_pstate_epp_perf(cpudata->cpu,
> + trace_invoke_amd_pstate_epp_perf(cpudata->cpu,
> perf.highest_perf,
> epp,
> min_perf,
> @@ -588,7 +588,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u8 min_perf,
> }
>
> if (trace_amd_pstate_perf_enabled() && amd_pstate_sample(cpudata)) {
> - trace_amd_pstate_perf(min_perf, des_perf, max_perf, cpudata->freq,
> + trace_invoke_amd_pstate_perf(min_perf, des_perf, max_perf, cpudata->freq,
> cpudata->cur.mperf, cpudata->cur.aperf, cpudata->cur.tsc,
> cpudata->cpu, fast_switch);
> }
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 277884d91913c..cf57aeb503790 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2222,7 +2222,7 @@ unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
>
> if (trace_cpu_frequency_enabled()) {
> for_each_cpu(cpu, policy->cpus)
> - trace_cpu_frequency(freq, cpu);
> + trace_invoke_cpu_frequency(freq, cpu);
> }
>
> return freq;
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 11c58af419006..a0da9b31c4ffe 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -3132,7 +3132,7 @@ static void intel_cpufreq_trace(struct cpudata *cpu, unsigned int trace_type, in
> return;
>
> sample = &cpu->sample;
> - trace_pstate_sample(trace_type,
> + trace_invoke_pstate_sample(trace_type,
> 0,
> old_pstate,
> cpu->pstate.current_pstate,
> --
> 2.53.0
>
^ permalink raw reply
* [PATCH 3/3] lib/bootconfig: fix snprintf truncation check in xbc_node_compose_key_after()
From: Josh Law @ 2026-03-12 18:45 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton
Cc: Josh Law, linux-kernel, linux-trace-kernel
In-Reply-To: <20260312184520.24399-1-objecting@objecting.org>
snprintf() returns the number of characters that would have been
written excluding the NUL terminator. Output is truncated when the
return value is >= the buffer size, not just > the buffer size.
When ret == size, the current code takes the non-truncated path,
advancing buf by ret and reducing size to 0. This is wrong because
the output was actually truncated (the last character was replaced by
NUL). Fix by using >= so the truncation path is taken correctly.
Signed-off-by: Josh Law <objecting@objecting.org>
---
lib/bootconfig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 62b4ed7a0ba6..b0ef1e74e98a 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -316,7 +316,7 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
depth ? "." : "");
if (ret < 0)
return ret;
- if (ret > size) {
+ if (ret >= size) {
size = 0;
} else {
size -= ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()
From: Josh Law @ 2026-03-12 18:45 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton
Cc: Josh Law, linux-kernel, linux-trace-kernel
In-Reply-To: <20260312184520.24399-1-objecting@objecting.org>
The bounds check for brace_index happens after the array write.
While the current call pattern prevents an actual out-of-bounds
access (the previous call would have returned an error), the
write-before-check pattern is fragile and would become a real
out-of-bounds write if the error return were ever not propagated.
Move the bounds check before the array write so the function is
self-contained and safe regardless of caller behavior.
Signed-off-by: Josh Law <objecting@objecting.org>
---
lib/bootconfig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index a1e6a2e14b01..62b4ed7a0ba6 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
static int __init __xbc_open_brace(char *p)
{
/* Push the last key as open brace */
- open_brace[brace_index++] = xbc_node_index(last_parent);
if (brace_index >= XBC_DEPTH_MAX)
return xbc_parse_error("Exceed max depth of braces", p);
+ open_brace[brace_index++] = xbc_node_index(last_parent);
return 0;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 1/3] lib/bootconfig: fix off-by-one in xbc_verify_tree() unclosed brace error
From: Josh Law @ 2026-03-12 18:45 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton
Cc: Josh Law, linux-kernel, linux-trace-kernel
__xbc_open_brace() pushes entries with post-increment
(open_brace[brace_index++]), so brace_index always points one past
the last valid entry. xbc_verify_tree() reads open_brace[brace_index]
to report which brace is unclosed, but this is one past the last
pushed entry and contains stale/zero data, causing the error message
to reference the wrong node.
Use open_brace[brace_index - 1] to correctly identify the unclosed
brace. brace_index is known to be > 0 here since we are inside the
if (brace_index) guard.
Signed-off-by: Josh Law <objecting@objecting.org>
---
lib/bootconfig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 2bcd5c2aa87e..a1e6a2e14b01 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -802,7 +802,7 @@ static int __init xbc_verify_tree(void)
/* Brace closing */
if (brace_index) {
- n = &xbc_nodes[open_brace[brace_index]];
+ n = &xbc_nodes[open_brace[brace_index - 1]];
return xbc_parse_error("Brace is not closed",
xbc_node_get_data(n));
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] tracing: Generate undef symbols allowlist for simple_ring_buffer
From: Steven Rostedt @ 2026-03-12 18:43 UTC (permalink / raw)
To: Vincent Donnefort
Cc: maz, arnd, nathan, linux-trace-kernel, kvmarm, kernel-team
In-Reply-To: <20260312182010.111013-1-vdonnefort@google.com>
On Thu, 12 Mar 2026 18:20:10 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Compiler and tooling-generated symbols are difficult to maintain
> across all supported architectures. Make the allowlist more robust by
> replacing the harcoded list with a mechanism that automatically detects
> these symbols.
>
> This mechanism generates a C function designed to trigger common
> compiler-inserted symbols.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
>
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index beb15936829d..3b427b76434a 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -136,17 +136,37 @@ obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
> # simple_ring_buffer is used by the pKVM hypervisor which does not have access
> # to all kernel symbols. Fail the build if forbidden symbols are found.
> #
> -UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov __aeabi_unwind
> -UNDEFINED_ALLOWLIST += __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer llvm_gcda llvm_gcov
> -UNDEFINED_ALLOWLIST += .TOC\. __clear_pages_unrolled __memmove copy_page warn_slowpath_fmt
> -UNDEFINED_ALLOWLIST += ftrace_likely_update __hwasan_load __hwasan_store __hwasan_tag_memory
> -UNDEFINED_ALLOWLIST += warn_bogus_irq_restore __stack_chk_guard
> -UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
> +# undefsyms_base generates a set of compiler and tooling-generated symbols that can
> +# safely be ignored for simple_ring_buffer.
> +#
> +$(obj)/undefsyms_base.c: FORCE
> + $(Q)echo '#include <asm/page.h>' > $@
> + $(Q)echo '#include <asm/local.h>' >> $@
> + $(Q)echo 'static char page[PAGE_SIZE] __aligned(PAGE_SIZE);' >> $@
> + $(Q)echo 'void undefsyms_base(int n);' >> $@
> + $(Q)echo 'void undefsyms_base(int n) {' >> $@
> + $(Q)echo ' char buffer[256] = { 0 };' >> $@
> + $(Q)echo ' u32 u = 0;' >> $@
> + $(Q)echo ' memset((char * volatile)page, 8, PAGE_SIZE);' >> $@
> + $(Q)echo ' memset((char * volatile)buffer, 8, sizeof(buffer));' >> $@
> + $(Q)echo ' cmpxchg((u32 * volatile)&u, 0, 8);' >> $@
> + $(Q)echo ' WARN_ON(n == 0xdeadbeef);' >> $@
> + $(Q)echo '}' >> $@
Looking at the scripts/Makefile.* files, it looks like the proper way to do
C files is to make it a single command:
Perhaps something like:
$(obj)/undefsyms_base.c: FORCE
$(Q)( \
echo '#include <asm/page.h>'; \
echo '#include <asm/local.h>'; \
echo 'static char page[PAGE_SIZE] __aligned(PAGE_SIZE);'; \
echo 'void undefsyms_base(int n);'; \
echo 'void undefsyms_base(int n) {'; \
echo ' char buffer[256] = { 0 };'; \
echo ' u32 u = 0;'; \
echo ' memset((char * volatile)page, 8, PAGE_SIZE);'; \
echo ' memset((char * volatile)buffer, 8, sizeof(buffer));'; \
echo ' cmpxchg((u32 * volatile)&u, 0, 8);'; \
echo ' WARN_ON(n == 0xdeadbeef);'; \
echo '}' ) > $@
-- Steve
> +
> +clean-files += undefsyms_base.c
> +targets += undefsyms_base.c
> +
> +$(obj)/undefsyms_base.o: $(obj)/undefsyms_base.c
> +
> +extra-y += undefsyms_base.o
> +
> +UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sanitizer __tsan __ubsan __x86_indirect_thunk \
> + $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '{print $$2}')
>
> quiet_cmd_check_undefined = NM $<
> - cmd_check_undefined = test -z "`$(NM) -u $< | grep -v $(UNDEFINED_ALLOWLIST)`"
> + cmd_check_undefined = test -z "`$(NM) -u $< | grep -v $(addprefix -e ,$(UNDEFINED_ALLOWLIST))`"
>
> -$(obj)/%.o.checked: $(obj)/%.o FORCE
> +$(obj)/%.o.checked: $(obj)/%.o $(obj)/undefsyms_base.o FORCE
> $(call if_changed,check_undefined)
>
> always-$(CONFIG_SIMPLE_RING_BUFFER) += simple_ring_buffer.o.checked
>
> base-commit: 33f2e266515717c4b2df585dadefa0525557726c
^ permalink raw reply
* [PATCH] tracing: Generate undef symbols allowlist for simple_ring_buffer
From: Vincent Donnefort @ 2026-03-12 18:20 UTC (permalink / raw)
To: maz
Cc: rostedt, arnd, nathan, linux-trace-kernel, kvmarm, kernel-team,
Vincent Donnefort
Compiler and tooling-generated symbols are difficult to maintain
across all supported architectures. Make the allowlist more robust by
replacing the harcoded list with a mechanism that automatically detects
these symbols.
This mechanism generates a C function designed to trigger common
compiler-inserted symbols.
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index beb15936829d..3b427b76434a 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -136,17 +136,37 @@ obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
# simple_ring_buffer is used by the pKVM hypervisor which does not have access
# to all kernel symbols. Fail the build if forbidden symbols are found.
#
-UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov __aeabi_unwind
-UNDEFINED_ALLOWLIST += __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer llvm_gcda llvm_gcov
-UNDEFINED_ALLOWLIST += .TOC\. __clear_pages_unrolled __memmove copy_page warn_slowpath_fmt
-UNDEFINED_ALLOWLIST += ftrace_likely_update __hwasan_load __hwasan_store __hwasan_tag_memory
-UNDEFINED_ALLOWLIST += warn_bogus_irq_restore __stack_chk_guard
-UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
+# undefsyms_base generates a set of compiler and tooling-generated symbols that can
+# safely be ignored for simple_ring_buffer.
+#
+$(obj)/undefsyms_base.c: FORCE
+ $(Q)echo '#include <asm/page.h>' > $@
+ $(Q)echo '#include <asm/local.h>' >> $@
+ $(Q)echo 'static char page[PAGE_SIZE] __aligned(PAGE_SIZE);' >> $@
+ $(Q)echo 'void undefsyms_base(int n);' >> $@
+ $(Q)echo 'void undefsyms_base(int n) {' >> $@
+ $(Q)echo ' char buffer[256] = { 0 };' >> $@
+ $(Q)echo ' u32 u = 0;' >> $@
+ $(Q)echo ' memset((char * volatile)page, 8, PAGE_SIZE);' >> $@
+ $(Q)echo ' memset((char * volatile)buffer, 8, sizeof(buffer));' >> $@
+ $(Q)echo ' cmpxchg((u32 * volatile)&u, 0, 8);' >> $@
+ $(Q)echo ' WARN_ON(n == 0xdeadbeef);' >> $@
+ $(Q)echo '}' >> $@
+
+clean-files += undefsyms_base.c
+targets += undefsyms_base.c
+
+$(obj)/undefsyms_base.o: $(obj)/undefsyms_base.c
+
+extra-y += undefsyms_base.o
+
+UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sanitizer __tsan __ubsan __x86_indirect_thunk \
+ $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '{print $$2}')
quiet_cmd_check_undefined = NM $<
- cmd_check_undefined = test -z "`$(NM) -u $< | grep -v $(UNDEFINED_ALLOWLIST)`"
+ cmd_check_undefined = test -z "`$(NM) -u $< | grep -v $(addprefix -e ,$(UNDEFINED_ALLOWLIST))`"
-$(obj)/%.o.checked: $(obj)/%.o FORCE
+$(obj)/%.o.checked: $(obj)/%.o $(obj)/undefsyms_base.o FORCE
$(call if_changed,check_undefined)
always-$(CONFIG_SIMPLE_RING_BUFFER) += simple_ring_buffer.o.checked
base-commit: 33f2e266515717c4b2df585dadefa0525557726c
--
2.53.0.851.ga537e3e6e9-goog
^ permalink raw reply related
* Re: [PATCH v3 1/4] tracing/preemptirq: Optimize preempt_disable/enable() tracepoint overhead
From: Wander Lairson Costa @ 2026-03-12 17:19 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton,
open list:SCHEDULER, open list:TRACING, acme, williams, gmonaco
In-Reply-To: <20260311193503.GS606826@noisy.programming.kicks-ass.net>
On Wed, Mar 11, 2026 at 08:35:03PM +0100, Peter Zijlstra wrote:
> On Wed, Mar 11, 2026 at 09:50:15AM -0300, Wander Lairson Costa wrote:
>
> > +extern void __trace_preempt_on(void);
> > +extern void __trace_preempt_off(void);
> > +
> > +DECLARE_TRACEPOINT(preempt_enable);
> > +DECLARE_TRACEPOINT(preempt_disable);
> > +
> > +#define __preempt_trace_enabled(type, val) \
> > + (tracepoint_enabled(preempt_##type) && preempt_count() == (val))
> > +
> > +static __always_inline void preempt_count_add(int val)
> > +{
> > + __preempt_count_add(val);
> > +
> > + if (__preempt_trace_enabled(disable, val))
> > + __trace_preempt_off();
> > +}
> > +
> > +static __always_inline void preempt_count_sub(int val)
> > +{
> > + if (__preempt_trace_enabled(enable, val))
> > + __trace_preempt_on();
> > +
> > + __preempt_count_sub(val);
> > +}
> > #else
> > #define preempt_count_add(val) __preempt_count_add(val)
> > #define preempt_count_sub(val) __preempt_count_sub(val)
> > #define preempt_count_dec_and_test() __preempt_count_dec_and_test()
> > #endif
> >
> > +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> > +#define preempt_count_dec_and_test() \
> > + ({ preempt_count_sub(1); should_resched(0); })
> > +#endif
>
> Why!?!
>
> Why can't you simply have:
>
> static __always_inline bool preempt_count_dec_and_test(void)
> {
> if (__preempt_trace_enabled(enable, 1))
> __trace_preempt_on();
>
> return __preempt_count_dec_and_test();
> }
Indeed it improved the generated code. Thanks a lot.
>
> Also, given how !x86 architectures were just complaining about how
> terrible their preempt_emable() is, I'm really not liking this much at
> all.
>
I will look deeper in arm64 and riscv generated code. If there are other
specific architectures that concerns you, please let me know.
> Currently the x86 preempt_disable() is _1_ instruction and
> preempt_enable() is all of 3. Adding in these tracepoints will bloat
> every single such site by at least another 4-5.
>
Yes, there is a tradeoff. As I said before, I am looking at the hot path
(tracepoint no activated). Furthermore, when CONFIG_TRACE_PREEMPT_TOGGLE
and CONFIG_TRACE_IRQFLAGS_TOGGLE are off (default config), the code
generated is the same, byte by byte, of the stock kernel.
> That's significant bloat, for really very little gain. Realistically
> nobody is going to need these.
>
Of course, I can't speak for others, but more than once I debugged issues
that those tracepoints had made my life far easier. Those cases convinced
me that such a feature would be worth it. But if you don't see
value and will reject the patches no matter what, nothing can be done,
and I will have to accept defeat.
^ permalink raw reply
* Re: [PATCH v3 4/4] trace/preemptirq: Implement trace_irqflags hooks
From: Wander Lairson Costa @ 2026-03-12 17:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton, open list,
open list:TRACING, acme, williams, gmonaco
In-Reply-To: <20260311194305.GT606826@noisy.programming.kicks-ass.net>
On Wed, Mar 11, 2026 at 08:43:05PM +0100, Peter Zijlstra wrote:
> On Wed, Mar 11, 2026 at 09:50:18AM -0300, Wander Lairson Costa wrote:
> > +#define local_irq_enable() \
> > + do { \
> > + if (tracepoint_enabled(irq_enable)) \
> > + trace_local_irq_enable(); \
>
> I'm thinking you didn't even look at the assembly generated :/
Yes, I did. But I was more concerned with the hot path, making sure it
only adds the NOP instruction generated by the static_key machinery.
>
> Otherwise you would have written this like:
>
> if (tracepoint_enabled(irq_enable))
> __do_trace_local_irq_enable();
>
> > + raw_local_irq_enable(); \
> > + } while (0)
>
Steve already opposed to using internal functions. When trace invoke
public API is available, we can always come back and replace the call.
> Again, this was one instruction, and you clearly didn't bother looking
> at the mess you've generated :/
>
I might have missed something (as was the case for preempt_enable), but
I spent quite some time looking at the assembly code. I did my best, but
there lots of people far more skilled than me. And patch submission is
the way to connect to these people.
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Steven Rostedt @ 2026-03-12 17:02 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Vineeth Remanan Pillai, Mathieu Desnoyers, Peter Zijlstra,
Dmitry Ilvokhin, Masami Hiramatsu, Ingo Molnar, Jens Axboe,
io-uring, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Marcelo Ricardo Leitner, Xin Long, Jon Maloy, Aaron Conole,
Eelco Chaudron, Ilya Maximets, netdev, bpf, linux-sctp,
tipc-discussion, dev, Oded Gabbay, Koby Elbaz, dri-devel,
Rafael J. Wysocki, Viresh Kumar, Gautham R. Shenoy, Huang Rui,
Mario Limonciello, Len Brown, Srinivas Pandruvada, linux-pm,
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Christian König,
Sumit Semwal, linaro-mm-sig, Eddie James, Andrew Jeffery,
Joel Stanley, linux-fsi, David Airlie, Simona Vetter,
Alex Deucher, Danilo Krummrich, Matthew Brost, Philipp Stanner,
Harry Wentland, Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires,
linux-input, Wolfram Sang, linux-i2c, Mark Brown,
Michael Hennerich, Nuno Sá, linux-spi, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, Chris Mason, David Sterba,
linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <CAEf4BzbnfyhCqp0ne=2gRnVxp-mdGmuZwDeFRyhRYH+eDcz2-w@mail.gmail.com>
On Thu, 12 Mar 2026 09:54:29 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > emit_trace_foo()
> > > __trace_foo()
>
> this seems like the best approach, IMO. double-underscored variants
> are usually used for some specialized/internal version of a function
> when we know that some conditions are correct (e.g., lock is already
> taken, or something like that). Which fits here: trace_xxx() will
> check if tracepoint is enabled, while __trace_xxx() will not check and
> just invoke the tracepoint? It's short, it's distinct, and it says "I
> know what I am doing".
Honestly, I consider double underscore as internal only and not something
anyone but the subsystem maintainers use.
This, is a normal function where it's just saying: If you have it already
enabled, then you can use this. Thus, I don't think it qualifies as a "you
know what you are doing".
Perhaps: call_trace_foo() ?
-- Steve
^ permalink raw reply
* Re: [PATCH 13/15] spi: Use trace_invoke_##name() at guarded tracepoint call sites
From: Steven Rostedt @ 2026-03-12 17:00 UTC (permalink / raw)
To: Mark Brown
Cc: Vineeth Pillai (Google), Peter Zijlstra, Michael Hennerich,
Nuno Sá, David Lechner, linux-spi, linux-kernel,
linux-trace-kernel
In-Reply-To: <06438818-2e3e-426f-aec5-ea6344e4b057@sirena.org.uk>
On Thu, 12 Mar 2026 16:54:00 +0000
Mark Brown <broonie@kernel.org> wrote:
> Possibly an _unchecked or something? Honestly the suggestion someone
> had for _do seemed OK to me. Part of it is that I wouldn't think of
> tracepoints as being something that I'd call.
The "__do_trace.." is an internal function I don't want to expose.
I'm thinking of: call_trace_foo(), as that should be pretty obvious to what
it is.
I want to avoid the do_trace_foo() because that's usually the name of the
wrapper code that is done in header files. Where the header calls:
do_trace_foo()
and the C file has:
void do_trace_foo(..)
{
trace_foo(..);
}
Which could be:
void do_trace_foo(..)
{
call_trace(..);
}
And remove the static branch there.
-- Steve
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Jason Gunthorpe @ 2026-03-12 16:54 UTC (permalink / raw)
To: James Bottomley
Cc: Kuan-Wei Chiu, Philipp Hahn, amd-gfx, apparmor, bpf, ceph-devel,
cocci, dm-devel, dri-devel, gfs2, intel-gfx, intel-wired-lan,
iommu, kvm, linux-arm-kernel, linux-block, linux-bluetooth,
linux-btrfs, linux-cifs, linux-clk, linux-erofs, linux-ext4,
linux-fsdevel, linux-gpio, linux-hyperv, linux-input,
linux-kernel, linux-leds, linux-media, linux-mips, linux-mm,
linux-modules, linux-mtd, linux-nfs, linux-omap, linux-phy,
linux-pm, linux-rockchip, linux-s390, linux-scsi, linux-sctp,
linux-security-module, linux-sh, linux-sound, linux-stm32,
linux-trace-kernel, linux-usb, linux-wireless, netdev, ntfs3,
samba-technical, sched-ext, target-devel, tipc-discussion, v9fs
In-Reply-To: <f5688b895eaebabae6545a0d9baf8f1404e8454e.camel@HansenPartnership.com>
On Thu, Mar 12, 2026 at 11:32:37AM -0400, James Bottomley wrote:
> On Thu, 2026-03-12 at 09:57 -0300, Jason Gunthorpe wrote:
> > On Wed, Mar 11, 2026 at 02:40:36AM +0800, Kuan-Wei Chiu wrote:
> >
> > > IMHO, the necessity of IS_ERR_OR_NULL() often highlights a
> > > confusing or flawed API design. It usually implies that the caller
> > > is unsure whether a failure results in an error pointer or a NULL
> > > pointer.
> >
> > +1
> >
> > IS_ERR_OR_NULL() should always be looked on with suspicion. Very
> > little should be returning some tri-state 'ERR' 'NULL' 'SUCCESS'
> > pointer. What does the middle condition even mean? IS_ERR_OR_NULL()
> > implies ERR and NULL are semanticly the same, so fix the things to
> > always use ERR.
>
> Not in any way supporting the original patch. However, the pattern
> ERR, NULL, PTR is used extensively in the dentry code of filesystems.
> See the try_lookup..() set of functions in fs/namei.c
>
> The meaning is
>
> PTR - I found it
> NULL - It definitely doesn't exist
> ERR - something went wrong during the lookup.
>
> So I don't think you can blanket say this pattern is wrong.
Lots of places also would return ENOENT, I'd argue that is easier to
use..
But yes, I did use the word "suspicion" not blanket wrong :)
Jason
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Andrii Nakryiko @ 2026-03-12 16:54 UTC (permalink / raw)
To: Vineeth Remanan Pillai
Cc: Mathieu Desnoyers, Steven Rostedt, Peter Zijlstra,
Dmitry Ilvokhin, Masami Hiramatsu, Ingo Molnar, Jens Axboe,
io-uring, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Marcelo Ricardo Leitner, Xin Long, Jon Maloy, Aaron Conole,
Eelco Chaudron, Ilya Maximets, netdev, bpf, linux-sctp,
tipc-discussion, dev, Oded Gabbay, Koby Elbaz, dri-devel,
Rafael J. Wysocki, Viresh Kumar, Gautham R. Shenoy, Huang Rui,
Mario Limonciello, Len Brown, Srinivas Pandruvada, linux-pm,
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Christian König,
Sumit Semwal, linaro-mm-sig, Eddie James, Andrew Jeffery,
Joel Stanley, linux-fsi, David Airlie, Simona Vetter,
Alex Deucher, Danilo Krummrich, Matthew Brost, Philipp Stanner,
Harry Wentland, Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires,
linux-input, Wolfram Sang, linux-i2c, Mark Brown,
Michael Hennerich, Nuno Sá, linux-spi, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, Chris Mason, David Sterba,
linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <CAO7JXPjnnruhM5oC6xMgnYaQ9efzYFqMCFiJLNM3HCQ+ZeCiJw@mail.gmail.com>
On Thu, Mar 12, 2026 at 9:15 AM Vineeth Remanan Pillai
<vineeth@bitbyteword.org> wrote:
>
> On Thu, Mar 12, 2026 at 11:49 AM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> >
> > On 2026-03-12 11:40, Steven Rostedt wrote:
> > > On Thu, 12 Mar 2026 11:28:07 -0400
> > > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> > >
> > >>> Note, Vineeth came up with the naming. I would have done "do" but when I
> > >>> saw "invoke" I thought it sounded better.
> > >>
> > >> It works as long as you don't have a tracing subsystem called
> > >> "invoke", then you get into identifier clash territory.
> > >
> > > True. Perhaps we should do the double underscore trick.
> > >
> > > Instead of: trace_invoke_foo()
> > >
> > > use: trace_invoke__foo()
> > >
> > >
> > > Which will make it more visible to what the trace event is.
> > >
> > > Hmm, we probably should have used: trace__foo() for all tracepoints, as
> > > there's still functions that are called trace_foo() that are not
> > > tracepoints :-p
> >
> > One certain way to eliminate identifier clash would be to go for a
> > prefix to "trace_", e.g.
> >
> > do_trace_foo()
> > call_trace_foo()
>
> This was the initial idea, but it had conflict in the existing source:
> call_trace_sched_update_nr_running. do_trace_##name also had
> collisions when I checked. So, went with trace_invoke_##name. Did not
> check rest of the suggestions here though.
>
> Thanks,
> Vineeth
>
> > emit_trace_foo()
> > __trace_foo()
this seems like the best approach, IMO. double-underscored variants
are usually used for some specialized/internal version of a function
when we know that some conditions are correct (e.g., lock is already
taken, or something like that). Which fits here: trace_xxx() will
check if tracepoint is enabled, while __trace_xxx() will not check and
just invoke the tracepoint? It's short, it's distinct, and it says "I
know what I am doing".
> > invoke_trace_foo()
> > dispatch_trace_foo()
> >
> > Thanks,
> >
> > Mathieu
> >
> >
> >
> > --
> > Mathieu Desnoyers
> > EfficiOS Inc.
> > https://www.efficios.com
>
^ permalink raw reply
* Re: [PATCH 13/15] spi: Use trace_invoke_##name() at guarded tracepoint call sites
From: Mark Brown @ 2026-03-12 16:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: Vineeth Pillai (Google), Peter Zijlstra, Michael Hennerich,
Nuno Sá, David Lechner, linux-spi, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260312114254.6467c03d@gandalf.local.home>
[-- Attachment #1: Type: text/plain, Size: 521 bytes --]
On Thu, Mar 12, 2026 at 11:42:54AM -0400, Steven Rostedt wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > though not loving the naming here, I'll have a hard time figuring out
> > what the weird call is about next time I look at that code
> Would:
> trace_call__foo()
> Be better?
> Or do you have another name we should use?
Possibly an _unchecked or something? Honestly the suggestion someone
had for _do seemed OK to me. Part of it is that I wouldn't think of
tracepoints as being something that I'd call.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 8/8] mm/hmm: Indicate that HMM requires DMA coherency
From: Leon Romanovsky @ 2026-03-12 16:50 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Marek Szyprowski, Robin Murphy, Michael S. Tsirkin, Petr Tesarik,
Jonathan Corbet, Shuah Khan, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Joerg Roedel, Will Deacon, Andrew Morton,
iommu, linux-kernel, linux-doc, virtualization, linux-rdma,
linux-trace-kernel, linux-mm
In-Reply-To: <20260312122645.GG1469476@ziepe.ca>
On Thu, Mar 12, 2026 at 09:26:45AM -0300, Jason Gunthorpe wrote:
> On Wed, Mar 11, 2026 at 09:08:51PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> >
> > HMM mirroring can work on coherent systems without SWIOTLB path only.
> > Until introduction of DMA_ATTR_REQUIRE_COHERENT, there was no reliable
> > way to indicate that and various approximation was done:
>
> HMM is fundamentally about allowing a sophisticated device to
> independently DMA to a process's memory concurrently with the CPU
> accessing the same memory. It is similar to SVA but does not rely on
> IOMMU support. Since the entire use model is concurrent access to the
> same memory it becomes fatally broken as a uAPI if SWIOTLB is
> replacing the memory, or the CPU caches are incoherent with DMA.
>
> Till now there was no reliable way to indicate that and various
> approximation was done:
>
> > int hmm_dma_map_alloc(struct device *dev, struct hmm_dma_map *map,
> > size_t nr_entries, size_t dma_entry_size)
> > {
> > <...>
> > /*
> > * The HMM API violates our normal DMA buffer ownership rules and can't
> > * transfer buffer ownership. The dma_addressing_limited() check is a
> > * best approximation to ensure no swiotlb buffering happens.
> > */
> > dma_need_sync = !dev->dma_skip_sync;
> > if (dma_need_sync || dma_addressing_limited(dev))
> > return -EOPNOTSUPP;
>
> Can it get dropped now then?
Better not, it allows us to reject caller much earlier than DMA mapping
flow. It is much saner to fail during UMEM ODP creation than start to
fail for ODP pagefaults.
>
> > So let's mark mapped buffers with DMA_ATTR_REQUIRE_COHERENT attribute
> > to prevent DMA debugging warnings for cache overlapped entries.
>
> Well, that isn't the main motivation, this prevents silent data
> corruption if someone tries to use hmm in a system with swiotlb or
> incoherent DMA,
>
> Looks OK otherwise
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Jason
>
^ permalink raw reply
* Re: [PATCH 12/15] i2c: Use trace_invoke_##name() at guarded tracepoint call sites
From: Wolfram Sang @ 2026-03-12 16:50 UTC (permalink / raw)
To: Vineeth Pillai (Google)
Cc: Steven Rostedt, Peter Zijlstra, linux-i2c, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260312150523.2054552-13-vineeth@bitbyteword.org>
On Thu, Mar 12, 2026 at 11:05:07AM -0400, Vineeth Pillai (Google) wrote:
> Replace trace_foo() with the new trace_invoke_foo() at sites already
> guarded by trace_foo_enabled(), avoiding a redundant
> static_branch_unlikely() re-evaluation inside the tracepoint.
> trace_invoke_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
From my side, this can go upstream with the rest of this series (when it
is ready). So:
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
^ permalink raw reply
* Re: [PATCH v2 5/8] dma-direct: prevent SWIOTLB path when DMA_ATTR_REQUIRE_COHERENT is set
From: Leon Romanovsky @ 2026-03-12 16:47 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Marek Szyprowski, Robin Murphy, Michael S. Tsirkin, Petr Tesarik,
Jonathan Corbet, Shuah Khan, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Joerg Roedel, Will Deacon, Andrew Morton,
iommu, linux-kernel, linux-doc, virtualization, linux-rdma,
linux-trace-kernel, linux-mm
In-Reply-To: <20260312122058.GE1469476@ziepe.ca>
On Thu, Mar 12, 2026 at 09:20:58AM -0300, Jason Gunthorpe wrote:
> On Wed, Mar 11, 2026 at 09:08:48PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> >
> > DMA_ATTR_REQUIRE_COHERENT indicates that SWIOTLB must not be used.
> > Ensure the SWIOTLB path is declined whenever the DMA direct path is
> > selected.
> >
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > ---
> > kernel/dma/direct.h | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
> > index e89f175e9c2d0..6184ff303f080 100644
> > --- a/kernel/dma/direct.h
> > +++ b/kernel/dma/direct.h
> > @@ -84,7 +84,7 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
> > dma_addr_t dma_addr;
> >
> > if (is_swiotlb_force_bounce(dev)) {
> > - if (attrs & DMA_ATTR_MMIO)
> > + if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> > return DMA_MAPPING_ERROR;
> >
> > return swiotlb_map(dev, phys, size, dir, attrs);
>
> Oh here it is, still maybe it is better to put it in swiotlb_map() ?
It is better do not call function which is not going to work. We have
same flow for DMA_ATTR_REQUIRE_COHERENT and DMA_ATTR_MMIO, which both
don't work with SWIOTLB.
Thanks
>
> Jason
>
^ permalink raw reply
* Re: [PATCH v2 4/8] dma-mapping: Introduce DMA require coherency attribute
From: Leon Romanovsky @ 2026-03-12 16:46 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Marek Szyprowski, Robin Murphy, Michael S. Tsirkin, Petr Tesarik,
Jonathan Corbet, Shuah Khan, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Joerg Roedel, Will Deacon, Andrew Morton,
iommu, linux-kernel, linux-doc, virtualization, linux-rdma,
linux-trace-kernel, linux-mm
In-Reply-To: <20260312121937.GD1469476@ziepe.ca>
On Thu, Mar 12, 2026 at 09:19:37AM -0300, Jason Gunthorpe wrote:
> On Wed, Mar 11, 2026 at 09:08:47PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> >
> > The mapping buffers which carry this attribute require DMA coherent system.
> > This means that they can't take SWIOTLB path, can perform CPU cache overlap
> > and doesn't perform cache flushing.
> >
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > ---
> > Documentation/core-api/dma-attributes.rst | 12 ++++++++++++
> > include/linux/dma-mapping.h | 7 +++++++
> > include/trace/events/dma.h | 3 ++-
> > kernel/dma/debug.c | 3 ++-
> > kernel/dma/mapping.c | 6 ++++++
> > 5 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/core-api/dma-attributes.rst b/Documentation/core-api/dma-attributes.rst
> > index 48cfe86cc06d7..69d094f144c70 100644
> > --- a/Documentation/core-api/dma-attributes.rst
> > +++ b/Documentation/core-api/dma-attributes.rst
> > @@ -163,3 +163,15 @@ data corruption.
> >
> > All mappings that share a cache line must set this attribute to suppress DMA
> > debug warnings about overlapping mappings.
> > +
> > +DMA_ATTR_REQUIRE_COHERENT
> > +-------------------------
> > +
> > +The mapping buffers which carry this attribute require DMA coherent system. This means
> > +that they can't take SWIOTLB path, can perform CPU cache overlap and doesn't perform
> > +cache flushing.
>
> DMA mapping requests with the DMA_ATTR_REQUIRE_COHERENT fail on any
> system where SWIOTLB or cache management is required. This should only
> be used to support uAPI designs that require continuous HW DMA
> coherence with userspace processes, for example RDMA and DRM. At a
> minimum the memory being mapped must be userspace memory from
> pin_user_pages() or similar.
>
> Drivers should consider using dma_mmap_pages() instead of this
> interface when building their uAPIs, when possible.
>
> It must never be used in an in-kernel driver that only works with
> kernal memory.
>
> > @@ -164,6 +164,9 @@ dma_addr_t dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
> > if (WARN_ON_ONCE(!dev->dma_mask))
> > return DMA_MAPPING_ERROR;
> >
> > + if (!dev_is_dma_coherent(dev) && (attrs & DMA_ATTR_REQUIRE_COHERENT))
> > + return DMA_MAPPING_ERROR;
>
> This doesn't capture enough conditions.. is_swiotlb_force_bounce(),
> dma_kmalloc_needs_bounce(), dma_capable(), etc all need to be blocked
> too
These checks exist in dma_direct_map_phys() and here is the common check
between direct and IOMMU modes.
Thanks
>
> So check it inside swiotlb_map() too, and maybe shift the above
> into the existing branches:
>
> if (!dev_is_dma_coherent(dev) &&
> !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
> arch_sync_dma_for_device(phys, size, dir);
>
> Jason
^ permalink raw reply
* Re: [PATCH 38/61] net: Prefer IS_ERR_OR_NULL over manual NULL check
From: Przemek Kitszel @ 2026-03-12 16:11 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Igor Russkikh, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
Russell King
In-Reply-To: <20260310-b4-is_err_or_null-v1-38-bd63b656022d@avm.de>
On 3/10/26 12:49, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Igor Russkikh <irusskikh@marvell.com>
> To: Andrew Lunn <andrew+netdev@lunn.ch>
> To: "David S. Miller" <davem@davemloft.net>
> To: Eric Dumazet <edumazet@google.com>
> To: Jakub Kicinski <kuba@kernel.org>
> To: Paolo Abeni <pabeni@redhat.com>
> To: Pavan Chebbi <pavan.chebbi@broadcom.com>
> To: Michael Chan <mchan@broadcom.com>
> To: Potnuri Bharat Teja <bharat@chelsio.com>
> To: Tony Nguyen <anthony.l.nguyen@intel.com>
> To: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> To: Taras Chornyi <taras.chornyi@plvision.eu>
> To: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> To: Alexandre Torgue <alexandre.torgue@foss.st.com>
> To: Iyappan Subramanian <iyappan@os.amperecomputing.com>
> To: Keyur Chudgar <keyur@os.amperecomputing.com>
> To: Quan Nguyen <quan@os.amperecomputing.com>
> To: Heiner Kallweit <hkallweit1@gmail.com>
> To: Russell King <linux@armlinux.org.uk>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
this is too trivial change, especially when combined like that
https://docs.kernel.org/process/maintainer-netdev.html#clean-up-patches
> ---
> drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
> drivers/net/ethernet/broadcom/tg3.c | 2 +-
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 3 +--
> drivers/net/ethernet/intel/ice/devlink/devlink.c | 2 +-
> drivers/net/ethernet/marvell/prestera/prestera_router.c | 2 +-
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> drivers/net/mdio/mdio-xgene.c | 2 +-
> drivers/net/usb/r8152.c | 2 +-
> 8 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index e270327e47fd804cc8ee5cfd53ed1b993c955c41..43edef35c4b1ff606b2f1519a07fad4c9a990ad4 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> @@ -810,7 +810,7 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
> }
>
> skb = aq_xdp_run_prog(aq_nic, &xdp, rx_ring, buff);
> - if (IS_ERR(skb) || !skb)
> + if (IS_ERR_OR_NULL(skb))
> continue;
>
> if (ptp_hwtstamp_len > 0)
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 2328fce336447eb4a796f9300ccc0ab536ff0a35..8ed79f34f03d81184dcc12e6eaff009cb8f7756e 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -7943,7 +7943,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
>
> segs = skb_gso_segment(skb, tp->dev->features &
> ~(NETIF_F_TSO | NETIF_F_TSO6));
> - if (IS_ERR(segs) || !segs) {
> + if (IS_ERR_OR_NULL(segs)) {
> tnapi->tx_dropped++;
> goto tg3_tso_bug_end;
> }
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> index 3307e50426819087ad985178c4a5383f16b8e7b4..1c8a6445d4b2e3535d8f1b7908dd02d8dd2f23fa 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> @@ -1032,8 +1032,7 @@ static void ch_flower_stats_handler(struct work_struct *work)
> do {
> rhashtable_walk_start(&iter);
>
> - while ((flower_entry = rhashtable_walk_next(&iter)) &&
> - !IS_ERR(flower_entry)) {
> + while (!IS_ERR_OR_NULL((flower_entry = rhashtable_walk_next(&iter)))) {
> ret = cxgb4_get_filter_counters(adap->port[0],
> flower_entry->filter_id,
> &packets, &bytes,
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> index 6c72bd15db6d75a1d4fa04ef8fefbd26fb6e84bd..3d08b9187fd76ca3198af28111b6f1c1765ea01e 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> @@ -791,7 +791,7 @@ static void ice_traverse_tx_tree(struct devlink *devlink, struct ice_sched_node
> node->parent->rate_node);
> }
>
> - if (rate_node && !IS_ERR(rate_node))
> + if (!IS_ERR_OR_NULL(rate_node))
> node->rate_node = rate_node;
>
> traverse_children:
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index b036b173a308b5f994ad8538eb010fa27196988c..4492938e8a3da91d32efe8d45ccbe2eb437c0e49 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -1061,7 +1061,7 @@ static void __prestera_k_arb_hw_state_upd(struct prestera_switch *sw,
> n = NULL;
> }
>
> - if (!IS_ERR(n) && n) {
> + if (!IS_ERR_OR_NULL(n)) {
> neigh_event_send(n, NULL);
> neigh_release(n);
> } else {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6827c99bde8c22db42b363d2d36ad6f26075ed50..356a4e9ce04b1fcf8786d7274d31ace404be2cf6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1275,7 +1275,7 @@ static int stmmac_init_phy(struct net_device *dev)
> /* Some DT bindings do not set-up the PHY handle. Let's try to
> * manually parse it
> */
> - if (!phy_fwnode || IS_ERR(phy_fwnode)) {
> + if (IS_ERR_OR_NULL(phy_fwnode)) {
> int addr = priv->plat->phy_addr;
> struct phy_device *phydev;
>
> diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c
> index a8f91a4b7fed0927ee14e408000cd3a2bfb9b09a..09b30b563295c6085dc1358ac361301e5cf6b2a8 100644
> --- a/drivers/net/mdio/mdio-xgene.c
> +++ b/drivers/net/mdio/mdio-xgene.c
> @@ -265,7 +265,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr)
> struct phy_device *phy_dev;
>
> phy_dev = get_phy_device(bus, phy_addr, false);
> - if (!phy_dev || IS_ERR(phy_dev))
> + if (IS_ERR_OR_NULL(phy_dev))
> return NULL;
>
> if (phy_device_register(phy_dev))
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0c83bbbea2e7c322ee6339893e281237663bd3ae..73f17ebd7d40007eec5004f887a46249defd28ab 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -2218,7 +2218,7 @@ static void r8152_csum_workaround(struct r8152 *tp, struct sk_buff *skb,
>
> features &= ~(NETIF_F_SG | NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
> segs = skb_gso_segment(skb, features);
> - if (IS_ERR(segs) || !segs)
> + if (IS_ERR_OR_NULL(segs))
> goto drop;
>
> __skb_queue_head_init(&seg_list);
>
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Vineeth Remanan Pillai @ 2026-03-12 16:08 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, Peter Zijlstra, Dmitry Ilvokhin, Masami Hiramatsu,
Ingo Molnar, Jens Axboe, io-uring, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Marcelo Ricardo Leitner, Xin Long, Jon Maloy, Aaron Conole,
Eelco Chaudron, Ilya Maximets, netdev, bpf, linux-sctp,
tipc-discussion, dev, Oded Gabbay, Koby Elbaz, dri-devel,
Rafael J. Wysocki, Viresh Kumar, Gautham R. Shenoy, Huang Rui,
Mario Limonciello, Len Brown, Srinivas Pandruvada, linux-pm,
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Christian König,
Sumit Semwal, linaro-mm-sig, Eddie James, Andrew Jeffery,
Joel Stanley, linux-fsi, David Airlie, Simona Vetter,
Alex Deucher, Danilo Krummrich, Matthew Brost, Philipp Stanner,
Harry Wentland, Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires,
linux-input, Wolfram Sang, linux-i2c, Mark Brown,
Michael Hennerich, Nuno Sá, linux-spi, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, Chris Mason, David Sterba,
linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <1becdbce-2c01-468a-bbab-42b5dea9fdf8@efficios.com>
On Thu, Mar 12, 2026 at 11:49 AM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2026-03-12 11:40, Steven Rostedt wrote:
> > On Thu, 12 Mar 2026 11:28:07 -0400
> > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> >
> >>> Note, Vineeth came up with the naming. I would have done "do" but when I
> >>> saw "invoke" I thought it sounded better.
> >>
> >> It works as long as you don't have a tracing subsystem called
> >> "invoke", then you get into identifier clash territory.
> >
> > True. Perhaps we should do the double underscore trick.
> >
> > Instead of: trace_invoke_foo()
> >
> > use: trace_invoke__foo()
> >
> >
> > Which will make it more visible to what the trace event is.
> >
> > Hmm, we probably should have used: trace__foo() for all tracepoints, as
> > there's still functions that are called trace_foo() that are not
> > tracepoints :-p
>
> One certain way to eliminate identifier clash would be to go for a
> prefix to "trace_", e.g.
>
> do_trace_foo()
> call_trace_foo()
This was the initial idea, but it had conflict in the existing source:
call_trace_sched_update_nr_running. do_trace_##name also had
collisions when I checked. So, went with trace_invoke_##name. Did not
check rest of the suggestions here though.
Thanks,
Vineeth
> emit_trace_foo()
> __trace_foo()
> invoke_trace_foo()
> dispatch_trace_foo()
>
> Thanks,
>
> Mathieu
>
>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply
* Re: [PATCH 01/15] tracepoint: Add trace_invoke_##name() API
From: Vineeth Remanan Pillai @ 2026-03-12 16:05 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Steven Rostedt, Dmitry Ilvokhin, Masami Hiramatsu,
Mathieu Desnoyers, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Oded Gabbay,
Koby Elbaz, dri-devel, Rafael J. Wysocki, Viresh Kumar,
Gautham R. Shenoy, Huang Rui, Mario Limonciello, Len Brown,
Srinivas Pandruvada, linux-pm, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian König, Sumit Semwal, linaro-mm-sig,
Eddie James, Andrew Jeffery, Joel Stanley, linux-fsi,
David Airlie, Simona Vetter, Alex Deucher, Danilo Krummrich,
Matthew Brost, Philipp Stanner, Harry Wentland, Leo Li, amd-gfx,
Jiri Kosina, Benjamin Tissoires, linux-input, Wolfram Sang,
linux-i2c, Mark Brown, Michael Hennerich, Nuno Sá, linux-spi,
James E.J. Bottomley, Martin K. Petersen, linux-scsi, Chris Mason,
David Sterba, linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <20260312155326.GB1282955@noisy.programming.kicks-ass.net>
On Thu, Mar 12, 2026 at 11:53 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Mar 12, 2026 at 11:39:06AM -0400, Vineeth Remanan Pillai wrote:
> > On Thu, Mar 12, 2026 at 11:13 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > On Thu, 12 Mar 2026 11:04:56 -0400
> > > "Vineeth Pillai (Google)" <vineeth@bitbyteword.org> wrote:
> > >
> > > > Add trace_invoke_##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_invoke_##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
> > >
> > > I'm guessing Claude helped with the other patches. Did it really help with this one?
> > >
> >
> > Claude wrote and build tested the whole series based on my guidance
> > and prompt :-). I verified the series before sending it out, but
> > claude did the initial work.
>
> That seems like an unreasonable waste of energy. You could've had claude
> write a Coccinelle script for you and saved a ton of tokens.
Yeah true, Steve also mentioned this to me offline. Haven't used
Coccinelle before, but now I know :-)
Thanks,
Vineeth
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Mathieu Desnoyers @ 2026-03-12 15:57 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Steven Rostedt, Vineeth Pillai (Google), Dmitry Ilvokhin,
Masami Hiramatsu, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Oded Gabbay,
Koby Elbaz, dri-devel, Rafael J. Wysocki, Viresh Kumar,
Gautham R. Shenoy, Huang Rui, Mario Limonciello, Len Brown,
Srinivas Pandruvada, linux-pm, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian König, Sumit Semwal, linaro-mm-sig,
Eddie James, Andrew Jeffery, Joel Stanley, linux-fsi,
David Airlie, Simona Vetter, Alex Deucher, Danilo Krummrich,
Matthew Brost, Philipp Stanner, Harry Wentland, Leo Li, amd-gfx,
Jiri Kosina, Benjamin Tissoires, linux-input, Wolfram Sang,
linux-i2c, Mark Brown, Michael Hennerich, Nuno Sá, linux-spi,
James E.J. Bottomley, Martin K. Petersen, linux-scsi, Chris Mason,
David Sterba, linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <20260312155429.GC1282955@noisy.programming.kicks-ass.net>
On 2026-03-12 11:54, Peter Zijlstra wrote:
> On Thu, Mar 12, 2026 at 11:49:23AM -0400, Mathieu Desnoyers wrote:
>> On 2026-03-12 11:40, Steven Rostedt wrote:
>>> On Thu, 12 Mar 2026 11:28:07 -0400
>>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>>
>>>>> Note, Vineeth came up with the naming. I would have done "do" but when I
>>>>> saw "invoke" I thought it sounded better.
>>>>
>>>> It works as long as you don't have a tracing subsystem called
>>>> "invoke", then you get into identifier clash territory.
>>>
>>> True. Perhaps we should do the double underscore trick.
>>>
>>> Instead of: trace_invoke_foo()
>>>
>>> use: trace_invoke__foo()
>>>
>>>
>>> Which will make it more visible to what the trace event is.
>>>
>>> Hmm, we probably should have used: trace__foo() for all tracepoints, as
>>> there's still functions that are called trace_foo() that are not
>>> tracepoints :-p
>>
>> One certain way to eliminate identifier clash would be to go for a
>> prefix to "trace_", e.g.
>
> Oh, I know!, call them __do_trace_##foo().
>
> /me runs like hell
So s/__do_trace_/do_trace_/g and call it a day ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Peter Zijlstra @ 2026-03-12 15:54 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, Vineeth Pillai (Google), Dmitry Ilvokhin,
Masami Hiramatsu, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Oded Gabbay,
Koby Elbaz, dri-devel, Rafael J. Wysocki, Viresh Kumar,
Gautham R. Shenoy, Huang Rui, Mario Limonciello, Len Brown,
Srinivas Pandruvada, linux-pm, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian König, Sumit Semwal, linaro-mm-sig,
Eddie James, Andrew Jeffery, Joel Stanley, linux-fsi,
David Airlie, Simona Vetter, Alex Deucher, Danilo Krummrich,
Matthew Brost, Philipp Stanner, Harry Wentland, Leo Li, amd-gfx,
Jiri Kosina, Benjamin Tissoires, linux-input, Wolfram Sang,
linux-i2c, Mark Brown, Michael Hennerich, Nuno Sá, linux-spi,
James E.J. Bottomley, Martin K. Petersen, linux-scsi, Chris Mason,
David Sterba, linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <1becdbce-2c01-468a-bbab-42b5dea9fdf8@efficios.com>
On Thu, Mar 12, 2026 at 11:49:23AM -0400, Mathieu Desnoyers wrote:
> On 2026-03-12 11:40, Steven Rostedt wrote:
> > On Thu, 12 Mar 2026 11:28:07 -0400
> > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> >
> > > > Note, Vineeth came up with the naming. I would have done "do" but when I
> > > > saw "invoke" I thought it sounded better.
> > >
> > > It works as long as you don't have a tracing subsystem called
> > > "invoke", then you get into identifier clash territory.
> >
> > True. Perhaps we should do the double underscore trick.
> >
> > Instead of: trace_invoke_foo()
> >
> > use: trace_invoke__foo()
> >
> >
> > Which will make it more visible to what the trace event is.
> >
> > Hmm, we probably should have used: trace__foo() for all tracepoints, as
> > there's still functions that are called trace_foo() that are not
> > tracepoints :-p
>
> One certain way to eliminate identifier clash would be to go for a
> prefix to "trace_", e.g.
Oh, I know!, call them __do_trace_##foo().
/me runs like hell
^ permalink raw reply
* Re: [PATCH 01/15] tracepoint: Add trace_invoke_##name() API
From: Peter Zijlstra @ 2026-03-12 15:53 UTC (permalink / raw)
To: Vineeth Remanan Pillai
Cc: Steven Rostedt, Dmitry Ilvokhin, Masami Hiramatsu,
Mathieu Desnoyers, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Oded Gabbay,
Koby Elbaz, dri-devel, Rafael J. Wysocki, Viresh Kumar,
Gautham R. Shenoy, Huang Rui, Mario Limonciello, Len Brown,
Srinivas Pandruvada, linux-pm, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian König, Sumit Semwal, linaro-mm-sig,
Eddie James, Andrew Jeffery, Joel Stanley, linux-fsi,
David Airlie, Simona Vetter, Alex Deucher, Danilo Krummrich,
Matthew Brost, Philipp Stanner, Harry Wentland, Leo Li, amd-gfx,
Jiri Kosina, Benjamin Tissoires, linux-input, Wolfram Sang,
linux-i2c, Mark Brown, Michael Hennerich, Nuno Sá, linux-spi,
James E.J. Bottomley, Martin K. Petersen, linux-scsi, Chris Mason,
David Sterba, linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <CAO7JXPhg-Etspj9YahZrq8cmZ2K6AGWDrMnHO+oD96P_SmOLBw@mail.gmail.com>
On Thu, Mar 12, 2026 at 11:39:06AM -0400, Vineeth Remanan Pillai wrote:
> On Thu, Mar 12, 2026 at 11:13 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Thu, 12 Mar 2026 11:04:56 -0400
> > "Vineeth Pillai (Google)" <vineeth@bitbyteword.org> wrote:
> >
> > > Add trace_invoke_##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_invoke_##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
> >
> > I'm guessing Claude helped with the other patches. Did it really help with this one?
> >
>
> Claude wrote and build tested the whole series based on my guidance
> and prompt :-). I verified the series before sending it out, but
> claude did the initial work.
That seems like an unreasonable waste of energy. You could've had claude
write a Coccinelle script for you and saved a ton of tokens.
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Mathieu Desnoyers @ 2026-03-12 15:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: Vineeth Pillai (Google), Peter Zijlstra, Dmitry Ilvokhin,
Masami Hiramatsu, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Oded Gabbay,
Koby Elbaz, dri-devel, Rafael J. Wysocki, Viresh Kumar,
Gautham R. Shenoy, Huang Rui, Mario Limonciello, Len Brown,
Srinivas Pandruvada, linux-pm, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian König, Sumit Semwal, linaro-mm-sig,
Eddie James, Andrew Jeffery, Joel Stanley, linux-fsi,
David Airlie, Simona Vetter, Alex Deucher, Danilo Krummrich,
Matthew Brost, Philipp Stanner, Harry Wentland, Leo Li, amd-gfx,
Jiri Kosina, Benjamin Tissoires, linux-input, Wolfram Sang,
linux-i2c, Mark Brown, Michael Hennerich, Nuno Sá, linux-spi,
James E.J. Bottomley, Martin K. Petersen, linux-scsi, Chris Mason,
David Sterba, linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <20260312114041.5193c729@gandalf.local.home>
On 2026-03-12 11:40, Steven Rostedt wrote:
> On Thu, 12 Mar 2026 11:28:07 -0400
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
>>> Note, Vineeth came up with the naming. I would have done "do" but when I
>>> saw "invoke" I thought it sounded better.
>>
>> It works as long as you don't have a tracing subsystem called
>> "invoke", then you get into identifier clash territory.
>
> True. Perhaps we should do the double underscore trick.
>
> Instead of: trace_invoke_foo()
>
> use: trace_invoke__foo()
>
>
> Which will make it more visible to what the trace event is.
>
> Hmm, we probably should have used: trace__foo() for all tracepoints, as
> there's still functions that are called trace_foo() that are not
> tracepoints :-p
One certain way to eliminate identifier clash would be to go for a
prefix to "trace_", e.g.
do_trace_foo()
call_trace_foo()
emit_trace_foo()
__trace_foo()
invoke_trace_foo()
dispatch_trace_foo()
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* Re: [PATCH] tracing: Update undefined symbols allow list for simple_ring_buffer
From: Marc Zyngier @ 2026-03-12 15:44 UTC (permalink / raw)
To: Vincent Donnefort
Cc: rostedt, linux-trace-kernel, kvmarm, kernel-team,
Nathan Chancellor
In-Reply-To: <20260312113535.2213350-1-vdonnefort@google.com>
On Thu, 12 Mar 2026 11:35:35 +0000, Vincent Donnefort wrote:
> Undefined symbols are not allowed for simple_ring_buffer.c. But some
> compiler emitted symbols are missing in the allowlist. Update it.
>
>
Applied to next, thanks!
[1/1] tracing: Update undefined symbols allow list for simple_ring_buffer
commit: 5f2f83047126f1cb2986d142d2e76e1fa3cef3f0
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox