Re: [PATCH v6] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output

[Ian Rogers]

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?

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