[PATCH 1/2] perf tools: Always uniquify event names

James Clark posted 2 patches 2 weeks, 1 day ago
[PATCH 1/2] perf tools: Always uniquify event names
Posted by James Clark 2 weeks, 1 day ago
evlist__uniquify_evsel_names() only gets called in __parse_events() if
verbose is > 0. This means that the auto added "slots" events stay as
"slots" rather than being expanded to "cpu_core/slots/" unless Perf is
run in verbose mode. This is invisible to users when running Perf stat
because evlist__print_counters() always calls it regardless of verbose
mode before displaying.

The only thing this seems to affect is the test "Parsing of all PMU
events from sysfs" which fails when not run in verbose mode.
test__checkevent_pmu_events() always expects event names to be prefixed
with the pmu name, but this only happens for "slots" events after
evlist__uniquify_evsel_names() is called.

One fix could be to relax the test to accept the non prefixed name in
normal mode. But seeing as Perf stat uniquifies unconditionally, make
parse_events() do the same.

This fixes the following test failure:

  $ perf test "Parsing of all PMU events from sysfs"
  5.2: Parsing of all PMU events from sysfs                    : FAILED!

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/util/parse-events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 17c1c36a7bf9..50e88c9d3d46 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2217,12 +2217,12 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
 	evlist__splice_list_tail(evlist, &parse_state.list);
 
 	if (ret2 && warn_if_reordered && !parse_state.wild_card_pmus) {
+		evlist__uniquify_evsel_names(evlist, &stat_config);
 		pr_warning("WARNING: events were regrouped to match PMUs\n");
 
 		if (verbose > 0) {
 			struct strbuf sb = STRBUF_INIT;
 
-			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);

-- 
2.34.1
Re: [PATCH 1/2] perf tools: Always uniquify event names
Posted by Ian Rogers 2 weeks, 1 day ago
On Thu, Dec 4, 2025 at 1:11 AM James Clark <james.clark@linaro.org> wrote:
>
> evlist__uniquify_evsel_names() only gets called in __parse_events() if
> verbose is > 0. This means that the auto added "slots" events stay as
> "slots" rather than being expanded to "cpu_core/slots/" unless Perf is
> run in verbose mode. This is invisible to users when running Perf stat
> because evlist__print_counters() always calls it regardless of verbose
> mode before displaying.
>
> The only thing this seems to affect is the test "Parsing of all PMU
> events from sysfs" which fails when not run in verbose mode.
> test__checkevent_pmu_events() always expects event names to be prefixed
> with the pmu name, but this only happens for "slots" events after
> evlist__uniquify_evsel_names() is called.
>
> One fix could be to relax the test to accept the non prefixed name in
> normal mode. But seeing as Perf stat uniquifies unconditionally, make
> parse_events() do the same.
>
> This fixes the following test failure:
>
>   $ perf test "Parsing of all PMU events from sysfs"
>   5.2: Parsing of all PMU events from sysfs                    : FAILED!
>
> Signed-off-by: James Clark <james.clark@linaro.org>

Reviewed-by: Ian Rogers <irogers@google.com>

I wonder if we should just always uniquify event names. It could be a
thing that is changing as more and more PMUs are available. I'm
reminded that we used to uniquify sometimes with "<event> [<pmu>]"
prior to commit 057f8bfc6f70 ("perf stat: Uniquify event name
improvements").

Thanks,
Ian

> ---
>  tools/perf/util/parse-events.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 17c1c36a7bf9..50e88c9d3d46 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -2217,12 +2217,12 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
>         evlist__splice_list_tail(evlist, &parse_state.list);
>
>         if (ret2 && warn_if_reordered && !parse_state.wild_card_pmus) {
> +               evlist__uniquify_evsel_names(evlist, &stat_config);
>                 pr_warning("WARNING: events were regrouped to match PMUs\n");
>
>                 if (verbose > 0) {
>                         struct strbuf sb = STRBUF_INIT;
>
> -                       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);
>
> --
> 2.34.1
>