Re: [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Bypass of `fake_pmu` testing abstraction and masking of genuine syntax errors in metric validation tests.
--

commit a6c8a008b016717763a4f877bfefc1ddfed146b3
Author: Leo Yan <leo.yan@arm.com>

perf pmu: Skip test on Arm64 when #slots is zero

This commit modifies the PMU metric testing infrastructure to skip metrics
containing "#slots" on Arm64 systems where the CPU does not implement slot
counts. It introduces a function to identify these scenarios and mask the
resulting parsing errors to prevent the test suite from failing.

> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -817,6 +818,26 @@ struct metric {
>  	struct metric_ref metric_ref;
>  };
>  
> +static bool is_expected_broken_metric(const struct pmu_metric *pm)
> +{
> +	if (!strcmp(pm->metric_name, "M1") || !strcmp(pm->metric_name, "M2") ||
> +	    !strcmp(pm->metric_name, "M3"))
> +		return true;
> +
> +#if defined(__aarch64__)
> +	/*
> +	 * Arm64 platforms may return "#slots == 0", which is treated as a
> +	 * syntax error by the parser. Don't test these metrics when running
> +	 * on such platforms.
> +	 */
> +	if (strstr(pm->metric_expr, "#slots") &&
> +	    !tool_pmu__cpu_slots_per_cycle())

Does reading host capabilities via tool_pmu__cpu_slots_per_cycle() bypass
the fake PMU abstraction used by these tests?

The fake PMU tests are intended to validate the metric parser across all
architectures without requiring the actual referenced PMUs or hardware
capabilities to exist on the host machine.

Because tool_pmu__cpu_slots_per_cycle() checks the host's
/sys/bus/event_source/devices/ caps/slots, does this break the test's
hardware-independent validation?

> +		return true;
> +#endif
> +
> +	return false;
> +}
> +

[ ... ]

> @@ -852,8 +873,7 @@ static int test__parsing_callback(const struct pmu_metric *pm,
>  
>  	err = metricgroup__parse_groups_test(evlist, table, pm->metric_name);
>  	if (err) {
> -		if (!strcmp(pm->metric_name, "M1") || !strcmp(pm->metric_name, "M2") ||
> -		    !strcmp(pm->metric_name, "M3")) {
> +		if (is_expected_broken_metric(pm)) {
>  			(*failures)--;
>  			pr_debug("Expected broken metric %s skipping\n", pm->metric_name);
>  			err = 0;

Does unconditionally clearing the error here create a testing blind spot
for genuine syntax errors?

If a metric expression contains "#slots" on an Arm64 host without slot
support, the failure is ignored entirely. Could this silently hide other
issues, like typos in other events or malformed operators within the same
metric expression?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410-perf_fix_pmu_metrics_test-v2-1-61826ab3ca8b@arm.com?part=1