[PATCH] perf metricgroup: Correctly return syntax error

On arm64 platforms, some CPU variants return zero for the number of slots
when invoking tool_pmu__cpu_slots_per_cycle().  This leads to metric
expression parsing failures, e.g.:

  metric expr 100 * (STALL_SLOT_BACKEND / (CPU_CYCLES * #slots) - BR_MIS_PRED * 3 / CPU_CYCLES) for backend_bound
  parsing metric: 100 * (STALL_SLOT_BACKEND / (CPU_CYCLES * #slots) - BR_MIS_PRED * 3 / CPU_CYCLES)
  Failure to read '#slots' literal: #slots = nan
  syntax error

expr__find_ids() returns a positive value for syntax errors, but this is
not respected.  Fix this by checking for any non-zero return value from
expr__find_ids().

Do not add a Fixes tag, as this is a rare case that can only be reproduced
by deliberately setting the PERF_CPUS env variable.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/metricgroup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 7e39d469111b2995f5a2636529d6985eac595f76..1119bb1c99dbd9d32601b8aeebed7d5638872366 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c

@@ -909,10 +909,12 @@ static int __add_metric(struct list_head *metric_list,
 		expr = metric_no_threshold ? pm->metric_name : pm->metric_threshold;
 		visited_node.name = "__threshold__";
 	}
-	if (expr__find_ids(expr, NULL, root_metric->pctx) < 0) {
+
+	/* The expression parser returns a positive value on syntax error */
+	if (expr__find_ids(expr, NULL, root_metric->pctx))
 		/* Broken metric. */
 		ret = -EINVAL;
-	}
+
 	if (!ret) {
 		/* Resolve referenced metrics. */
 		struct perf_pmu *pmu;

---
base-commit: 54fcc7f6ec3944ae7c1b0246a999744e33839cdb
change-id: 20260327-perf_fix_metrics_group-4be55b236f96

Best regards,
-- 
Leo Yan [off-list ref]

On 27/03/2026 5:25 pm, Leo Yan wrote:

On arm64 platforms, some CPU variants return zero for the number of slots
when invoking tool_pmu__cpu_slots_per_cycle().  This leads to metric
expression parsing failures, e.g.:

   metric expr 100 * (STALL_SLOT_BACKEND / (CPU_CYCLES * #slots) - BR_MIS_PRED * 3 / CPU_CYCLES) for backend_bound
   parsing metric: 100 * (STALL_SLOT_BACKEND / (CPU_CYCLES * #slots) - BR_MIS_PRED * 3 / CPU_CYCLES)
   Failure to read '#slots' literal: #slots = nan
   syntax error

expr__find_ids() returns a positive value for syntax errors, but this is
not respected.  Fix this by checking for any non-zero return value from
expr__find_ids().

What command does this impact exactly, and what does the new output look like?

Because "failure to read '#slots'" seems like ok output to me. I also saw this related commit where it seems like showing NAN is intentional:

https://lore.kernel.org/linux-perf-users/20260203230733.1474840-1-ctshao@google.com/

Do not add a Fixes tag, as this is a rare case that can only be reproduced
by deliberately setting the PERF_CPUS env variable.

Slots is always 0 in guests so I think this might not be that rare, although that should be fixed in a different way.
Maybe a fixes tag is worthwhile until we fix #slots in guests somehow.