Re: [PATCH v6] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
From: Chun-Tse Shao
Date: Fri Oct 30 2026 - 15:27:21 EST
Hi Ian, thanks for your review, please check my v7 patch at:
lore.kernel.org/20261030192537.3079368-1-ctshao@xxxxxxxxxx
On Tue, Oct 27, 2026 at 3:10 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> On Tue, Oct 27, 2026 at 1:31 PM Chun-Tse Shao <ctshao@xxxxxxxxxx> wrote:
> >
> > When running `perf stat` with `-A` (--no-aggr) and `--metric-only` in
> > JSON output mode (`-j`), `perf stat` evaluates metric expressions
> > across all matching PMUs (including uncore PMUs like `uncore_iio_0`,
> > `uncore_iio_1`, etc.).
> >
> > However, `perf stat` previously formatted JSON output by printing only
> > "cpu" : "<id>" and grouping all metric values on a single line per CPU
> > without identifying which PMU instance evaluated each metric. As a
> > result, when an uncore event spans multiple PMU boxes, `perf stat`
> > printed repeated, ambiguous metric keys without PMU names.
> >
> > Fix this by:
> > 1. Including "pmu" : "<pmu_name>" in print_aggr_id_json when evsel->pmu
> > is a non-core or hybrid PMU in AGGR_NONE mode (-A). Tool events are
> > not labeled as they don't belong to a hardware PMU instance.
> > 2. Starting a new JSON metric line in AGGR_NONE mode (-A) whenever the
> > PMU name changes, so that a line only has metrics from the PMU it is
> > labeled with.
> > 3. Opening a metric line only for events that have a metric, as other
> > events print nothing in metric-only mode. This avoids spurious lines
> > like {"cpu" : "0", "pmu" : "tool"} for events like user_time, while
> > metrics on tool events, like tma_info_system_time (duration_time),
> > are still printed with the line prefix.
> > 4. Updating perf_json_output_lint.py to recognize the new "pmu" key in
> > the JSON test suite.
> >
> > Before the fix:
> > $ perf stat -M iio_bandwidth_read -a -A --metric-only -j -I 1000
> > {"interval" : 1.001017947, "cpu" : "0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "22.5", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.2", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0"}
> >
> > There is no way to determine which uncore device generated the metrics.
> >
> > After:
> > $ perf stat -M iio_bandwidth_read -a -A --metric-only -j -I 1000
> > {"interval" : 1.000314908, "cpu" : "0", "pmu" : "uncore_iio_0", "MB/s iio_bandwidth_read" : "0.0"}
> > {"interval" : 1.000314908, "cpu" : "0", "pmu" : "uncore_iio_1", "MB/s iio_bandwidth_read" : "0.1"}
> > {"interval" : 1.000314908, "cpu" : "0", "pmu" : "uncore_iio_11", "MB/s iio_bandwidth_read" : "0.0"}
> > ...
> > {"interval" : 1.000314908, "cpu" : "56", "pmu" : "uncore_iio_0", "MB/s iio_bandwidth_read" : "0.0"}
> > {"interval" : 1.000314908, "cpu" : "56", "pmu" : "uncore_iio_1", "MB/s iio_bandwidth_read" : "0.0"}
> > {"interval" : 1.000314908, "cpu" : "56", "pmu" : "uncore_iio_11", "MB/s iio_bandwidth_read" : "0.0"}
> >
> > Assisted-by: Gemini:gemini-3.1-pro-preview
> > Signed-off-by: Chun-Tse Shao <ctshao@xxxxxxxxxx>
> > ---
> > v6:
> > - Open metric lines for events that have a metric instead of skipping
> > tool events, so metrics on tool events (e.g. tma_info_system_time)
> > keep the line prefix and are terminated properly. (Sashiko)
> > - Split JSON lines only when the shown PMU name changes. Tool event
> > metrics are no longer mislabeled with an uncore PMU name, and the
> > STD and CSV output are no longer split.
> > - Rebase onto the latest perf-tools-next.
> > v5: lore.kernel.org/20260811230518.3360-2-ctshao@xxxxxxxxxx
> > - Fix perf test "perf stat JSON output linter" failure.
> > v4: lore.kernel.org/20260804210659.112192-1-ctshao@xxxxxxxxxx
> > - Fix the line splitting issue, causing spurious empty {"pmu": "tool"}
> > objects and order-dependent PMU mislabeling.
> > v3: lore.kernel.org/20260729170416.41904-1-ctshao@xxxxxxxxxx
> > - Add before the fix output.
> > v2: lore.kernel.org/20260715203055.2981363-1-ctshao@xxxxxxxxxx
> > - Fix print_metric_begin() initialization in print_no_aggr_metric()
> > to ensure tool events always open lines cleanly.
> >
> > .../tests/shell/lib/perf_json_output_lint.py | 5 +-
> > tools/perf/util/stat-display.c | 74 ++++++++++++++++---
> > 2 files changed, 66 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > index 8cde22ac9d97..982be893bd69 100644
> > --- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > +++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > @@ -65,6 +65,7 @@ def check_json_output(expected_count):
> > 'metricgroup': lambda x: True,
> > 'node': lambda x: True,
> > 'pcnt-running': isfloat,
> > + 'pmu': lambda x: True,
> > 'socket': lambda x: True,
> > 'thread': lambda x: True,
> > 'unit': lambda x: True,
> > @@ -99,8 +100,10 @@ def check_json_output(expected_count):
> > try:
> > if args.no_args or args.system_wide or args.event:
> > expected_items = [5, 7]
> > - elif args.interval or args.per_thread or args.system_wide_no_aggr:
> > + elif args.interval or args.per_thread:
> > expected_items = [6, 8]
> > + elif args.system_wide_no_aggr:
> > + expected_items = [6, 7, 8, 9]
> > elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cluster or args.per_cache:
> > expected_items = [7, 9]
> > elif args.metric_only:
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index b337cc23f413..9f1d74019d1e 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -1,6 +1,7 @@
> > #include <stdlib.h>
> > #include <stdio.h>
> > #include <inttypes.h>
> > +#include <string.h>
> > #include <linux/string.h>
> > #include <linux/time64.h>
> > #include <math.h>
> > @@ -23,6 +24,7 @@
> > #include "pmu.h"
> > #include "pmus.h"
> > #include "tool_pmu.h"
> > +#include "metricgroup.h"
> >
> > #define CNTR_NOT_SUPPORTED "<not supported>"
> > #define CNTR_NOT_COUNTED "<not counted>"
> > @@ -361,9 +363,33 @@ static void print_aggr_id_csv(struct perf_stat_config *config,
> > }
> > }
> >
> > +/*
> > + * In no-aggregation mode, JSON output includes the PMU name of non-core and
> > + * hybrid events so that events and metrics from different PMU instances, like
> > + * uncore_iio_0 and uncore_iio_1, can be told apart. Tool events don't belong
> > + * to a hardware PMU instance and so are not labeled.
> > + */
> > +static const char *json_pmu_name(const struct evsel *evsel)
> > +{
> > + if (!evsel || !evsel->pmu || evsel__is_tool(evsel))
> > + return NULL;
> > + if (evsel->pmu->is_core && !evsel__is_hybrid(evsel))
> > + return NULL;
> > + return evsel->pmu->name;
> > +}
> > +
> > +static bool json_pmu_name_changed(const char *name, const char *prev)
> > +{
> > + if (!name || !prev)
> > + return name != prev;
> > + return strcmp(name, prev) != 0;
> > +}
> > +
> > static void print_aggr_id_json(struct perf_stat_config *config, struct outstate *os,
> > struct evsel *evsel, struct aggr_cpu_id id, int aggr_nr)
> > {
> > + const char *pmu_name;
> > +
> > switch (config->aggr_mode) {
> > case AGGR_CORE:
> > json_out(os, "\"core\" : \"S%d-D%d-C%d\", \"counters\" : %d",
> > @@ -397,6 +423,9 @@ static void print_aggr_id_json(struct perf_stat_config *config, struct outstate
> > json_out(os, "\"cpu\" : \"%d\"",
> > id.cpu.cpu);
> > }
> > + pmu_name = json_pmu_name(evsel);
> > + if (pmu_name)
> > + json_out(os, "\"pmu\" : \"%s\"", pmu_name);
> > break;
> > case AGGR_THREAD:
> > json_out(os, "\"thread\" : \"%s-%d\"",
> > @@ -1012,11 +1041,12 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> >
> > static void print_metric_begin(struct perf_stat_config *config,
> > struct evlist *evlist,
> > - struct outstate *os, int aggr_idx)
> > + struct outstate *os, int aggr_idx,
> > + struct evsel *counter)
> > {
> > struct perf_stat_aggr *aggr;
> > struct aggr_cpu_id id;
> > - struct evsel *evsel;
> > + struct evsel *evsel = counter ?: evlist__first(evlist);
>
> This seems a bit random, what if there are two metrics? Will they have
> the same evsel?
I think the evsel is picked in printout, and print_metric_begin is
only for the shared prefix. Test on two metrics sharing the same PMU:
```
$ perf stat -a -A --metric-only -j -M
iio_bandwidth_write,iio_bandwidth_read -- sleep 1
{"cpu" : "0", "pmu" : "uncore_iio_1", "MB/s iio_bandwidth_read" : "0.0"}
{"cpu" : "0", "pmu" : "uncore_iio_11", "MB/s iio_bandwidth_read" : "0.0"}
{"cpu" : "0", "pmu" : "uncore_iio_12", "MB/s iio_bandwidth_read" : "22.5"}
...
{"cpu" : "0", "pmu" : "uncore_iio_11", "MB/s iio_bandwidth_write" : "0.6"}
{"cpu" : "0", "pmu" : "uncore_iio_12", "MB/s iio_bandwidth_write" : "0.0"}
{"cpu" : "0", "pmu" : "uncore_iio_14", "MB/s iio_bandwidth_write" : "0.0"}
...
```
It looks good to me.
Thanks,
CT
>
> >
> > os->first = true;
> > if (!config->metric_only)
> > @@ -1031,7 +1061,6 @@ static void print_metric_begin(struct perf_stat_config *config,
> > else
> > fprintf(config->output, "%s", os->timestamp);
> > }
> > - evsel = evlist__first(evlist);
> > id = config->aggr_map->map[aggr_idx];
> > aggr = &evsel->stats->aggr[aggr_idx];
> > aggr_printout(config, os, evsel, id, aggr->nr);
> > @@ -1069,7 +1098,7 @@ static void print_aggr(struct perf_stat_config *config,
> > * Without each counter has its own line.
> > */
> > cpu_aggr_map__for_each_idx(aggr_idx, config->aggr_map) {
> > - print_metric_begin(config, evlist, os, aggr_idx);
> > + print_metric_begin(config, evlist, os, aggr_idx, NULL);
>
> Please name arguments that are constants. Pass evlist__first here?
>
> >
> > evlist__for_each_entry(evlist, counter) {
> > print_counter_aggrdata(config, counter, aggr_idx, os);
> > @@ -1095,7 +1124,7 @@ static void print_aggr_cgroup(struct perf_stat_config *config,
> > os->cgrp = evsel->cgrp;
> >
> > cpu_aggr_map__for_each_idx(aggr_idx, config->aggr_map) {
> > - print_metric_begin(config, evlist, os, aggr_idx);
> > + print_metric_begin(config, evlist, os, aggr_idx, NULL);
>
> Use evsel here?
>
> >
> > evlist__for_each_entry(evlist, counter) {
> > if (counter->cgrp != os->cgrp)
> > @@ -1131,7 +1160,8 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> >
> > perf_cpu_map__for_each_cpu(cpu, all_idx, evlist__core(evlist)->user_requested_cpus) {
> > struct evsel *counter;
> > - bool first = true;
> > + const char *line_pmu = NULL;
> > + bool line_open = false;
> >
> > evlist__for_each_entry(evlist, counter) {
> > u64 ena, run, val;
> > @@ -1146,12 +1176,32 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> > if (config->aggr_map->map[aggr_idx].cpu.cpu == cpu.cpu)
> > break;
> > }
> > + if (aggr_idx >= config->aggr_map->nr)
> > + continue;
> >
> > os->evsel = counter;
> > os->id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
> > - if (first) {
> > - print_metric_begin(config, evlist, os, aggr_idx);
> > - first = false;
> > +
> > + /*
> > + * Only events with a metric print something in metric-only
> > + * mode, so let them open the line. For JSON, start a new
> > + * line when the PMU name changes so that a line only has
> > + * metrics from the PMU it is labeled with.
> > + */
> > + if (config->iostat_run ||
> > + metricgroup__lookup(evlist__metric_events(evlist), counter, false)) {
> > + const char *pmu_name = json_pmu_name(counter);
> > +
> > + if (line_open && config->json_output &&
> > + json_pmu_name_changed(pmu_name, line_pmu)) {
> > + print_metric_end(config, os);
> > + line_open = false;
> > + }
> > + if (!line_open) {
> > + print_metric_begin(config, evlist, os, aggr_idx, counter);
> > + line_pmu = pmu_name;
> > + line_open = true;
> > + }
> > }
> > val = ps->aggr[aggr_idx].counts.val;
> > ena = ps->aggr[aggr_idx].counts.ena;
> > @@ -1160,7 +1210,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> > uval = val * counter->scale;
> > printout(config, os, uval, run, ena, 1.0, aggr_idx);
> > }
> > - if (!first)
> > + if (line_open)
> > print_metric_end(config, os);
> > }
> > }
> > @@ -1523,7 +1573,7 @@ static void print_cgroup_counter(struct perf_stat_config *config, struct evlist
> > print_metric_end(config, os);
> >
> > os->cgrp = counter->cgrp;
> > - print_metric_begin(config, evlist, os, /*aggr_idx=*/0);
> > + print_metric_begin(config, evlist, os, /*aggr_idx=*/0, NULL);
>
> In this case, would passing counter be more obvious? The function will
> turn this into evlist__first(evlist) but there's no guarantee the
> event comes first.
>
> > }
> >
> > print_counter(config, counter, os);
> > @@ -1573,7 +1623,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
> > } else if (config->cgroup_list) {
> > print_cgroup_counter(config, evlist, &os);
> > } else {
> > - print_metric_begin(config, evlist, &os, /*aggr_idx=*/0);
> > + print_metric_begin(config, evlist, &os, /*aggr_idx=*/0, NULL);
>
> Maybe pass evlist__first here?
>
> Thanks,
> Ian
>
> > evlist__for_each_entry(evlist, counter) {
> > print_counter(config, counter, &os);
> > }
> > --
> > 2.56.0.317.g454f8302e1-goog
> >