mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
@ 2026-06-12 22:56 Aaron Tomlin
  2026-06-15  9:32 ` James Clark
  0 siblings, 1 reply; 3+ messages in thread
From: Aaron Tomlin @ 2026-06-12 22:56 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>
---
Changes since v1:

 - Fixed a bug where the hardcoded format string for "__probe_ip"
   bypassed the trace->show_arg_names configuration. The formatting
   logic is now properly decoupled to respect trace.show_arg_names when
   verbose mode is enabled.

 - Linked to v1: https://lore.kernel.org/lkml/20260612220843.1320348-1-atomlin@atomlin.com/
---
 tools/perf/builtin-trace.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 48615ddccd93..ac15b5588a27 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3234,6 +3234,27 @@ 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", printed ? ", " : "");
+			if (trace->show_arg_names)
+				printed += scnprintf(bf + printed, size - printed,
+						     "%s: ", field->name);
+
+			printed += scnprintf(bf + printed, size - printed, "%#016llx",
+					     (unsigned long long)val);
+			continue;
+		}
+
 		printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
 
 		if (trace->show_arg_names)
-- 
2.51.0


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

* Re: [PATCH v2] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
  2026-06-12 22:56 [PATCH v2] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes Aaron Tomlin
@ 2026-06-15  9:32 ` James Clark
  2026-06-15 12:43   ` Aaron Tomlin
  0 siblings, 1 reply; 3+ messages in thread
From: James Clark @ 2026-06-15  9:32 UTC (permalink / raw)
  To: Aaron Tomlin
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	howardchu95, neelx, sean, linux-perf-users, linux-kernel, peterz,
	mingo, acme, namhyung



On 12/06/2026 11:56 pm, Aaron Tomlin wrote:
> 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>
> ---
> Changes since v1:
> 
>   - Fixed a bug where the hardcoded format string for "__probe_ip"
>     bypassed the trace->show_arg_names configuration. The formatting
>     logic is now properly decoupled to respect trace.show_arg_names when
>     verbose mode is enabled.
> 
>   - Linked to v1: https://lore.kernel.org/lkml/20260612220843.1320348-1-atomlin@atomlin.com/
> ---
>   tools/perf/builtin-trace.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 48615ddccd93..ac15b5588a27 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -3234,6 +3234,27 @@ 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")) {

I checked to see if it was possible to include the #define for this 
constant (FIELD_STRING_IP) in the tools folder, but there wasn't 
anything existing in include/ that fits nicely. Either way:

Reviewed-by: James Clark <james.clark@linaro.org>

> +			if (!verbose)
> +				continue;
> +
> +			printed += scnprintf(bf + printed, size - printed,
> +					     "%s", printed ? ", " : "");
> +			if (trace->show_arg_names)
> +				printed += scnprintf(bf + printed, size - printed,
> +						     "%s: ", field->name);
> +
> +			printed += scnprintf(bf + printed, size - printed, "%#016llx",
> +					     (unsigned long long)val);
> +			continue;
> +		}
> +
>   		printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
>   
>   		if (trace->show_arg_names)


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

* Re: [PATCH v2] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes
  2026-06-15  9:32 ` James Clark
@ 2026-06-15 12:43   ` Aaron Tomlin
  0 siblings, 0 replies; 3+ messages in thread
From: Aaron Tomlin @ 2026-06-15 12:43 UTC (permalink / raw)
  To: James Clark
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	howardchu95, neelx, sean, linux-perf-users, linux-kernel, peterz,
	mingo, acme, namhyung

[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]

On Mon, Jun 15, 2026 at 10:32:59AM +0100, James Clark wrote:
> > 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>
> > ---
> > Changes since v1:
> > 
> >   - Fixed a bug where the hardcoded format string for "__probe_ip"
> >     bypassed the trace->show_arg_names configuration. The formatting
> >     logic is now properly decoupled to respect trace.show_arg_names when
> >     verbose mode is enabled.
> > 
> >   - Linked to v1: https://lore.kernel.org/lkml/20260612220843.1320348-1-atomlin@atomlin.com/
> > ---
> >   tools/perf/builtin-trace.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index 48615ddccd93..ac15b5588a27 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -3234,6 +3234,27 @@ 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")) {
> 
> I checked to see if it was possible to include the #define for this constant
> (FIELD_STRING_IP) in the tools folder, but there wasn't anything existing in
> include/ that fits nicely. Either way:
> 
> Reviewed-by: James Clark <james.clark@linaro.org>

Hi James,

Yes, unfortunately.

Thank you for the review.

Kind regards,
-- 
Aaron Tomlin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2026-06-15 12:43 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:56 [PATCH v2] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes Aaron Tomlin
2026-06-15  9:32 ` James Clark
2026-06-15 12:43   ` Aaron Tomlin

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

all inboxes | Powered by JetHome®