[PATCH] perf stat: Check aliases in should_skip_zero_counter

[PATCH] perf stat: Check aliases in should_skip_zero_counter

[Chun-Tse Shao]

Aggregation IDs (e.g. sockets, caches) with event zero counts are hidden
in aggregated modes (like `--per-socket`) because alias merging
aggregates counts but not CPU maps. This causes display logic to skip
aggregation IDs not present in the leader's map.

Instead of mutating CPU maps in `evsel__merge_aggr_counters` (which
could have side effects on shared CPU maps), check aliases in
`should_skip_zero_counter` to ensure all applicable aggregation IDs are
displayed.

Before the fix:
  $ perf stat -e amd_umc/config=0xff/ --per-socket -a -- sleep 1

   Performance counter stats for 'system wide':

  S0       12                  0      amd_umc/config=0xff/

         1.000721604 seconds time elapsed

After the fix:
  $ perf stat -e amd_umc/config=0xff/ --per-socket -a -- sleep 1

   Performance counter stats for 'system wide':

  S0       12                  0      amd_umc/config=0xff/
  S1       12                  0      amd_umc/config=0xff/

         1.000879376 seconds time elapsed

`perf test` looks good too.

Assisted-by: Gemini:gemini-3.1-pro-preview
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
 tools/perf/util/stat-display.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 993f4c4b8f44..c12a3405118a 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -945,6 +945,27 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
 		if (aggr_cpu_id__equal(id, &own_id))
 			return false;
 	}
+
+	/*
+	 * If the counter is a leader in alias merging, check if the aggr id
+	 * matches any of its aliases.
+	 */
+	if (config->aggr_mode != AGGR_NONE && counter->first_wildcard_match == NULL) {
+		struct evsel *alias;
+
+		evlist__for_each_entry(counter->evlist, alias) {
+			if (alias->first_wildcard_match == counter) {
+				perf_cpu_map__for_each_cpu(cpu, idx, alias->core.cpus) {
+					struct aggr_cpu_id own_id =
+						config->aggr_get_id(config, cpu);
+
+					if (aggr_cpu_id__equal(id, &own_id))
+						return false;
+				}
+			}
+		}
+	}
+
 	return true;
 }
 
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH] perf stat: Check aliases in should_skip_zero_counter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The alias checking loop introduces an O(M * N^2) time complexity during the display phase, causing unnecessary overhead.
--

commit 1ba80061bda839b47a8c18bdc8aa600c735044d6
Author: Chun-Tse Shao <ctshao@google.com>

perf stat: Check aliases in should_skip_zero_counter

This commit checks aliases directly in should_skip_zero_counter instead of
mutating CPU maps in evsel__merge_aggr_counters. This is done to prevent
zero count aggregation IDs from being hidden in aggregated modes like
--per-socket.

> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 993f4c4b8f442..c12a3405118a5 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -945,6 +945,27 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
>  		if (aggr_cpu_id__equal(id, &own_id))
>  			return false;
>  	}
> +
> +	/*
> +	 * If the counter is a leader in alias merging, check if the aggr id
> +	 * matches any of its aliases.
> +	 */
> +	if (config->aggr_mode != AGGR_NONE && counter->first_wildcard_match == NULL) {
> +		struct evsel *alias;
> +
> +		evlist__for_each_entry(counter->evlist, alias) {

Does this introduce an O(M * N^2) time complexity during the display phase?

Since should_skip_zero_counter() is called for each aggregation unit (M) and
each event (N), the function is called O(M * N) times.

If the event is a standalone event or an alias leader, this block iterates
over the entire evlist (N events) to find its aliases.

Could this scale poorly for very large event lists by introducing unnecessary
iterations, leading to an overall O(M * N^2) complexity?

> +			if (alias->first_wildcard_match == counter) {
> +				perf_cpu_map__for_each_cpu(cpu, idx, alias->core.cpus) {
> +					struct aggr_cpu_id own_id =
> +						config->aggr_get_id(config, cpu);
> +
> +					if (aggr_cpu_id__equal(id, &own_id))
> +						return false;
> +				}
> +			}
> +		}
> +	}
> +
>  	return true;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260501181129.938256-1-ctshao@google.com?part=1

Re: [PATCH] perf stat: Check aliases in should_skip_zero_counter

[Ian Rogers]

On Fri, May 1, 2026 at 11:11 AM Chun-Tse Shao <ctshao@google.com> wrote:
>
> Aggregation IDs (e.g. sockets, caches) with event zero counts are hidden
> in aggregated modes (like `--per-socket`) because alias merging
> aggregates counts but not CPU maps. This causes display logic to skip
> aggregation IDs not present in the leader's map.
>
> Instead of mutating CPU maps in `evsel__merge_aggr_counters` (which
> could have side effects on shared CPU maps), check aliases in
> `should_skip_zero_counter` to ensure all applicable aggregation IDs are
> displayed.
>
> Before the fix:
>   $ perf stat -e amd_umc/config=0xff/ --per-socket -a -- sleep 1
>
>    Performance counter stats for 'system wide':
>
>   S0       12                  0      amd_umc/config=0xff/
>
>          1.000721604 seconds time elapsed
>
> After the fix:
>   $ perf stat -e amd_umc/config=0xff/ --per-socket -a -- sleep 1
>
>    Performance counter stats for 'system wide':
>
>   S0       12                  0      amd_umc/config=0xff/
>   S1       12                  0      amd_umc/config=0xff/
>
>          1.000879376 seconds time elapsed

Does this change the behavior without --per-socket when the uncore
PMUs have different cpumasks?

> `perf test` looks good too.
>
> Assisted-by: Gemini:gemini-3.1-pro-preview
> Signed-off-by: Chun-Tse Shao <ctshao@google.com>
> ---
>  tools/perf/util/stat-display.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 993f4c4b8f44..c12a3405118a 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -945,6 +945,27 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
>                 if (aggr_cpu_id__equal(id, &own_id))
>                         return false;
>         }
> +
> +       /*
> +        * If the counter is a leader in alias merging, check if the aggr id
> +        * matches any of its aliases.

Could you expand why in this case we want the zero count?

> +        */
> +       if (config->aggr_mode != AGGR_NONE && counter->first_wildcard_match == NULL) {
> +               struct evsel *alias;
> +
> +               evlist__for_each_entry(counter->evlist, alias) {
> +                       if (alias->first_wildcard_match == counter) {

Could this also have a "&& !perf_cpu_map__equal(counter->core.cpu,
alias->core.cpus)" ? ie if the alias' cpumask matches that of the
counter, then the loop above covers it. Perhaps the cpumaps of each
alias could be merged and the test only run if their intersection is
not empty.

> +                               perf_cpu_map__for_each_cpu(cpu, idx, alias->core.cpus) {
> +                                       struct aggr_cpu_id own_id =
> +                                               config->aggr_get_id(config, cpu);

Could this fit on one line?

Thanks,
Ian

> +
> +                                       if (aggr_cpu_id__equal(id, &own_id))
> +                                               return false;
> +                               }
> +                       }
> +               }
> +       }
> +
>         return true;
>  }
>
> --
> 2.54.0.545.g6539524ca2-goog
>

Re: [PATCH] perf stat: Check aliases in should_skip_zero_counter

[Chun-Tse Shao]

Hi Ian, thanks for your review.

On Fri, May 1, 2026 at 12:12 PM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, May 1, 2026 at 11:11 AM Chun-Tse Shao <ctshao@google.com> wrote:
> >
> > Aggregation IDs (e.g. sockets, caches) with event zero counts are hidden
> > in aggregated modes (like `--per-socket`) because alias merging
> > aggregates counts but not CPU maps. This causes display logic to skip
> > aggregation IDs not present in the leader's map.
> >
> > Instead of mutating CPU maps in `evsel__merge_aggr_counters` (which
> > could have side effects on shared CPU maps), check aliases in
> > `should_skip_zero_counter` to ensure all applicable aggregation IDs are
> > displayed.
> >
> > Before the fix:
> >   $ perf stat -e amd_umc/config=0xff/ --per-socket -a -- sleep 1
> >
> >    Performance counter stats for 'system wide':
> >
> >   S0       12                  0      amd_umc/config=0xff/
> >
> >          1.000721604 seconds time elapsed
> >
> > After the fix:
> >   $ perf stat -e amd_umc/config=0xff/ --per-socket -a -- sleep 1
> >
> >    Performance counter stats for 'system wide':
> >
> >   S0       12                  0      amd_umc/config=0xff/
> >   S1       12                  0      amd_umc/config=0xff/
> >
> >          1.000879376 seconds time elapsed
>
> Does this change the behavior without --per-socket when the uncore
> PMUs have different cpumasks?

Nope, the behavior is unchanged for `-a`, since counts aggregate to
the leader's `aggr[0]` and all CPUs's `aggr_id` is 0 in AGGR_GLOBAL
mode.

>
> > `perf test` looks good too.
> >
> > Assisted-by: Gemini:gemini-3.1-pro-preview
> > Signed-off-by: Chun-Tse Shao <ctshao@google.com>
> > ---
> >  tools/perf/util/stat-display.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 993f4c4b8f44..c12a3405118a 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -945,6 +945,27 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
> >                 if (aggr_cpu_id__equal(id, &own_id))
> >                         return false;
> >         }
> > +
> > +       /*
> > +        * If the counter is a leader in alias merging, check if the aggr id
> > +        * matches any of its aliases.
>
> Could you expand why in this case we want the zero count?

We don't want to hide counts if it is not in leader's cpumask even
though it is 0. It would cause confusion to users/parsers.
In my scenario, I tested an event that was all zeros on S1 and was
hidden by perf. Initially, I thought perf wasn't collecting the event
on S1, then realized collecting was working but hidden.
>
> > +        */
> > +       if (config->aggr_mode != AGGR_NONE && counter->first_wildcard_match == NULL) {
> > +               struct evsel *alias;
> > +
> > +               evlist__for_each_entry(counter->evlist, alias) {
> > +                       if (alias->first_wildcard_match == counter) {
>
> Could this also have a "&& !perf_cpu_map__equal(counter->core.cpu,
> alias->core.cpus)" ? ie if the alias' cpumask matches that of the
> counter, then the loop above covers it. Perhaps the cpumaps of each
> alias could be merged and the test only run if their intersection is
> not empty.

Thanks for the suggestion, I will add it in the next patch.
>
> > +                               perf_cpu_map__for_each_cpu(cpu, idx, alias->core.cpus) {
> > +                                       struct aggr_cpu_id own_id =
> > +                                               config->aggr_get_id(config, cpu);
>
> Could this fit on one line?

Then it will make the line 101 characters long...
>
> Thanks,
> Ian
>
> > +
> > +                                       if (aggr_cpu_id__equal(id, &own_id))
> > +                                               return false;
> > +                               }
> > +                       }
> > +               }
> > +       }
> > +
> >         return true;
> >  }
> >
> > --
> > 2.54.0.545.g6539524ca2-goog
> >