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

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Incorrect field check and negated logic in `is_expected_broken_metric()` masks legitimate test failures.
--

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

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

This commit introduces a check to skip metric tests on Arm64 platforms when
the slot count is zero, preventing syntax errors from failing the PMU metric
tests.

> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c

[ ... ]

> +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_name, "#slots") &&
> +	    !tool_pmu__cpu_slots_per_cycle())
> +		return true;

Should this check pm->metric_expr instead of pm->metric_name, and should
the negation be removed?

The string "#slots" appears to be part of the metric expression rather than
the metric name. Additionally, the !strstr() condition evaluates to true
when the string is not found.

Because most metric names do not contain "#slots", this condition seems to
evaluate to true for almost all metrics. On Arm64 platforms where
tool_pmu__cpu_slots_per_cycle() returns 0, could this cause any metric
parsing failure to be falsely identified as an expected broken metric and
silently skipped?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410-perf_fix_pmu_metrics_test-v1-1-18a5d80f71b6@arm.com?part=1