| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fXL0Dw5v4VoNJhQ78adCHhvPQLzyjsophXx-j-FXGwuWg@mail.gmail.com>
Date: Tue, 3 Feb 2026 15:41:30 -0800
From: Ian Rogers <irogers@...gle.com>
To: Chun-Tse Shao <ctshao@...gle.com>
Cc: linux-kernel@...r.kernel.org, peterz@...radead.org, mingo@...hat.com,
acme@...nel.org, namhyung@...nel.org, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org, adrian.hunter@...el.com,
james.clark@...aro.org, kan.liang@...ux.intel.com, yang.lee@...ux.alibaba.com,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH] perf stat: Ensure metrics are displayed even with failed events
On Tue, Feb 3, 2026 at 3:07 PM Chun-Tse Shao <ctshao@...gle.com> wrote:
>
> Currently, `perf stat` skips or hides metrics when the underlying
> hardware events cannot be counted (e.g., due to insufficient permissions
> or unsupported events). In `--metric-only` mode, this often results in
> missing columns or blank spaces, making the output difficult to parse.
>
> Modify the logic to ensure metrics are consistently displayed by
> propagating NAN (Not a Number) through the expression evaluator.
> Specifically:
>
> 1. Update `prepare_metric()` in stat-shadow.c to treat uncounted events
> (where `run == 0`) as NAN. This leverages the existing math in expr.y
> to propagate NAN through metric expressions.
> 2. Remove the early return in the display logic's `printout()` function
> that was previously skipping metrics in `--metric-only` mode for
> failed events.
> 3. Simplify `perf_stat__skip_metric_event()` to no longer depend on
> event runtime.
>
> Tested:
> 1. `perf all metrics test` did not crash while paranoid is 2.
> 2. Multiple combinations with `CPUs_utilized` while paranoid is 2.
>
> $ ./perf stat -M CPUs_utilized -a -- sleep 1
>
> Performance counter stats for 'system wide':
>
> <not supported> msec cpu-clock:u # nan CPUs CPUs_utilized
> 1,006,356,120 duration_time
>
> 1.004375550 seconds time elapsed
>
> $ ./perf stat -M CPUs_utilized -a -j -- sleep 1
> {"counter-value" : "<not supported>", "unit" : "msec", "event" : "cpu-clock:u", "event-runtime" : 0, "pcnt-running" : 100.00, "metric-value" : "nan", "metric-unit" : "CPUs CPUs_utilized"}
> {"counter-value" : "1006642462.000000", "unit" : "", "event" : "duration_time", "event-runtime" : 1, "pcnt-running" : 100.00}
>
> $ ./perf stat -M CPUs_utilized -a --metric-only -- sleep 1
>
> Performance counter stats for 'system wide':
>
> CPUs CPUs_utilized
> nan
>
> 1.004424652 seconds time elapsed
>
> $ ./perf stat -M CPUs_utilized -a --metric-only -j -- sleep 1
> {"CPUs CPUs_utilized" : "none"}
>
> Signed-off-by: ctshao@...gle.com
Reviewed-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/util/stat-display.c | 59 +++++++++++++++-------------------
> tools/perf/util/stat-shadow.c | 8 ++---
> tools/perf/util/stat.h | 2 +-
> 3 files changed, 29 insertions(+), 40 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 2ce0602974a1..dc2b66855f6c 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -820,12 +820,6 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
> }
>
> if (run == 0 || ena == 0 || counter->counts->scaled == -1) {
> - if (config->metric_only) {
> - pm(config, os, METRIC_THRESHOLD_UNKNOWN, /*format=*/NULL,
> - /*unit=*/NULL, /*val=*/0);
> - return;
> - }
> -
> ok = false;
>
> if (counter->supported) {
> @@ -848,33 +842,32 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
> print_running(config, os, run, ena, /*before_metric=*/true);
> }
>
> - if (ok) {
> - if (!config->metric_only && counter->default_metricgroup && !counter->default_show_events) {
> - void *from = NULL;
> -
> - aggr_printout(config, os, os->evsel, os->id, os->aggr_nr);
> - /* Print out all the metricgroup with the same metric event. */
> - do {
> - int num = 0;
> -
> - /* Print out the new line for the next new metricgroup. */
> - if (from) {
> - if (config->json_output)
> - new_line_json(config, (void *)os);
> - else
> - __new_line_std_csv(config, os);
> - }
> -
> - print_noise(config, os, counter, noise, /*before_metric=*/true);
> - print_running(config, os, run, ena, /*before_metric=*/true);
> - from = perf_stat__print_shadow_stats_metricgroup(config, counter, aggr_idx,
> - &num, from, &out);
> - } while (from != NULL);
> - } else {
> - perf_stat__print_shadow_stats(config, counter, aggr_idx, &out);
> - }
> + if (!config->metric_only && counter->default_metricgroup &&
> + !counter->default_show_events) {
> + void *from = NULL;
> +
> + aggr_printout(config, os, os->evsel, os->id, os->aggr_nr);
> + /* Print out all the metricgroup with the same metric event. */
> + do {
> + int num = 0;
> +
> + /* Print out the new line for the next new metricgroup. */
> + if (from) {
> + if (config->json_output)
> + new_line_json(config, (void *)os);
> + else
> + __new_line_std_csv(config, os);
> + }
> +
> + print_noise(config, os, counter, noise,
> + /*before_metric=*/true);
> + print_running(config, os, run, ena,
> + /*before_metric=*/true);
> + from = perf_stat__print_shadow_stats_metricgroup(
> + config, counter, aggr_idx, &num, from, &out);
> + } while (from != NULL);
> } else {
> - pm(config, os, METRIC_THRESHOLD_UNKNOWN, /*format=*/NULL, /*unit=*/NULL, /*val=*/0);
> + perf_stat__print_shadow_stats(config, counter, aggr_idx, &out);
> }
>
> if (!config->metric_only) {
> @@ -987,7 +980,7 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> ena = aggr->counts.ena;
> run = aggr->counts.run;
>
> - if (perf_stat__skip_metric_event(counter, ena, run))
> + if (perf_stat__skip_metric_event(counter))
> return;
>
> if (val == 0 && should_skip_zero_counter(config, counter, &id))
> diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
> index 9c83f7d96caa..5d8d09e0e6ae 100644
> --- a/tools/perf/util/stat-shadow.c
> +++ b/tools/perf/util/stat-shadow.c
> @@ -83,7 +83,7 @@ static int prepare_metric(struct perf_stat_config *config,
> }
> /* Time events are always on CPU0, the first aggregation index. */
> aggr = &ps->aggr[is_tool_time ? tool_aggr_idx : aggr_idx];
> - if (!aggr || !metric_events[i]->supported) {
> + if (!aggr || !metric_events[i]->supported || aggr->counts.run == 0) {
This will conflict with:
https://lore.kernel.org/linux-perf-users/20260203225129.4077140-3-irogers@google.com/
but the same addition just needs adding in this place too.
Thanks,
Ian
> /*
> * Not supported events will have a count of 0, which
> * can be confusing in a metric. Explicitly set the
> @@ -335,14 +335,10 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config,
> * perf_stat__skip_metric_event - Skip the evsel in the Default metricgroup,
> * if it's not running or not the metric event.
> */
> -bool perf_stat__skip_metric_event(struct evsel *evsel,
> - u64 ena, u64 run)
> +bool perf_stat__skip_metric_event(struct evsel *evsel)
> {
> if (!evsel->default_metricgroup)
> return false;
>
> - if (!ena || !run)
> - return true;
> -
> return !metricgroup__lookup(&evsel->evlist->metric_events, evsel, false);
> }
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index f986911c9296..4bced233d2fc 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -163,7 +163,7 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config,
> struct evsel *evsel,
> int aggr_idx,
> struct perf_stat_output_ctx *out);
> -bool perf_stat__skip_metric_event(struct evsel *evsel, u64 ena, u64 run);
> +bool perf_stat__skip_metric_event(struct evsel *evsel);
> void *perf_stat__print_shadow_stats_metricgroup(struct perf_stat_config *config,
> struct evsel *evsel,
> int aggr_idx,
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
Powered by blists - more mailing lists