Re: [PATCH] perf trace: Introduce --show-cpu option to display cpu id

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] The `--show-cpu` option fails to request the PERF_SAMPLE_CPU attribute and incorrectly uses `%03d` to print a u32, resulting in `[-01]` being printed.
- [Medium] The `--show-cpu` feature has no effect on the core events of `perf trace` (system calls and page faults).
--

commit 85a27484af518bd604f94598d0e741862be09625
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Introduce --show-cpu option to display cpu id

This commit introduces a new "--show-cpu" command-line option to perf trace.
When provided, it extracts the CPU from the perf sample and prints it
alongside the timestamp.

> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c

[ ... ]

> @@ -3277,6 +3278,9 @@ static int trace__event_handler(struct trace *trace, struct evsel *evsel,
>  	trace__printf_interrupted_entry(trace);
>  	trace__fprintf_tstamp(trace, sample->time, trace->output);
>  
> +	if (trace->show_cpu)
> +		fprintf(trace->output, "[%03d] ", sample->cpu);
> +

When tracing specific processes using non-tracepoint events,
target__has_cpu() is false and CPU samples might not be recorded. If the CPU
data is missing, evsel__parse_sample() assigns a default value of -1
(0xFFFFFFFF) to the u32 sample->cpu field.

Since %03d is a signed format specifier, will this cause [-01] to be printed
instead of a valid CPU id?

Furthermore, this modifies trace__event_handler(), which processes secondary
events. System calls and page faults, which make up the primary workload of
perf trace, are processed by dedicated handlers like trace__sys_enter(),
trace__sys_exit(), and trace__pgfault().

Because those core handlers format their output prefixes using
trace__fprintf_entry_head(), will --show-cpu silently fail to display CPU
information for the default trace output?

[ ... ]

> @@ -5422,6 +5426,7 @@ int cmd_trace(int argc, const char **argv)
>  	OPT_CALLBACK('m', "mmap-pages", &trace.opts.mmap_pages, "pages",
>  		     "number of mmap data pages", evlist__parse_mmap_pages),
>  	OPT_STRING('u', "uid", &trace.uid_str, "user", "user to profile"),
> +	OPT_BOOLEAN(0, "show-cpu", &trace.show_cpu, "show cpu id"),

Does trace.opts.sample_cpu need to be set to true when --show-cpu is
provided, to ensure the kernel is instructed to actually record the CPU
samples?

>  	OPT_CALLBACK(0, "duration", &trace, "float",
>  		     "show only events with duration > N.M ms",
>  		     trace__set_duration),

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