Re: [PATCH v1 01/12] perf parse-events: Restrict core PMU bypass to --cputype option

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch fails to correctly distinguish between `--cputype` and `--pmu-filter` when parsing metrics, causing uncore PMU events to incorrectly bypass the PMU filter.
--

commit 4dcfee88ee0f6425d471a5e115dd295cc67baf7d
Author: Ian Rogers <irogers@google.com>

perf parse-events: Restrict core PMU bypass to --cputype option

This commit introduces a cputype_filter flag to distinguish filtering
initiated by --cputype from --pmu-filter. This restricts the uncore
PMU bypass during event parsing to only apply when using --cputype.

> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
[ ... ]
> @@ -1317,7 +1317,9 @@ static int parse_ids(bool metric_no_merge, bool fake_pmu,
>  	pr_debug("Parsing metric events '%s'\n", events.buf);
>  	parse_events_error__init(&parse_error);
>  	ret = __parse_events(parsed_evlist, events.buf, filter_pmu,
> -			     &parse_error, fake_pmu, /*warn_if_reordered=*/false,
> +			     /*cputype_filter=*/filter_pmu != NULL,

[Severity: Medium]
Does hardcoding this argument to filter_pmu != NULL cause metric parsing to
lose the distinction between --cputype and --pmu-filter?

If metric parsing is initiated and a --pmu-filter was provided, this will
cause cputype_filter to be evaluated as true in __parse_events().

This will then trigger the new bypass logic in parse_events__filter_pmu(),
preventing uncore PMUs from being filtered out. As a result, users specifying
--pmu-filter with uncore metrics might silently aggregate events from all
identical uncore PMUs instead of the single requested one.

> +			     &parse_error, fake_pmu,
> +			     /*warn_if_reordered=*/false,
>  			     /*fake_tp=*/false);
>  	if (ret) {
>  		parse_events_error__print(&parse_error, events.buf);
[ ... ]
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
[ ... ]
> @@ -429,6 +429,9 @@ bool parse_events__filter_pmu(const struct parse_events_state *parse_state,
>  	if (parse_state->pmu_filter == NULL)
>  		return false;
>  
> +	if (parse_state->cputype_filter && !pmu->is_core)
> +		return false;
> +

[Severity: Medium]
Is it possible for this block to be incorrectly reached during metric
evaluation if cputype_filter was erroneously set to true by parse_ids()?

>  	return perf_pmu__wildcard_match(pmu, parse_state->pmu_filter) == 0;
>  }

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