The Linux Kernel Mailing List
 help / color / mirror / Atom feed
  • * Re: [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero
           [not found] <20260410-perf_fix_pmu_metrics_test-v2-1-61826ab3ca8b@arm.com>
           [not found] ` <98ddaa65-747a-4b3e-9f72-05b90fc4eadb@linaro.org>
    @ 2026-05-13 13:10 ` Ian Rogers
      2026-05-13 14:37   ` Leo Yan
      1 sibling, 1 reply; 3+ messages in thread
    From: Ian Rogers @ 2026-05-13 13:10 UTC (permalink / raw)
      To: Leo Yan
      Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
    	Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark,
    	linux-perf-users, linux-kernel
    
    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>
    >
    
    ^ permalink raw reply	[flat|nested] 3+ messages in thread

  • end of thread, other threads:[~2026-05-13 14:37 UTC | newest]
    
    Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
    -- links below jump to the message on this page --
         [not found] <20260410-perf_fix_pmu_metrics_test-v2-1-61826ab3ca8b@arm.com>
         [not found] ` <98ddaa65-747a-4b3e-9f72-05b90fc4eadb@linaro.org>
    2026-05-13 12:52   ` [PATCH v2] perf pmu: Skip test on Arm64 when #slots is zero Leo Yan
    2026-05-13 13:10 ` Ian Rogers
    2026-05-13 14:37   ` Leo Yan
    

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