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

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

Posted by Leo Yan 2 months, 1 week ago

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>

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

Posted by Ian Rogers 1 month, 1 week ago

On Fri, Apr 10, 2026 at 4:13 AM Leo Yan <leo.yan@arm.com> wrote:
>
> 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>

We get this failure for these metrics on x86 when building perf with
JEVENTS_ARCH=all. Rather than expecting the parse failure perhaps we
should just always return true in tool_pmu__read_event:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/tool_pmu.c#n421
I guess the problem there is that when these metrics are broken (no
slots value) you can't distinguish this case from other valid cases.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

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

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

Posted by Leo Yan 1 month, 1 week ago

On Wed, May 13, 2026 at 06:10:00AM -0700, Ian Rogers wrote:

[...]

> We get this failure for these metrics on x86 when building perf with
> JEVENTS_ARCH=all. Rather than expecting the parse failure perhaps we
> should just always return true in tool_pmu__read_event:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/tool_pmu.c#n421

I considered returning true when slots == 0 so can mute parser error
and allow the test to pass. However, if so platforms which do not
support #slots would be able to use those metrics and generate
meaningless statistics.

I would keep the parser errors so this is a reminding when users wrongly
use unsupported metrics.

> I guess the problem there is that when these metrics are broken (no
> slots value) you can't distinguish this case from other valid cases.

IMO, this is a test design issue: tests should validate metrics while
remaining hardware-agnostic. Hardware-specific cases should either run
only on supported platforms, or the tests should be refined to run
transparently across different hardware.

> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks for review!

Leo

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

Posted by Arnaldo Carvalho de Melo 1 month ago

On Wed, May 13, 2026 at 03:37:05PM +0100, Leo Yan wrote:
> On Wed, May 13, 2026 at 06:10:00AM -0700, Ian Rogers wrote:
> 
> [...]
> 
> > We get this failure for these metrics on x86 when building perf with
> > JEVENTS_ARCH=all. Rather than expecting the parse failure perhaps we
> > should just always return true in tool_pmu__read_event:
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/tool_pmu.c#n421
> 
> I considered returning true when slots == 0 so can mute parser error
> and allow the test to pass. However, if so platforms which do not
> support #slots would be able to use those metrics and generate
> meaningless statistics.
> 
> I would keep the parser errors so this is a reminding when users wrongly
> use unsupported metrics.
> 
> > I guess the problem there is that when these metrics are broken (no
> > slots value) you can't distinguish this case from other valid cases.
> 
> IMO, this is a test design issue: tests should validate metrics while
> remaining hardware-agnostic. Hardware-specific cases should either run
> only on supported platforms, or the tests should be refined to run
> transparently across different hardware.
> 
> > Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Thanks for review!

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo

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

Posted by James Clark 2 months ago


On 10/04/2026 12:13, Leo Yan wrote:
> 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>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
> 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,

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

Posted by Leo Yan 1 month, 1 week ago

On Tue, Apr 21, 2026 at 03:20:08PM +0100, James Clark wrote:

[...]

> > 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>
> 
> Reviewed-by: James Clark <james.clark@linaro.org>

Gentle ping. Thanks!