Re: [PATCH v11 13/19] perf lock-contention: Use perf_env e_machine rather than arch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `.feature` callback in perf_tool initialization for pipe mode cross-platform analysis
--

commit 6123af68f287b03d324e8ede7eac36aa4cd98913
Author: Ian Rogers <irogers@google.com>

perf lock-contention: Use perf_env e_machine rather than arch

Use the e_machine rather than arch string matching for powerpc.

> diff --git a/tools/perf/util/lock-contention.c b/tools/perf/util/lock-contention.c
> index 92e7b7b572a22..119a7206f3cd2 100644
> --- a/tools/perf/util/lock-contention.c
> +++ b/tools/perf/util/lock-contention.c
> @@ -104,7 +104,8 @@ bool match_callstack_filter(struct machine *machine, u64 *callstack, int max_sta
>  	struct map *kmap;
>  	struct symbol *sym;
>  	u64 ip;
> -	const char *arch = perf_env__arch(machine->env);
> +	uint16_t e_machine = perf_env__e_machine(machine->env, /*e_flags=*/NULL);

[Severity: High]
This isn't a bug introduced by this patch, but won't this cross-platform 
analysis fail when running in pipe mode?

Looking at tools/perf/builtin-lock.c, the perf_tool initialization in both 
__cmd_contention() and __cmd_report() omits the feature callback:

	perf_tool__init(&eops, /*ordered_events=*/true);
	eops.attr		 = perf_event__process_attr;
	eops.event_update	 = process_event_update;
	eops.sample		 = process_sample_event;
	eops.comm		 = perf_event__process_comm;
	eops.mmap		 = perf_event__process_mmap;
	eops.mmap2		 = perf_event__process_mmap2;
	eops.tracing_data	 = perf_event__process_tracing_data;
	/* missing eops.feature = perf_event__process_feature; */

When reading a perf.data stream from a pipe, dropping the feature callback 
causes feature events (like HEADER_ARCH) to be silently dropped. This 
prevents machine->env from being properly populated. 

Could this cause perf_env__e_machine() to incorrectly fall back to querying 
the host architecture via uname(), breaking cross-platform analysis 
entirely during pipe mode?

> +	bool is_powerpc = e_machine == EM_PPC64 || e_machine == EM_PPC;
>  
>  	if (list_empty(&callstack_filters))
>  		return true;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602062452.2583619-1-irogers@google.com?part=13