Skip to content

Commit 7d45f40

Browse files
captain5050acmel
authored andcommitted
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>
1 parent ef60b8f commit 7d45f40

8 files changed

Lines changed: 177 additions & 174 deletions

File tree

tools/perf/builtin-record.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "util/target.h"
2727
#include "util/session.h"
2828
#include "util/tool.h"
29+
#include "util/stat.h"
2930
#include "util/symbol.h"
3031
#include "util/record.h"
3132
#include "util/cpumap.h"
@@ -2484,7 +2485,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
24842485
pr_warning("WARNING: --timestamp-filename option is not available in pipe mode.\n");
24852486
}
24862487

2487-
evlist__uniquify_name(rec->evlist);
2488+
/*
2489+
* Use global stat_config that is zero meaning aggr_mode is AGGR_NONE
2490+
* and hybrid_merge is false.
2491+
*/
2492+
evlist__uniquify_evsel_names(rec->evlist, &stat_config);
24882493

24892494
evlist__config(rec->evlist, opts, &callchain_param);
24902495

tools/perf/builtin-top.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "util/mmap.h"
3636
#include "util/session.h"
3737
#include "util/thread.h"
38+
#include "util/stat.h"
3839
#include "util/symbol.h"
3940
#include "util/synthetic-events.h"
4041
#include "util/top.h"
@@ -1309,7 +1310,11 @@ static int __cmd_top(struct perf_top *top)
13091310
}
13101311
}
13111312

1312-
evlist__uniquify_name(top->evlist);
1313+
/*
1314+
* Use global stat_config that is zero meaning aggr_mode is AGGR_NONE
1315+
* and hybrid_merge is false.
1316+
*/
1317+
evlist__uniquify_evsel_names(top->evlist, &stat_config);
13131318
ret = perf_top__start_counters(top);
13141319
if (ret)
13151320
return ret;

tools/perf/util/evlist.c

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2565,34 +2565,56 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
25652565
perf_cpu_map__put(user_requested_cpus);
25662566
}
25672567

2568-
void evlist__uniquify_name(struct evlist *evlist)
2568+
/* Should uniquify be disabled for the evlist? */
2569+
static bool evlist__disable_uniquify(const struct evlist *evlist)
25692570
{
2570-
char *new_name, empty_attributes[2] = ":", *attributes;
2571-
struct evsel *pos;
2571+
struct evsel *counter;
2572+
struct perf_pmu *last_pmu = NULL;
2573+
bool first = true;
25722574

2573-
if (perf_pmus__num_core_pmus() == 1)
2574-
return;
2575+
evlist__for_each_entry(evlist, counter) {
2576+
/* If PMUs vary then uniquify can be useful. */
2577+
if (!first && counter->pmu != last_pmu)
2578+
return false;
2579+
first = false;
2580+
if (counter->pmu) {
2581+
/* Allow uniquify for uncore PMUs. */
2582+
if (!counter->pmu->is_core)
2583+
return false;
2584+
/* Keep hybrid event names uniquified for clarity. */
2585+
if (perf_pmus__num_core_pmus() > 1)
2586+
return false;
2587+
}
2588+
last_pmu = counter->pmu;
2589+
}
2590+
return true;
2591+
}
25752592

2576-
evlist__for_each_entry(evlist, pos) {
2577-
if (!evsel__is_hybrid(pos))
2578-
continue;
2593+
static bool evlist__set_needs_uniquify(struct evlist *evlist, const struct perf_stat_config *config)
2594+
{
2595+
struct evsel *counter;
2596+
bool needs_uniquify = false;
25792597

2580-
if (strchr(pos->name, '/'))
2581-
continue;
2598+
if (evlist__disable_uniquify(evlist)) {
2599+
evlist__for_each_entry(evlist, counter)
2600+
counter->uniquified_name = true;
2601+
return false;
2602+
}
2603+
2604+
evlist__for_each_entry(evlist, counter) {
2605+
if (evsel__set_needs_uniquify(counter, config))
2606+
needs_uniquify = true;
2607+
}
2608+
return needs_uniquify;
2609+
}
25822610

2583-
attributes = strchr(pos->name, ':');
2584-
if (attributes)
2585-
*attributes = '\0';
2586-
else
2587-
attributes = empty_attributes;
2611+
void evlist__uniquify_evsel_names(struct evlist *evlist, const struct perf_stat_config *config)
2612+
{
2613+
if (evlist__set_needs_uniquify(evlist, config)) {
2614+
struct evsel *pos;
25882615

2589-
if (asprintf(&new_name, "%s/%s/%s", pos->pmu ? pos->pmu->name : "",
2590-
pos->name, attributes + 1)) {
2591-
free(pos->name);
2592-
pos->name = new_name;
2593-
} else {
2594-
*attributes = ':';
2595-
}
2616+
evlist__for_each_entry(evlist, pos)
2617+
evsel__uniquify_counter(pos);
25962618
}
25972619
}
25982620

tools/perf/util/evlist.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
struct pollfd;
2020
struct thread_map;
2121
struct perf_cpu_map;
22+
struct perf_stat_config;
2223
struct record_opts;
2324
struct strbuf;
2425
struct target;
@@ -434,7 +435,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx);
434435
void evlist__format_evsels(struct evlist *evlist, struct strbuf *sb, size_t max_length);
435436
void evlist__check_mem_load_aux(struct evlist *evlist);
436437
void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list);
437-
void evlist__uniquify_name(struct evlist *evlist);
438+
void evlist__uniquify_evsel_names(struct evlist *evlist, const struct perf_stat_config *config);
438439
bool evlist__has_bpf_output(struct evlist *evlist);
439440
bool evlist__needs_bpf_sb_event(struct evlist *evlist);
440441

tools/perf/util/evsel.c

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3954,3 +3954,116 @@ void evsel__remove_from_group(struct evsel *evsel, struct evsel *leader)
39543954
leader->core.nr_members--;
39553955
}
39563956
}
3957+
3958+
bool evsel__set_needs_uniquify(struct evsel *counter, const struct perf_stat_config *config)
3959+
{
3960+
struct evsel *evsel;
3961+
3962+
if (counter->needs_uniquify) {
3963+
/* Already set. */
3964+
return true;
3965+
}
3966+
3967+
if (counter->merged_stat) {
3968+
/* Counter won't be shown. */
3969+
return false;
3970+
}
3971+
3972+
if (counter->use_config_name || counter->is_libpfm_event) {
3973+
/* Original name will be used. */
3974+
return false;
3975+
}
3976+
3977+
if (!config->hybrid_merge && evsel__is_hybrid(counter)) {
3978+
/* Unique hybrid counters necessary. */
3979+
counter->needs_uniquify = true;
3980+
return true;
3981+
}
3982+
3983+
if (counter->core.attr.type < PERF_TYPE_MAX && counter->core.attr.type != PERF_TYPE_RAW) {
3984+
/* Legacy event, don't uniquify. */
3985+
return false;
3986+
}
3987+
3988+
if (counter->pmu && counter->pmu->is_core &&
3989+
counter->alternate_hw_config != PERF_COUNT_HW_MAX) {
3990+
/* A sysfs or json event replacing a legacy event, don't uniquify. */
3991+
return false;
3992+
}
3993+
3994+
if (config->aggr_mode == AGGR_NONE) {
3995+
/* Always unique with no aggregation. */
3996+
counter->needs_uniquify = true;
3997+
return true;
3998+
}
3999+
4000+
/*
4001+
* Do other non-merged events in the evlist have the same name? If so
4002+
* uniquify is necessary.
4003+
*/
4004+
evlist__for_each_entry(counter->evlist, evsel) {
4005+
if (evsel == counter || evsel->merged_stat || evsel->pmu == counter->pmu)
4006+
continue;
4007+
4008+
if (evsel__name_is(counter, evsel__name(evsel))) {
4009+
counter->needs_uniquify = true;
4010+
return true;
4011+
}
4012+
}
4013+
return false;
4014+
}
4015+
4016+
void evsel__uniquify_counter(struct evsel *counter)
4017+
{
4018+
const char *name, *pmu_name;
4019+
char *new_name, *config;
4020+
int ret;
4021+
4022+
/* No uniquification necessary. */
4023+
if (!counter->needs_uniquify)
4024+
return;
4025+
4026+
/* The evsel was already uniquified. */
4027+
if (counter->uniquified_name)
4028+
return;
4029+
4030+
/* Avoid checking to uniquify twice. */
4031+
counter->uniquified_name = true;
4032+
4033+
name = evsel__name(counter);
4034+
pmu_name = counter->pmu->name;
4035+
/* Already prefixed by the PMU name. */
4036+
if (!strncmp(name, pmu_name, strlen(pmu_name)))
4037+
return;
4038+
4039+
config = strchr(name, '/');
4040+
if (config) {
4041+
int len = config - name;
4042+
4043+
if (config[1] == '/') {
4044+
/* case: event// */
4045+
ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 2);
4046+
} else {
4047+
/* case: event/.../ */
4048+
ret = asprintf(&new_name, "%s/%.*s,%s", pmu_name, len, name, config + 1);
4049+
}
4050+
} else {
4051+
config = strchr(name, ':');
4052+
if (config) {
4053+
/* case: event:.. */
4054+
int len = config - name;
4055+
4056+
ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 1);
4057+
} else {
4058+
/* case: event */
4059+
ret = asprintf(&new_name, "%s/%s/", pmu_name, name);
4060+
}
4061+
}
4062+
if (ret > 0) {
4063+
free(counter->name);
4064+
counter->name = new_name;
4065+
} else {
4066+
/* ENOMEM from asprintf. */
4067+
counter->uniquified_name = false;
4068+
}
4069+
}

tools/perf/util/evsel.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
struct bpf_object;
1717
struct cgroup;
1818
struct perf_counts;
19+
struct perf_stat_config;
1920
struct perf_stat_evsel;
2021
union perf_event;
2122
struct bpf_counter_ops;
@@ -548,6 +549,9 @@ void evsel__remove_from_group(struct evsel *evsel, struct evsel *leader);
548549

549550
bool arch_evsel__must_be_in_group(const struct evsel *evsel);
550551

552+
bool evsel__set_needs_uniquify(struct evsel *counter, const struct perf_stat_config *config);
553+
void evsel__uniquify_counter(struct evsel *counter);
554+
551555
/*
552556
* Macro to swap the bit-field postition and size.
553557
* Used when,

tools/perf/util/parse-events.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2267,7 +2267,7 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
22672267
if (verbose > 0) {
22682268
struct strbuf sb = STRBUF_INIT;
22692269

2270-
evlist__uniquify_name(evlist);
2270+
evlist__uniquify_evsel_names(evlist, &stat_config);
22712271
evlist__format_evsels(evlist, &sb, 2048);
22722272
pr_debug("evlist after sorting/fixing: '%s'\n", sb.buf);
22732273
strbuf_release(&sb);

0 commit comments

Comments
 (0)