Re: [PATCH v1 13/41] perf pmus: Add perf_pmus__read_from_env()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf header: missing HEADER_E_MACHINE pmus reset in perf_event__process_feature()
--

commit 523e239f6cc1817fa333720299701f5ca84f8562
Author: Ian Rogers <irogers@google.com>

perf pmus: Add perf_pmus__read_from_env()

This commit adds perf_pmus__read_from_env() and perf_pmu__add_cap() to
reconstruct PMU lists and capabilities from perf.data header metadata and
lazily populate env->pmus in perf_env__pmus().

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index a442f2a904a9d..27fc21fa3a6a7 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -5765,6 +5765,28 @@ int perf_event__process_feature(const struct perf_tool *tool __maybe_unused,
>  		goto out;
>  	}
>  
> +	switch (feat) {
> +	case HEADER_ARCH:
> +	case HEADER_CPUID:
> +	case HEADER_NRCPUS:
> +	case HEADER_PMU_MAPPINGS:
> +	case HEADER_CPU_PMU_CAPS:
> +	case HEADER_HYBRID_TOPOLOGY:
> +	case HEADER_PMU_CAPS:
> +		mutex_lock(&header->env.pmus.lock);
> +		if (session->evlist) {
> +			struct evsel *evsel;
> +
> +			evlist__for_each_entry(session->evlist, evsel)
> +				evsel->pmu = NULL;
> +		}
> +		perf_pmus__exit(&header->env.pmus);
> +		mutex_unlock(&header->env.pmus.lock);
> +		break;

[Severity: Medium]
Should HEADER_E_MACHINE also be included in this switch statement in
perf_event__process_feature()?

In tools/perf/util/pmus.c, perf_pmus__read_from_env() relies on
perf_env__e_machine(env, NULL) directly to look up core_table:

	if (env->cpuid)
		core_table = find_core_events_table(perf_env__e_machine(env, /*e_flags=*/NULL),
						    env->cpuid);

and indirectly via perf_env__arch(env) in env_is_pmu_core().

Since HEADER_E_MACHINE comes after all the PMU feature records in the
feature enum and sets env->e_machine in process_e_machine() (overriding
the env->arch or host uts_machine() fallback in perf_env__e_machine()),
if perf_env__pmus(&header->env) is evaluated in pipe mode before
HEADER_E_MACHINE arrives, won't header->env.pmus and cached evsel->pmu
pointers remain stale once HEADER_E_MACHINE is processed?

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