public inbox for linux-perf-users@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero
@ 2026-04-10 11:13 Leo Yan
  2026-04-10 11:28 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Leo Yan @ 2026-04-10 11:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	James Clark
  Cc: linux-perf-users, linux-kernel, Leo Yan

Some Arm64 PMUs expose 'caps/slots' as 0 when the slot count is not
implemented, tool_pmu__read_event() currently returns false for this,
so metrics that reference #slots are reported as syntax error.

Since the commit 3a61fd866ef9 ("perf expr: Return -EINVAL for syntax
error in expr__find_ids()"), these syntax errors are populated as
failures and make the PMU metric test fail:

    9.3: Parsing of PMU event table metrics:
    --- start ---
    ...

    Found metric 'backend_bound'
    metric expr 100 * (stall_slot_backend / (#slots * cpu_cycles)) for backend_bound
    parsing metric: 100 * (stall_slot_backend / (#slots * cpu_cycles))
    Failure to read '#slots'
    literal: #slots = nan
    syntax error
    Fail to parse metric or group `backend_bound'

    ...
    ---- end(-1) ----
    9.3: Parsing of PMU event table metrics    : FAILED!

This commit introduces a new function is_expected_broken_metric() to
identify broken metrics, and treats metrics containing "#slots" as
expected broken when #slots == 0 on Arm64 platforms.

Fixes: 3a61fd866ef9 ("perf expr: Return -EINVAL for syntax error in expr__find_ids()")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Changes in v2:
- Checked pm->metric_expr instead of pm->metric_name and removed the
  negation before strstr() suggested by sashiko.
- Link to v1: https://lore.kernel.org/r/20260410-perf_fix_pmu_metrics_test-v1-1-18a5d80f71b6@arm.com
---
 tools/perf/tests/pmu-events.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index a997168621688007c85495e2d9f6f459c2471516..b1609a7e1d8c9427e6bf9500380e8aac6167c7fa 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -15,6 +15,7 @@
 #include "util/expr.h"
 #include "util/hashmap.h"
 #include "util/parse-events.h"
+#include "util/tool_pmu.h"
 #include "metricgroup.h"
 #include "stat.h"
 
@@ -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())
+		return true;
+#endif
+
+	return false;
+}
+
 static int test__parsing_callback(const struct pmu_metric *pm,
 				  const struct pmu_metrics_table *table,
 				  void *data)
@@ -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;

---
base-commit: 4cf1f549bbcdfea9c20df52994bb342677472dcd
change-id: 20260408-perf_fix_pmu_metrics_test-73c4dedc8585

Best regards,
-- 
Leo Yan <leo.yan@arm.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero
  2026-04-10 11:13 [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero Leo Yan
@ 2026-04-10 11:28 ` sashiko-bot
  2026-04-10 14:09   ` Leo Yan
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-04-10 11:28 UTC (permalink / raw)
  To: Leo Yan; +Cc: linux-perf-users

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero
  2026-04-10 11:28 ` sashiko-bot
@ 2026-04-10 14:09   ` Leo Yan
  0 siblings, 0 replies; 3+ messages in thread
From: Leo Yan @ 2026-04-10 14:09 UTC (permalink / raw)
  To: sashiko; +Cc: linux-perf-users

On Fri, Apr 10, 2026 at 11:28:23AM +0000, sashiko-bot@kernel.org wrote:

[...]

> > +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?

I agreed that the test is designed for hardware-independent, however,
the low level's parser has already invoked
tool_pmu__cpu_slots_per_cycle() for event parsing, even with the fake
PMU.

[...]

> > @@ -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?

The test will run on PMUs that support #slots, so any malformed metric
expressions can be detected.  Simply say, if a metric is applicable to
a platform, it will be verified on that platform.

I don't think I need any update on this patch.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-10 14:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10 11:13 [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero Leo Yan
2026-04-10 11:28 ` sashiko-bot
2026-04-10 14:09   ` Leo Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox