Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
@ 2026-06-12 22:08 Aaron Tomlin
  2026-06-12 22:16 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Aaron Tomlin @ 2026-06-12 22:08 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, howardchu95, atomlin, neelx, sean, linux-perf-users,
	linux-kernel

When a dynamic probe is created without explicitly requested arguments
via perf probe --add, the Ftrace subsystem automatically appends
"__probe_ip" to the tracepoint format to record the instruction pointer.

Currently, perf trace prints this implicit field by default.
Furthermore, because the formatting logic defaults to a standard signed
integer representation, the kernel space memory address is erroneously
displayed as a meaningless negative integer.

    ❯ sudo ./perf trace --event probe:proc_sys_open --max-events 1
         0.000 ps/1316543 probe:proc_sys_open(__probe_ip: -1406056956)

This patch addresses the user experience by combining two refinements:
    1. "__probe_ip" is now hidden from the standard output, as its
       presence adds no contextual value for a bare probe.

    2. If the user explicitly requests verbose output (--verbose),
       "__probe_ip" is intercepted and properly formatted as a hexadecimal
       kernel address, restoring its utility for debugging inline
       function hits.

    ❯ sudo ./perf trace --event probe:proc_sys_open --max-events 1
         0.000 ps/1314074 probe:proc_sys_open()

    ❯ sudo ./perf trace --verbose --event probe:proc_sys_open --max-events 1
    Using CPUID GenuineIntel-6-8E-C
    mmap size 528384B
         0.000 ps/1314366 probe:proc_sys_open(__probe_ip: 0xffffffffac314604)

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 48615ddccd93..ca0b2680d531 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3234,6 +3234,23 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel,
 		if (val == 0 && !trace->show_zeros && !arg->show_zero && arg->strtoul != STUL_BTF_TYPE)
 			continue;
 
+		/*
+		 * __probe_ip is implicitly added to bare dynamic probes.
+		 * Suppress it by default to avoid cluttering the output.
+		 * If verbose mode is enabled, ensure it is formatted as a
+		 * hexadecimal memory address rather than a signed integer.
+		 */
+		if (!strcmp(field->name, "__probe_ip")) {
+			if (!verbose)
+				continue;
+
+			printed += scnprintf(bf + printed, size - printed,
+					     "%s%s: %#016llx", printed ?
+					     ", " : "", field->name,
+					     (unsigned long long)val);
+			continue;
+		}
+
 		printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
 
 		if (trace->show_arg_names)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
  2026-06-12 22:08 [PATCH] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes Aaron Tomlin
@ 2026-06-12 22:16 ` sashiko-bot
  2026-06-13  0:27   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-06-12 22:16 UTC (permalink / raw)
  To: Aaron Tomlin; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch hardcodes the `__probe_ip` argument name in the output format, ignoring the `trace->show_arg_names` configuration.
--

commit 52fd2385bfe746191bd8b95bc76fff9e349f4450
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes

This commit hides the implicit __probe_ip field in bare dynamic probes
by default, and formats it as a hexadecimal kernel address when verbose mode
is enabled.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 48615ddccd93a..ca0b2680d531b 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c

[ ... ]

> @@ -3234,6 +3234,23 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel,
>  		if (val == 0 && !trace->show_zeros && !arg->show_zero && arg->strtoul != STUL_BTF_TYPE)
>  			continue;
>  
> +		/*
> +		 * __probe_ip is implicitly added to bare dynamic probes.
> +		 * Suppress it by default to avoid cluttering the output.
> +		 * If verbose mode is enabled, ensure it is formatted as a
> +		 * hexadecimal memory address rather than a signed integer.
> +		 */
> +		if (!strcmp(field->name, "__probe_ip")) {
> +			if (!verbose)
> +				continue;
> +
> +			printed += scnprintf(bf + printed, size - printed,
> +					     "%s%s: %#016llx", printed ?
> +					     ", " : "", field->name,
> +					     (unsigned long long)val);

[Severity: Medium]
Does this hardcoded format string bypass the trace->show_arg_names
configuration?

In trace__fprintf_tp_fields(), other arguments dynamically check if their
names should be printed:

        if (trace->show_arg_names)
                printed += scnprintf(bf + printed, size - printed, "%s: ", field->name);

By explicitly formatting "%s%s: %#016llx" using field->name, it looks like
__probe_ip will always print its argument name here, even if perf trace
is executed with --no-arg-names alongside --verbose.

> +			continue;
> +		}
> +
>  		printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
>  
>  		if (trace->show_arg_names)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612220843.1320348-1-atomlin@atomlin.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
  2026-06-12 22:16 ` sashiko-bot
@ 2026-06-13  0:27   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-13  0:27 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Aaron Tomlin, linux-perf-users

On Fri, Jun 12, 2026 at 10:16:01PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch hardcodes the `__probe_ip` argument name in the output format, ignoring the `trace->show_arg_names` configuration.
> --
> 
> commit 52fd2385bfe746191bd8b95bc76fff9e349f4450
> Author: Aaron Tomlin <atomlin@atomlin.com>
> 
> perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
> 
> This commit hides the implicit __probe_ip field in bare dynamic probes
> by default, and formats it as a hexadecimal kernel address when verbose mode
> is enabled.
> 
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index 48615ddccd93a..ca0b2680d531b 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> 
> [ ... ]
> 
> > @@ -3234,6 +3234,23 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel,
> >  		if (val == 0 && !trace->show_zeros && !arg->show_zero && arg->strtoul != STUL_BTF_TYPE)
> >  			continue;
> >  
> > +		/*
> > +		 * __probe_ip is implicitly added to bare dynamic probes.
> > +		 * Suppress it by default to avoid cluttering the output.
> > +		 * If verbose mode is enabled, ensure it is formatted as a
> > +		 * hexadecimal memory address rather than a signed integer.
> > +		 */
> > +		if (!strcmp(field->name, "__probe_ip")) {
> > +			if (!verbose)
> > +				continue;
> > +
> > +			printed += scnprintf(bf + printed, size - printed,
> > +					     "%s%s: %#016llx", printed ?
> > +					     ", " : "", field->name,
> > +					     (unsigned long long)val);
> 
> [Severity: Medium]
> Does this hardcoded format string bypass the trace->show_arg_names
> configuration?
> 
> In trace__fprintf_tp_fields(), other arguments dynamically check if their
> names should be printed:
> 
>         if (trace->show_arg_names)
>                 printed += scnprintf(bf + printed, size - printed, "%s: ", field->name);
> 
> By explicitly formatting "%s%s: %#016llx" using field->name, it looks like
> __probe_ip will always print its argument name here, even if perf trace
> is executed with --no-arg-names alongside --verbose.

I think this is a valid concern, please make it follow the
trace->show_arg_names for consistency.

This is a nice improvement, thanks for working on it,

- Arnaldo
 
> > +			continue;
> > +		}
> > +
> >  		printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
> >  
> >  		if (trace->show_arg_names)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260612220843.1320348-1-atomlin@atomlin.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-13  0:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 22:08 [PATCH] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes Aaron Tomlin
2026-06-12 22:16 ` sashiko-bot
2026-06-13  0:27   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox