From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
James Clark <james.clark@linaro.org>, Leo Yan <leo.yan@arm.com>,
Chun-Tse Shao <ctshao@google.com>,
Qinxin Xia <xiaqinxin@huawei.com>, Yu Peng <pengyu@kylinos.cn>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] perf test: Fix PMU metric parsing tests for unknown literals
Date: Fri, 25 Sep 2026 12:53:25 +0200 [thread overview]
Message-ID: <arZSpXYtyUW_UhDr@x2> (raw)
In-Reply-To: <20260923224106.3268925-1-irogers@google.com>
On Wed, Sep 23, 2026 at 03:41:06PM -0700, Ian Rogers wrote:
> On ARM the #slots literal may or may not be present, it won't be
> present on x86. As it is missing it causes the ampereone and
> ampereonex PMU metric tests to fail when run say on x86. Extend the
> fake_pmu logic so that in these test cases we fake the unknown
> literals like #slots as 1.0. When building with JEVENTS_ARCH=all the
> ampere metric tests no longer fail.
> Signed-off-by: Ian Rogers <irogers@google.com>
Thanks, applied to perf-tools-next, for v7.4.
- Arnaldo
> ---
> tools/perf/util/expr.c | 5 ++++-
> tools/perf/util/metricgroup.c | 37 ++++++++++++++++++++++++-----------
> 2 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> index 8aef3c7418f7..f1164c4666b1 100644
> --- a/tools/perf/util/expr.c
> +++ b/tools/perf/util/expr.c
> @@ -431,7 +431,10 @@ double expr__get_literal(const char *literal, const struct expr_scanner_ctx *ctx
> }
>
> pr_debug2("literal: %s = %f\n", literal, result);
> - return result;
> + if (!ctx->is_test)
> + return result;
> + else
> + return isnan(result) ? 1.0 : result;
> }
>
> /* Does the event 'id' parse? Determine via ctx->ids if possible. */
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 5a60cb95e31c..779fe56fe8d0 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -224,7 +224,8 @@ static struct metric *metric__new(const struct pmu_metric *pm,
> bool metric_no_threshold,
> int runtime,
> const char *user_requested_cpu_list,
> - bool system_wide)
> + bool system_wide,
> + bool fake_pmu)
> {
> struct metric *m;
>
> @@ -256,6 +257,7 @@ static struct metric *metric__new(const struct pmu_metric *pm,
> }
> m->pctx->sctx.runtime = runtime;
> m->pctx->sctx.system_wide = system_wide;
> + m->pctx->sctx.is_test = fake_pmu;
> m->group_events = !metric_no_group && metric__group_events(pm, metric_no_threshold);
> m->default_show_events = pm->default_show_events;
> m->metric_refs = NULL;
> @@ -699,6 +701,7 @@ static int add_metric(struct list_head *metric_list,
> bool metric_no_threshold,
> const char *user_requested_cpu_list,
> bool system_wide,
> + bool fake_pmu,
> struct metric *root_metric,
> const struct visited_metric *visited,
> const struct pmu_metrics_table *table);
> @@ -724,6 +727,7 @@ static int metricgroup__find_metric_callback(const struct pmu_metric *pm,
> * user may override.
> * @user_requested_cpu_list: Command line specified CPUs to record on.
> * @system_wide: Are events for all processes recorded.
> + * @fake_pmu: Are PMUs and events being faked for testing?
> * @root_metric: Metrics may reference other metrics to form a tree. In this
> * case the root_metric holds all the IDs and a list of referenced
> * metrics. When adding a root this argument is NULL.
> @@ -739,6 +743,7 @@ static int resolve_metric(struct list_head *metric_list,
> bool metric_no_threshold,
> const char *user_requested_cpu_list,
> bool system_wide,
> + bool fake_pmu,
> struct metric *root_metric,
> const struct visited_metric *visited,
> const struct pmu_metrics_table *table)
> @@ -788,7 +793,7 @@ static int resolve_metric(struct list_head *metric_list,
> for (i = 0; i < pending_cnt; i++) {
> ret = add_metric(metric_list, &pending[i].pm, modifier, metric_no_group,
> metric_no_threshold, user_requested_cpu_list, system_wide,
> - root_metric, visited, table);
> + fake_pmu, root_metric, visited, table);
> if (ret)
> break;
> }
> @@ -809,6 +814,7 @@ static int resolve_metric(struct list_head *metric_list,
> * @runtime: A special argument for the parser only known at runtime.
> * @user_requested_cpu_list: Command line specified CPUs to record on.
> * @system_wide: Are events for all processes recorded.
> + * @fake_pmu: Are PMUs and events being faked for testing?
> * @root_metric: Metrics may reference other metrics to form a tree. In this
> * case the root_metric holds all the IDs and a list of referenced
> * metrics. When adding a root this argument is NULL.
> @@ -825,6 +831,7 @@ static int __add_metric(struct list_head *metric_list,
> int runtime,
> const char *user_requested_cpu_list,
> bool system_wide,
> + bool fake_pmu,
> struct metric *root_metric,
> const struct visited_metric *visited,
> const struct pmu_metrics_table *table)
> @@ -851,7 +858,7 @@ static int __add_metric(struct list_head *metric_list,
> * metrics that are added recursively.
> */
> root_metric = metric__new(pm, modifier, metric_no_group, metric_no_threshold,
> - runtime, user_requested_cpu_list, system_wide);
> + runtime, user_requested_cpu_list, system_wide, fake_pmu);
> if (!root_metric)
> return -ENOMEM;
>
> @@ -924,7 +931,7 @@ static int __add_metric(struct list_head *metric_list,
>
> ret = resolve_metric(metric_list, pmu, modifier, metric_no_group,
> metric_no_threshold, user_requested_cpu_list,
> - system_wide, root_metric, &visited_node,
> + system_wide, fake_pmu, root_metric, &visited_node,
> table);
> }
> if (ret) {
> @@ -944,6 +951,7 @@ static int add_metric(struct list_head *metric_list,
> bool metric_no_threshold,
> const char *user_requested_cpu_list,
> bool system_wide,
> + bool fake_pmu,
> struct metric *root_metric,
> const struct visited_metric *visited,
> const struct pmu_metrics_table *table)
> @@ -955,7 +963,7 @@ static int add_metric(struct list_head *metric_list,
> if (!strstr(pm->metric_expr, "?")) {
> ret = __add_metric(metric_list, pm, modifier, metric_no_group,
> metric_no_threshold, 0, user_requested_cpu_list,
> - system_wide, root_metric, visited, table);
> + system_wide, fake_pmu, root_metric, visited, table);
> } else {
> int j, count;
>
> @@ -969,7 +977,7 @@ static int add_metric(struct list_head *metric_list,
> for (j = 0; j < count && !ret; j++)
> ret = __add_metric(metric_list, pm, modifier, metric_no_group,
> metric_no_threshold, j, user_requested_cpu_list,
> - system_wide, root_metric, visited, table);
> + system_wide, fake_pmu, root_metric, visited, table);
> }
>
> return ret;
> @@ -1030,6 +1038,7 @@ struct metricgroup__add_metric_data {
> bool metric_no_group;
> bool metric_no_threshold;
> bool system_wide;
> + bool fake_pmu;
> bool has_match;
> };
>
> @@ -1047,7 +1056,7 @@ static int metricgroup__add_metric_callback(const struct pmu_metric *pm,
> data->has_match = true;
> ret = add_metric(data->list, pm, data->modifier, metric_no_group,
> data->metric_no_threshold, data->user_requested_cpu_list,
> - data->system_wide, /*root_metric=*/NULL,
> + data->system_wide, data->fake_pmu, /*root_metric=*/NULL,
> /*visited_metrics=*/NULL, table);
> }
> return ret;
> @@ -1065,6 +1074,7 @@ static int metricgroup__add_metric_callback(const struct pmu_metric *pm,
> * user may override.
> * @user_requested_cpu_list: Command line specified CPUs to record on.
> * @system_wide: Are events for all processes recorded.
> + * @fake_pmu: Are PMUs and events being faked for testing?
> * @metric_list: The list that the metric or metric group are added to.
> * @table: The table that is searched for metrics, most commonly the table for the
> * architecture perf is running upon.
> @@ -1072,7 +1082,7 @@ static int metricgroup__add_metric_callback(const struct pmu_metric *pm,
> static int metricgroup__add_metric(const char *pmu, const char *metric_name, const char *modifier,
> bool metric_no_group, bool metric_no_threshold,
> const char *user_requested_cpu_list,
> - bool system_wide,
> + bool system_wide, bool fake_pmu,
> struct list_head *metric_list,
> const struct pmu_metrics_table *table)
> {
> @@ -1087,6 +1097,7 @@ static int metricgroup__add_metric(const char *pmu, const char *metric_name, con
> .metric_no_threshold = metric_no_threshold,
> .user_requested_cpu_list = user_requested_cpu_list,
> .system_wide = system_wide,
> + .fake_pmu = fake_pmu,
> .has_match = false,
> };
>
> @@ -1118,6 +1129,7 @@ static int metricgroup__add_metric(const char *pmu, const char *metric_name, con
> * user may override.
> * @user_requested_cpu_list: Command line specified CPUs to record on.
> * @system_wide: Are events for all processes recorded.
> + * @fake_pmu: Are PMUs and events being faked for testing?
> * @metric_list: The list that metrics are added to.
> * @table: The table that is searched for metrics, most commonly the table for the
> * architecture perf is running upon.
> @@ -1126,7 +1138,8 @@ static int metricgroup__add_metric_list(const char *pmu, const char *list,
> bool metric_no_group,
> bool metric_no_threshold,
> const char *user_requested_cpu_list,
> - bool system_wide, struct list_head *metric_list,
> + bool system_wide, bool fake_pmu,
> + struct list_head *metric_list,
> const struct pmu_metrics_table *table)
> {
> char *list_itr, *list_copy, *metric_name, *modifier;
> @@ -1145,7 +1158,8 @@ static int metricgroup__add_metric_list(const char *pmu, const char *list,
> ret = metricgroup__add_metric(pmu, metric_name, modifier,
> metric_no_group, metric_no_threshold,
> user_requested_cpu_list,
> - system_wide, metric_list, table);
> + system_wide, fake_pmu,
> + metric_list, table);
> if (ret == -EINVAL)
> pr_err("Fail to parse metric or group `%s'\n", metric_name);
> else if (ret == -ENOENT)
> @@ -1403,7 +1417,8 @@ static int parse_groups(struct evlist *perf_evlist,
>
> ret = metricgroup__add_metric_list(pmu, str, metric_no_group, metric_no_threshold,
> user_requested_cpu_list,
> - system_wide, &metric_list, table);
> + system_wide, fake_pmu,
> + &metric_list, table);
> if (ret)
> goto out;
>
> --
> 2.56.0.rc1.310.g51773c2048-goog
prev parent reply other threads:[~2026-09-25 10:53 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-23 22:41 Ian Rogers
2026-09-25 10:53 ` Arnaldo Carvalho de Melo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=arZSpXYtyUW_UhDr@x2 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ctshao@google.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=leo.yan@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=pengyu@kylinos.cn \
--cc=peterz@infradead.org \
--cc=xiaqinxin@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®