perf evlist: Make uniquifying counter names consistent

'perf stat' has different uniquification logic to 'perf record' and perf
top. In the case of perf record and 'perf top' all hybrid event names
are uniquified.

'perf stat' is more disciplined respecting name config terms, libpfm4
events, etc.

'perf stat' will uniquify hybrid events and the non-core PMU cases
shouldn't apply to perf record or 'perf top'.

For consistency, remove the uniquification for 'perf record' and 'perf
top' and reuse the 'perf stat' uniquification, making the code more
globally visible for this.

Fix the detection of cross-PMU for disabling uniquify by correctly
setting last_pmu.

When setting uniquify on an evsel, make sure the PMUs between the 2
considered events differ otherwise the uniquify isn't adding value.

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Chun-Tse Shao <ctshao@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dr. David Alan Gilbert <linux@treblig.org>
Cc: Howard Chu <howardchu95@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Levi Yun <yeoreum.yun@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Weilin Wang <weilin.wang@intel.com>
Link: https://lore.kernel.org/r/20250513215401.2315949-2-ctshao@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 52cebacc19a7eb5783fcd945520677057393c559..94d9c97af7b79a2ccaa44e03b2e749c56875452a 100644 (file)
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -26,6 +26,7 @@
 #include "util/target.h"
 #include "util/session.h"
 #include "util/tool.h"
+#include "util/stat.h"
 #include "util/symbol.h"
 #include "util/record.h"
 #include "util/cpumap.h"
@@ -2484,7 +2485,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
                pr_warning("WARNING: --timestamp-filename option is not available in pipe mode.\n");
        }
 
-       evlist__uniquify_name(rec->evlist);
+       /*
+        * Use global stat_config that is zero meaning aggr_mode is AGGR_NONE
+        * and hybrid_merge is false.
+        */
+       evlist__uniquify_evsel_names(rec->evlist, &stat_config);
 
        evlist__config(rec->evlist, opts, &callchain_param);
 

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f9f31391bddba0749e7047e56741385362b587cf..7b6cde87d2afd13f21744ecb632cb63afe97a657 100644 (file)
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -35,6 +35,7 @@
 #include "util/mmap.h"
 #include "util/session.h"
 #include "util/thread.h"
+#include "util/stat.h"
 #include "util/symbol.h"
 #include "util/synthetic-events.h"
 #include "util/top.h"
@@ -1309,7 +1310,11 @@ static int __cmd_top(struct perf_top *top)
                }
        }
 
-       evlist__uniquify_name(top->evlist);
+       /*
+        * Use global stat_config that is zero meaning aggr_mode is AGGR_NONE
+        * and hybrid_merge is false.
+        */
+       evlist__uniquify_evsel_names(top->evlist, &stat_config);
        ret = perf_top__start_counters(top);
        if (ret)
                return ret;

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 05d1f4e8ee225e6946e3a5c3ee82b5cd15decb21..dcd1130502dfc24ad49a3132de37fe460f46f307 100644 (file)
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -2565,34 +2565,56 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
        perf_cpu_map__put(user_requested_cpus);
 }
 
-void evlist__uniquify_name(struct evlist *evlist)
+/* Should uniquify be disabled for the evlist? */
+static bool evlist__disable_uniquify(const struct evlist *evlist)
 {
-       char *new_name, empty_attributes[2] = ":", *attributes;
-       struct evsel *pos;
+       struct evsel *counter;
+       struct perf_pmu *last_pmu = NULL;
+       bool first = true;
 
-       if (perf_pmus__num_core_pmus() == 1)
-               return;
+       evlist__for_each_entry(evlist, counter) {
+               /* If PMUs vary then uniquify can be useful. */
+               if (!first && counter->pmu != last_pmu)
+                       return false;
+               first = false;
+               if (counter->pmu) {
+                       /* Allow uniquify for uncore PMUs. */
+                       if (!counter->pmu->is_core)
+                               return false;
+                       /* Keep hybrid event names uniquified for clarity. */
+                       if (perf_pmus__num_core_pmus() > 1)
+                               return false;
+               }
+               last_pmu = counter->pmu;
+       }
+       return true;
+}
 
-       evlist__for_each_entry(evlist, pos) {
-               if (!evsel__is_hybrid(pos))
-                       continue;
+static bool evlist__set_needs_uniquify(struct evlist *evlist, const struct perf_stat_config *config)
+{
+       struct evsel *counter;
+       bool needs_uniquify = false;
 
-               if (strchr(pos->name, '/'))
-                       continue;
+       if (evlist__disable_uniquify(evlist)) {
+               evlist__for_each_entry(evlist, counter)
+                       counter->uniquified_name = true;
+               return false;
+       }
+
+       evlist__for_each_entry(evlist, counter) {
+               if (evsel__set_needs_uniquify(counter, config))
+                       needs_uniquify = true;
+       }
+       return needs_uniquify;
+}
 
-               attributes = strchr(pos->name, ':');
-               if (attributes)
-                       *attributes = '\0';
-               else
-                       attributes = empty_attributes;
+void evlist__uniquify_evsel_names(struct evlist *evlist, const struct perf_stat_config *config)
+{
+       if (evlist__set_needs_uniquify(evlist, config)) {
+               struct evsel *pos;
 
-               if (asprintf(&new_name, "%s/%s/%s", pos->pmu ? pos->pmu->name : "",
-                            pos->name, attributes + 1)) {
-                       free(pos->name);
-                       pos->name = new_name;
-               } else {
-                       *attributes = ':';
-               }
+               evlist__for_each_entry(evlist, pos)
+                       evsel__uniquify_counter(pos);
        }
 }
 

diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 21f6bff319fdfb03d913a7371fc5499844155dcc..85859708393ef1e3788f1d551123b6692680e982 100644 (file)
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -19,6 +19,7 @@
 struct pollfd;
 struct thread_map;
 struct perf_cpu_map;
+struct perf_stat_config;
 struct record_opts;
 struct strbuf;
 struct target;
@@ -434,7 +435,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx);
 void evlist__format_evsels(struct evlist *evlist, struct strbuf *sb, size_t max_length);
 void evlist__check_mem_load_aux(struct evlist *evlist);
 void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list);
-void evlist__uniquify_name(struct evlist *evlist);
+void evlist__uniquify_evsel_names(struct evlist *evlist, const struct perf_stat_config *config);
 bool evlist__has_bpf_output(struct evlist *evlist);
 bool evlist__needs_bpf_sb_event(struct evlist *evlist);
 

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c06b191e73985f942602a62f2e37d7d9c12a598a..34409828f8ec46b71c402151a22f66d4416a5ef0 100644 (file)
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3954,3 +3954,116 @@ void evsel__remove_from_group(struct evsel *evsel, struct evsel *leader)
                leader->core.nr_members--;
        }
 }
+
+bool evsel__set_needs_uniquify(struct evsel *counter, const struct perf_stat_config *config)
+{
+       struct evsel *evsel;
+
+       if (counter->needs_uniquify) {
+               /* Already set. */
+               return true;
+       }
+
+       if (counter->merged_stat) {
+               /* Counter won't be shown. */
+               return false;
+       }
+
+       if (counter->use_config_name || counter->is_libpfm_event) {
+               /* Original name will be used. */
+               return false;
+       }
+
+       if (!config->hybrid_merge && evsel__is_hybrid(counter)) {
+               /* Unique hybrid counters necessary. */
+               counter->needs_uniquify = true;
+               return true;
+       }
+
+       if  (counter->core.attr.type < PERF_TYPE_MAX && counter->core.attr.type != PERF_TYPE_RAW) {
+               /* Legacy event, don't uniquify. */
+               return false;
+       }
+
+       if (counter->pmu && counter->pmu->is_core &&
+           counter->alternate_hw_config != PERF_COUNT_HW_MAX) {
+               /* A sysfs or json event replacing a legacy event, don't uniquify. */
+               return false;
+       }
+
+       if (config->aggr_mode == AGGR_NONE) {
+               /* Always unique with no aggregation. */
+               counter->needs_uniquify = true;
+               return true;
+       }
+
+       /*
+        * Do other non-merged events in the evlist have the same name? If so
+        * uniquify is necessary.
+        */
+       evlist__for_each_entry(counter->evlist, evsel) {
+               if (evsel == counter || evsel->merged_stat || evsel->pmu == counter->pmu)
+                       continue;
+
+               if (evsel__name_is(counter, evsel__name(evsel))) {
+                       counter->needs_uniquify = true;
+                       return true;
+               }
+       }
+       return false;
+}
+
+void evsel__uniquify_counter(struct evsel *counter)
+{
+       const char *name, *pmu_name;
+       char *new_name, *config;
+       int ret;
+
+       /* No uniquification necessary. */
+       if (!counter->needs_uniquify)
+               return;
+
+       /* The evsel was already uniquified. */
+       if (counter->uniquified_name)
+               return;
+
+       /* Avoid checking to uniquify twice. */
+       counter->uniquified_name = true;
+
+       name = evsel__name(counter);
+       pmu_name = counter->pmu->name;
+       /* Already prefixed by the PMU name. */
+       if (!strncmp(name, pmu_name, strlen(pmu_name)))
+               return;
+
+       config = strchr(name, '/');
+       if (config) {
+               int len = config - name;
+
+               if (config[1] == '/') {
+                       /* case: event// */
+                       ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 2);
+               } else {
+                       /* case: event/.../ */
+                       ret = asprintf(&new_name, "%s/%.*s,%s", pmu_name, len, name, config + 1);
+               }
+       } else {
+               config = strchr(name, ':');
+               if (config) {
+                       /* case: event:.. */
+                       int len = config - name;
+
+                       ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 1);
+               } else {
+                       /* case: event */
+                       ret = asprintf(&new_name, "%s/%s/", pmu_name, name);
+               }
+       }
+       if (ret > 0) {
+               free(counter->name);
+               counter->name = new_name;
+       } else {
+               /* ENOMEM from asprintf. */
+               counter->uniquified_name = false;
+       }
+}

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 3d47d9318d927c9bec417e40eed75e3910c66bc5..df8d2eea04653004832fba9905603d9b9f300f71 100644 (file)
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -16,6 +16,7 @@
 struct bpf_object;
 struct cgroup;
 struct perf_counts;
+struct perf_stat_config;
 struct perf_stat_evsel;
 union perf_event;
 struct bpf_counter_ops;
@@ -548,6 +549,9 @@ void evsel__remove_from_group(struct evsel *evsel, struct evsel *leader);
 
 bool arch_evsel__must_be_in_group(const struct evsel *evsel);
 
+bool evsel__set_needs_uniquify(struct evsel *counter, const struct perf_stat_config *config);
+void evsel__uniquify_counter(struct evsel *counter);
+
 /*
  * Macro to swap the bit-field postition and size.
  * Used when,

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 177d045577c4f6040258b758e3229d6ad21e8e9b..e4ef8a828ac9f76b6769f3d57c95d9b5c4c0b46c 100644 (file)
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2267,7 +2267,7 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
                if (verbose > 0) {
                        struct strbuf sb = STRBUF_INIT;
 
-                       evlist__uniquify_name(evlist);
+                       evlist__uniquify_evsel_names(evlist, &stat_config);
                        evlist__format_evsels(evlist, &sb, 2048);
                        pr_debug("evlist after sorting/fixing: '%s'\n", sb.buf);
                        strbuf_release(&sb);

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e31d9f64d3aef5a36fe9bf7befa28dca08e82331..c022afb28514a4ece7c83fa85b6892c5f60a081c 100644 (file)
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -915,61 +915,6 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
        }
 }
 
-static void evsel__uniquify_counter(struct evsel *counter)
-{
-       const char *name, *pmu_name;
-       char *new_name, *config;
-       int ret;
-
-       /* No uniquification necessary. */
-       if (!counter->needs_uniquify)
-               return;
-
-       /* The evsel was already uniquified. */
-       if (counter->uniquified_name)
-               return;
-
-       /* Avoid checking to uniquify twice. */
-       counter->uniquified_name = true;
-
-       name = evsel__name(counter);
-       pmu_name = counter->pmu->name;
-       /* Already prefixed by the PMU name. */
-       if (!strncmp(name, pmu_name, strlen(pmu_name)))
-               return;
-
-       config = strchr(name, '/');
-       if (config) {
-               int len = config - name;
-
-               if (config[1] == '/') {
-                       /* case: event// */
-                       ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 2);
-               } else {
-                       /* case: event/.../ */
-                       ret = asprintf(&new_name, "%s/%.*s,%s", pmu_name, len, name, config + 1);
-               }
-       } else {
-               config = strchr(name, ':');
-               if (config) {
-                       /* case: event:.. */
-                       int len = config - name;
-
-                       ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 1);
-               } else {
-                       /* case: event */
-                       ret = asprintf(&new_name, "%s/%s/", pmu_name, name);
-               }
-       }
-       if (ret > 0) {
-               free(counter->name);
-               counter->name = new_name;
-       } else {
-               /* ENOMEM from asprintf. */
-               counter->uniquified_name = false;
-       }
-}
-
 /**
  * should_skip_zero_count() - Check if the event should print 0 values.
  * @config: The perf stat configuration (including aggregation mode).
@@ -1060,8 +1005,6 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
        if (counter->merged_stat)
                return;
 
-       evsel__uniquify_counter(counter);
-
        val = aggr->counts.val;
        ena = aggr->counts.ena;
        run = aggr->counts.run;
@@ -1636,96 +1579,6 @@ static void print_cgroup_counter(struct perf_stat_config *config, struct evlist
                print_metric_end(config, os);
 }
 
-/* Should uniquify be disabled for the evlist? */
-static bool evlist__disable_uniquify(const struct evlist *evlist)
-{
-       struct evsel *counter;
-       struct perf_pmu *last_pmu = NULL;
-       bool first = true;
-
-       evlist__for_each_entry(evlist, counter) {
-               /* If PMUs vary then uniquify can be useful. */
-               if (!first && counter->pmu != last_pmu)
-                       return false;
-               first = false;
-               if (counter->pmu) {
-                       /* Allow uniquify for uncore PMUs. */
-                       if (!counter->pmu->is_core)
-                               return false;
-                       /* Keep hybrid event names uniquified for clarity. */
-                       if (perf_pmus__num_core_pmus() > 1)
-                               return false;
-               }
-       }
-       return true;
-}
-
-static void evsel__set_needs_uniquify(struct evsel *counter, const struct perf_stat_config *config)
-{
-       struct evsel *evsel;
-
-       if (counter->merged_stat) {
-               /* Counter won't be shown. */
-               return;
-       }
-
-       if (counter->use_config_name || counter->is_libpfm_event) {
-               /* Original name will be used. */
-               return;
-       }
-
-       if (!config->hybrid_merge && evsel__is_hybrid(counter)) {
-               /* Unique hybrid counters necessary. */
-               counter->needs_uniquify = true;
-               return;
-       }
-
-       if  (counter->core.attr.type < PERF_TYPE_MAX && counter->core.attr.type != PERF_TYPE_RAW) {
-               /* Legacy event, don't uniquify. */
-               return;
-       }
-
-       if (counter->pmu && counter->pmu->is_core &&
-           counter->alternate_hw_config != PERF_COUNT_HW_MAX) {
-               /* A sysfs or json event replacing a legacy event, don't uniquify. */
-               return;
-       }
-
-       if (config->aggr_mode == AGGR_NONE) {
-               /* Always unique with no aggregation. */
-               counter->needs_uniquify = true;
-               return;
-       }
-
-       /*
-        * Do other non-merged events in the evlist have the same name? If so
-        * uniquify is necessary.
-        */
-       evlist__for_each_entry(counter->evlist, evsel) {
-               if (evsel == counter || evsel->merged_stat)
-                       continue;
-
-               if (evsel__name_is(counter, evsel__name(evsel))) {
-                       counter->needs_uniquify = true;
-                       return;
-               }
-       }
-}
-
-static void evlist__set_needs_uniquify(struct evlist *evlist, const struct perf_stat_config *config)
-{
-       struct evsel *counter;
-
-       if (evlist__disable_uniquify(evlist)) {
-               evlist__for_each_entry(evlist, counter)
-                       counter->uniquified_name = true;
-               return;
-       }
-
-       evlist__for_each_entry(evlist, counter)
-               evsel__set_needs_uniquify(counter, config);
-}
-
 void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *config,
                            struct target *_target, struct timespec *ts,
                            int argc, const char **argv)
@@ -1737,7 +1590,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
                .first = true,
        };
 
-       evlist__set_needs_uniquify(evlist, config);
+       evlist__uniquify_evsel_names(evlist, config);
 
        if (config->iostat_run)
                evlist->selected = evlist__first(evlist);