[PATCH v1 0/2] perf tests: Add tests for uncore and perf metric event sorting

[PATCH v1 0/2] perf tests: Add tests for uncore and perf metric event sorting

[Ian Rogers]

A thread changing event sorting highlighted a lack of testing for the
more complicated uncore and x86 perf metric event sorting:
https://lore.kernel.org/linux-perf-users/CAP-5=fWRgDo7UnJAD4C--d=mVPRhOEWZVyU7nVM1YEp3jncAgg@mail.gmail.com/

Ian Rogers (2):
  perf tests: Add test for uncore event sorting
  perf arch x86 tests: Add test for topdown event sorting

 tools/perf/arch/x86/tests/topdown.c     | 137 +++++++++++++++++++++++-
 tools/perf/tests/Build                  |   1 +
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/uncore-event-sorting.c | 125 +++++++++++++++++++++
 5 files changed, 264 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/tests/uncore-event-sorting.c

-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH v1 1/2] perf tests: Add test for uncore event sorting

[Ian Rogers]

Add a test for uncore event sorting matching multiple PMUs. Uncore
PMUs may have a common prefix, like the PMUs uncore_imc_free_running_0
and uncore_imc_free_running_1 have a prefix of
uncore_imc_free_running. Parsing an event group like
"{data_read,data_write}" for those PMUs should result with two groups
"{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_0/data_write/},
{uncore_imc_free_running_1/data_read/,uncore_imc_free_running_1/data_write/}"
which means the evsels need resorting as when initially parsed the
evsels are ordered with mixed PMUs:
"{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_1/data_read/,
uncore_imc_free_running_0/data_write/,uncore_imc_free_running_1/data_write/}".

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/Build                  |   1 +
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/uncore-event-sorting.c | 125 ++++++++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 tools/perf/tests/uncore-event-sorting.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index c2a67ce45941..66944a4f4968 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -3,6 +3,7 @@
 perf-test-y += builtin-test.o
 perf-test-y += tests-scripts.o
 perf-test-y += parse-events.o
+perf-test-y += uncore-event-sorting.o
 perf-test-y += dso-data.o
 perf-test-y += vmlinux-kallsyms.o
 perf-test-y += openat-syscall.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 06507066213b..f2c135891477 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
 	&suite__basic_mmap,
 	&suite__mem,
 	&suite__parse_events,
+	&suite__uncore_event_sorting,
 	&suite__expr,
 	&suite__PERF_RECORD,
 	&suite__pmu,
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index f5f1238d1f7f..ee00518bf36f 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -177,6 +177,7 @@ DECLARE_SUITE(sigtrap);
 DECLARE_SUITE(event_groups);
 DECLARE_SUITE(symbols);
 DECLARE_SUITE(util);
+DECLARE_SUITE(uncore_event_sorting);
 DECLARE_SUITE(subcmd_help);
 DECLARE_SUITE(kallsyms_split);
 
diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
new file mode 100644
index 000000000000..91e1a580709b
--- /dev/null
+++ b/tools/perf/tests/uncore-event-sorting.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "tests.h"
+#include "debug.h"
+#include "parse-events.h"
+#include "pmu.h"
+#include "pmus.h"
+#include "evlist.h"
+#include <string.h>
+
+struct match_state {
+	char *event1;
+	char *event2;
+};
+
+static int event_cb(void *state, struct pmu_event_info *info)
+{
+	struct match_state *m = state;
+
+	if (!m->event1) {
+		m->event1 = strdup(info->name);
+	} else if (!m->event2) {
+		if (strcmp(m->event1, info->name)) {
+			m->event2 = strdup(info->name);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
+				      int subtest __maybe_unused)
+{
+	struct evlist *evlist;
+	struct parse_events_error err;
+	struct evsel *evsel;
+	struct perf_pmu *pmu = NULL;
+	char *pmu_prefix = NULL;
+	struct match_state m = { NULL, NULL };
+	char buf[1024];
+	int ret;
+
+	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+		size_t len;
+		struct perf_pmu *sibling;
+
+		if (pmu->is_core)
+			continue;
+
+		len = pmu_name_len_no_suffix(pmu->name);
+		if (len == strlen(pmu->name))
+			continue;
+
+		sibling = pmu;
+		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
+			if (sibling->is_core)
+				continue;
+			if (pmu_name_len_no_suffix(sibling->name) == len &&
+			    !strncmp(pmu->name, sibling->name, len))
+				break;
+		}
+
+		if (!sibling)
+			continue;
+
+		m.event1 = m.event2 = NULL;
+		perf_pmu__for_each_event(pmu, false, &m, event_cb);
+
+		if (m.event1 && m.event2) {
+			pmu_prefix = strndup(pmu->name, len);
+			break;
+		}
+		free(m.event1);
+	}
+
+	if (!pmu_prefix) {
+		pr_debug("No suitable uncore PMU found\n");
+		return TEST_SKIP;
+	}
+
+	evlist = evlist__new();
+	if (!evlist)
+		return TEST_FAIL;
+
+	snprintf(buf, sizeof(buf), "{%s/%s/,%s/%s/}",
+		 pmu_prefix, m.event1, pmu_prefix, m.event2);
+	pr_debug("Parsing: %s\n", buf);
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, buf, &err);
+	if (ret) {
+		pr_debug("parse_events failed\n");
+		goto out_err;
+	}
+
+	TEST_ASSERT_VAL("Number of events is > 0", evlist->core.nr_entries > 0);
+	TEST_ASSERT_EQUAL("Number of events is a multiple of 2", evlist->core.nr_entries % 2, 0);
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (evsel__is_group_leader(evsel)) {
+			struct evsel *next = evsel__next(evsel);
+
+			TEST_ASSERT_EQUAL("Group size is 2", evsel->core.nr_members, 2);
+			TEST_ASSERT_VAL("PMU match", evsel->pmu == next->pmu);
+			TEST_ASSERT_VAL("First event name", strstr(evsel->name, m.event1) != NULL);
+			TEST_ASSERT_VAL("Second event name", strstr(next->name, m.event2) != NULL);
+		}
+	}
+
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	free(pmu_prefix);
+	free(m.event1);
+	free(m.event2);
+	return TEST_OK;
+
+out_err:
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	free(pmu_prefix);
+	free(m.event1);
+	free(m.event2);
+	return TEST_FAIL;
+}
+
+DEFINE_SUITE("Uncore event sorting", uncore_event_sorting);
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH v1 1/2] perf tests: Add test for uncore event sorting

[Chen, Zide]

On 3/25/2026 11:30 AM, Ian Rogers wrote:
> Add a test for uncore event sorting matching multiple PMUs. Uncore
> PMUs may have a common prefix, like the PMUs uncore_imc_free_running_0
> and uncore_imc_free_running_1 have a prefix of
> uncore_imc_free_running. Parsing an event group like
> "{data_read,data_write}" for those PMUs should result with two groups
> "{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_0/data_write/},
> {uncore_imc_free_running_1/data_read/,uncore_imc_free_running_1/data_write/}"
> which means the evsels need resorting as when initially parsed the
> evsels are ordered with mixed PMUs:
> "{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_1/data_read/,
> uncore_imc_free_running_0/data_write/,uncore_imc_free_running_1/data_write/}".
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---

Tested-by: Zide Chen <zide.chen@intel.com>

>  tools/perf/tests/Build                  |   1 +
>  tools/perf/tests/builtin-test.c         |   1 +
>  tools/perf/tests/tests.h                |   1 +
>  tools/perf/tests/uncore-event-sorting.c | 125 ++++++++++++++++++++++++
>  4 files changed, 128 insertions(+)
>  create mode 100644 tools/perf/tests/uncore-event-sorting.c
> 
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index c2a67ce45941..66944a4f4968 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -3,6 +3,7 @@
>  perf-test-y += builtin-test.o
>  perf-test-y += tests-scripts.o
>  perf-test-y += parse-events.o
> +perf-test-y += uncore-event-sorting.o
>  perf-test-y += dso-data.o
>  perf-test-y += vmlinux-kallsyms.o
>  perf-test-y += openat-syscall.o
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 06507066213b..f2c135891477 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
>  	&suite__basic_mmap,
>  	&suite__mem,
>  	&suite__parse_events,
> +	&suite__uncore_event_sorting,
>  	&suite__expr,
>  	&suite__PERF_RECORD,
>  	&suite__pmu,
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index f5f1238d1f7f..ee00518bf36f 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -177,6 +177,7 @@ DECLARE_SUITE(sigtrap);
>  DECLARE_SUITE(event_groups);
>  DECLARE_SUITE(symbols);
>  DECLARE_SUITE(util);
> +DECLARE_SUITE(uncore_event_sorting);
>  DECLARE_SUITE(subcmd_help);
>  DECLARE_SUITE(kallsyms_split);
>  
> diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
> new file mode 100644
> index 000000000000..91e1a580709b
> --- /dev/null
> +++ b/tools/perf/tests/uncore-event-sorting.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +#include "tests.h"
> +#include "debug.h"
> +#include "parse-events.h"
> +#include "pmu.h"
> +#include "pmus.h"
> +#include "evlist.h"
> +#include <string.h>
> +
> +struct match_state {
> +	char *event1;
> +	char *event2;
> +};
> +
> +static int event_cb(void *state, struct pmu_event_info *info)
> +{
> +	struct match_state *m = state;
> +
> +	if (!m->event1) {
> +		m->event1 = strdup(info->name);
> +	} else if (!m->event2) {
> +		if (strcmp(m->event1, info->name)) {
> +			m->event2 = strdup(info->name);
> +			return 1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
> +				      int subtest __maybe_unused)
> +{
> +	struct evlist *evlist;
> +	struct parse_events_error err;
> +	struct evsel *evsel;
> +	struct perf_pmu *pmu = NULL;
> +	char *pmu_prefix = NULL;
> +	struct match_state m = { NULL, NULL };
> +	char buf[1024];
> +	int ret;
> +
> +	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> +		size_t len;
> +		struct perf_pmu *sibling;
> +
> +		if (pmu->is_core)
> +			continue;
> +
> +		len = pmu_name_len_no_suffix(pmu->name);
> +		if (len == strlen(pmu->name))
> +			continue;
> +
> +		sibling = pmu;
> +		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
> +			if (sibling->is_core)
> +				continue;
> +			if (pmu_name_len_no_suffix(sibling->name) == len &&
> +			    !strncmp(pmu->name, sibling->name, len))
> +				break;
> +		}
> +
> +		if (!sibling)
> +			continue;
> +
> +		m.event1 = m.event2 = NULL;
> +		perf_pmu__for_each_event(pmu, false, &m, event_cb);
> +
> +		if (m.event1 && m.event2) {
> +			pmu_prefix = strndup(pmu->name, len);
> +			break;
> +		}
> +		free(m.event1);
> +	}
> +
> +	if (!pmu_prefix) {
> +		pr_debug("No suitable uncore PMU found\n");
> +		return TEST_SKIP;
> +	}
> +
> +	evlist = evlist__new();
> +	if (!evlist)
> +		return TEST_FAIL;
> +
> +	snprintf(buf, sizeof(buf), "{%s/%s/,%s/%s/}",
> +		 pmu_prefix, m.event1, pmu_prefix, m.event2);
> +	pr_debug("Parsing: %s\n", buf);
> +
> +	parse_events_error__init(&err);
> +	ret = parse_events(evlist, buf, &err);
> +	if (ret) {
> +		pr_debug("parse_events failed\n");
> +		goto out_err;
> +	}
> +
> +	TEST_ASSERT_VAL("Number of events is > 0", evlist->core.nr_entries > 0);
> +	TEST_ASSERT_EQUAL("Number of events is a multiple of 2", evlist->core.nr_entries % 2, 0);
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		if (evsel__is_group_leader(evsel)) {
> +			struct evsel *next = evsel__next(evsel);
> +
> +			TEST_ASSERT_EQUAL("Group size is 2", evsel->core.nr_members, 2);
> +			TEST_ASSERT_VAL("PMU match", evsel->pmu == next->pmu);
> +			TEST_ASSERT_VAL("First event name", strstr(evsel->name, m.event1) != NULL);
> +			TEST_ASSERT_VAL("Second event name", strstr(next->name, m.event2) != NULL);
> +		}
> +	}
> +
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	free(pmu_prefix);
> +	free(m.event1);
> +	free(m.event2);
> +	return TEST_OK;
> +
> +out_err:
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	free(pmu_prefix);
> +	free(m.event1);
> +	free(m.event2);
> +	return TEST_FAIL;
> +}
> +
> +DEFINE_SUITE("Uncore event sorting", uncore_event_sorting);

Re: [PATCH v1 1/2] perf tests: Add test for uncore event sorting

[Namhyung Kim]

Hi Ian,

Just a few nitpicks.

On Fri, Mar 27, 2026 at 04:36:47PM -0700, Chen, Zide wrote:
> 
> 
> On 3/25/2026 11:30 AM, Ian Rogers wrote:
> > Add a test for uncore event sorting matching multiple PMUs. Uncore
> > PMUs may have a common prefix, like the PMUs uncore_imc_free_running_0
> > and uncore_imc_free_running_1 have a prefix of
> > uncore_imc_free_running. Parsing an event group like
> > "{data_read,data_write}" for those PMUs should result with two groups
> > "{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_0/data_write/},
> > {uncore_imc_free_running_1/data_read/,uncore_imc_free_running_1/data_write/}"
> > which means the evsels need resorting as when initially parsed the
> > evsels are ordered with mixed PMUs:
> > "{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_1/data_read/,
> > uncore_imc_free_running_0/data_write/,uncore_imc_free_running_1/data_write/}".
> > 
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> 
> Tested-by: Zide Chen <zide.chen@intel.com>
> 
> >  tools/perf/tests/Build                  |   1 +
> >  tools/perf/tests/builtin-test.c         |   1 +
> >  tools/perf/tests/tests.h                |   1 +
> >  tools/perf/tests/uncore-event-sorting.c | 125 ++++++++++++++++++++++++
> >  4 files changed, 128 insertions(+)
> >  create mode 100644 tools/perf/tests/uncore-event-sorting.c
> > 
> > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> > index c2a67ce45941..66944a4f4968 100644
> > --- a/tools/perf/tests/Build
> > +++ b/tools/perf/tests/Build
> > @@ -3,6 +3,7 @@
> >  perf-test-y += builtin-test.o
> >  perf-test-y += tests-scripts.o
> >  perf-test-y += parse-events.o
> > +perf-test-y += uncore-event-sorting.o
> >  perf-test-y += dso-data.o
> >  perf-test-y += vmlinux-kallsyms.o
> >  perf-test-y += openat-syscall.o
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index 06507066213b..f2c135891477 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
> >  	&suite__basic_mmap,
> >  	&suite__mem,
> >  	&suite__parse_events,
> > +	&suite__uncore_event_sorting,
> >  	&suite__expr,
> >  	&suite__PERF_RECORD,
> >  	&suite__pmu,
> > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> > index f5f1238d1f7f..ee00518bf36f 100644
> > --- a/tools/perf/tests/tests.h
> > +++ b/tools/perf/tests/tests.h
> > @@ -177,6 +177,7 @@ DECLARE_SUITE(sigtrap);
> >  DECLARE_SUITE(event_groups);
> >  DECLARE_SUITE(symbols);
> >  DECLARE_SUITE(util);
> > +DECLARE_SUITE(uncore_event_sorting);
> >  DECLARE_SUITE(subcmd_help);
> >  DECLARE_SUITE(kallsyms_split);
> >  
> > diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
> > new file mode 100644
> > index 000000000000..91e1a580709b
> > --- /dev/null
> > +++ b/tools/perf/tests/uncore-event-sorting.c
> > @@ -0,0 +1,125 @@
> > +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> > +#include "tests.h"
> > +#include "debug.h"
> > +#include "parse-events.h"
> > +#include "pmu.h"
> > +#include "pmus.h"
> > +#include "evlist.h"
> > +#include <string.h>
> > +
> > +struct match_state {
> > +	char *event1;
> > +	char *event2;
> > +};
> > +
> > +static int event_cb(void *state, struct pmu_event_info *info)
> > +{
> > +	struct match_state *m = state;
> > +
> > +	if (!m->event1) {
> > +		m->event1 = strdup(info->name);
> > +	} else if (!m->event2) {
> > +		if (strcmp(m->event1, info->name)) {
> > +			m->event2 = strdup(info->name);
> > +			return 1;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
> > +				      int subtest __maybe_unused)
> > +{
> > +	struct evlist *evlist;
> > +	struct parse_events_error err;
> > +	struct evsel *evsel;
> > +	struct perf_pmu *pmu = NULL;
> > +	char *pmu_prefix = NULL;
> > +	struct match_state m = { NULL, NULL };
> > +	char buf[1024];
> > +	int ret;
> > +
> > +	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> > +		size_t len;
> > +		struct perf_pmu *sibling;
> > +
> > +		if (pmu->is_core)
> > +			continue;
> > +
> > +		len = pmu_name_len_no_suffix(pmu->name);
> > +		if (len == strlen(pmu->name))
> > +			continue;
> > +
> > +		sibling = pmu;
> > +		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
> > +			if (sibling->is_core)
> > +				continue;
> > +			if (pmu_name_len_no_suffix(sibling->name) == len &&
> > +			    !strncmp(pmu->name, sibling->name, len))
> > +				break;
> > +		}
> > +
> > +		if (!sibling)
> > +			continue;
> > +
> > +		m.event1 = m.event2 = NULL;
> > +		perf_pmu__for_each_event(pmu, false, &m, event_cb);
> > +
> > +		if (m.event1 && m.event2) {
> > +			pmu_prefix = strndup(pmu->name, len);
> > +			break;
> > +		}
> > +		free(m.event1);
> > +	}
> > +
> > +	if (!pmu_prefix) {
> > +		pr_debug("No suitable uncore PMU found\n");
> > +		return TEST_SKIP;
> > +	}
> > +
> > +	evlist = evlist__new();
> > +	if (!evlist)
> > +		return TEST_FAIL;
> > +
> > +	snprintf(buf, sizeof(buf), "{%s/%s/,%s/%s/}",
> > +		 pmu_prefix, m.event1, pmu_prefix, m.event2);
> > +	pr_debug("Parsing: %s\n", buf);
> > +
> > +	parse_events_error__init(&err);
> > +	ret = parse_events(evlist, buf, &err);
> > +	if (ret) {
> > +		pr_debug("parse_events failed\n");
> > +		goto out_err;
> > +	}
> > +
> > +	TEST_ASSERT_VAL("Number of events is > 0", evlist->core.nr_entries > 0);
> > +	TEST_ASSERT_EQUAL("Number of events is a multiple of 2", evlist->core.nr_entries % 2, 0);

It'd be nice if we can hide libperf details in general.  But I've
realized there's no API in libperf to return the number of entries
in evlist. :(

> > +
> > +	evlist__for_each_entry(evlist, evsel) {
> > +		if (evsel__is_group_leader(evsel)) {

You can skip the loop if it's not a leader and reduce a level of
indentation.


> > +			struct evsel *next = evsel__next(evsel);
> > +
> > +			TEST_ASSERT_EQUAL("Group size is 2", evsel->core.nr_members, 2);
> > +			TEST_ASSERT_VAL("PMU match", evsel->pmu == next->pmu);
> > +			TEST_ASSERT_VAL("First event name", strstr(evsel->name, m.event1) != NULL);
> > +			TEST_ASSERT_VAL("Second event name", strstr(next->name, m.event2) != NULL);
> > +		}
> > +	}
> > +
> > +	evlist__delete(evlist);
> > +	parse_events_error__exit(&err);
> > +	free(pmu_prefix);
> > +	free(m.event1);
> > +	free(m.event2);
> > +	return TEST_OK;
> > +
> > +out_err:
> > +	evlist__delete(evlist);
> > +	parse_events_error__exit(&err);
> > +	free(pmu_prefix);
> > +	free(m.event1);
> > +	free(m.event2);
> > +	return TEST_FAIL;

Looks like the same logic.  Can we set a return value properly?

Thanks,
Namhyung


> > +}
> > +
> > +DEFINE_SUITE("Uncore event sorting", uncore_event_sorting);
> 

[PATCH v1 2/2] perf arch x86 tests: Add test for topdown event sorting

[Ian Rogers]

Add a test to capture the comment in
tools/perf/arch/x86/util/evlist.c. Test that slots and
topdown-retiring get appropriately sorted with respect to instructions
when they're all specified together. When the PMU requires topdown
event grouping (indicated by the pressence of the slots event) metric
events should be after slots, which should be the group leader.

Add a related test that when the slots event isn't given it is
injected into the appropriate group.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/arch/x86/tests/topdown.c | 137 +++++++++++++++++++++++++++-
 1 file changed, 136 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
index 3ee4e5e71be3..aca7faa16fc7 100644
--- a/tools/perf/arch/x86/tests/topdown.c
+++ b/tools/perf/arch/x86/tests/topdown.c
@@ -75,4 +75,139 @@ static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest
 	return ret;
 }
 
-DEFINE_SUITE("x86 topdown", x86_topdown);
+static int test_sort(const char *str, int expected_slots_group_size,
+		     int expected_instructions_group_size)
+{
+	struct evlist *evlist;
+	struct parse_events_error err;
+	struct evsel *evsel;
+	int ret;
+
+	evlist = evlist__new();
+	if (!evlist)
+		return TEST_FAIL;
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, str, &err);
+	if (ret) {
+		pr_debug("parse_events failed for %s\n", str);
+		goto out_err;
+	}
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (evsel__is_group_leader(evsel)) {
+			if (strstr(evsel->name, "slots")) {
+				/*
+				 * Slots as a leader means the PMU is for a perf
+				 * metric group as the slots event isn't present
+				 * when not.
+				 */
+				TEST_ASSERT_EQUAL("slots group size", evsel->core.nr_members,
+						  expected_slots_group_size);
+				if (expected_slots_group_size == 3) {
+					struct evsel *next = evsel__next(evsel);
+					struct evsel *next2 = evsel__next(next);
+
+					TEST_ASSERT_VAL("slots second event is instructions",
+							strstr(next->name, "instructions")
+							!= NULL);
+					TEST_ASSERT_VAL("slots third event is topdown-retiring",
+							strstr(next2->name, "topdown-retiring")
+							!= NULL);
+				} else if (expected_slots_group_size == 2) {
+					struct evsel *next = evsel__next(evsel);
+
+					TEST_ASSERT_VAL("slots second event is topdown-retiring",
+							strstr(next->name, "topdown-retiring")
+							!= NULL);
+				}
+			} else if (strstr(evsel->name, "instructions")) {
+				TEST_ASSERT_EQUAL("instructions group size", evsel->core.nr_members,
+						  expected_instructions_group_size);
+				if (expected_instructions_group_size == 2) {
+					/*
+					 * The instructions event leads a group
+					 * with a topdown-retiring event,
+					 * neither of which need reordering for
+					 * perf metric event support.
+					 */
+					struct evsel *next = evsel__next(evsel);
+
+					TEST_ASSERT_VAL("instructions second event is topdown-retiring",
+							strstr(next->name, "topdown-retiring")
+							!= NULL);
+				}
+			} else if (strstr(evsel->name, "topdown-retiring")) {
+				/*
+				 * A perf metric event where the PMU doesn't
+				 * require slots as a leader.
+				 */
+				TEST_ASSERT_EQUAL("topdown-retiring group size",
+						  evsel->core.nr_members, 1);
+			} else if (strstr(evsel->name, "cycles")) {
+				TEST_ASSERT_EQUAL("cycles group size", evsel->core.nr_members, 1);
+			}
+		}
+	}
+
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	return TEST_OK;
+
+out_err:
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	return TEST_FAIL;
+}
+
+static int test__x86_topdown_sorting(struct test_suite *test __maybe_unused,
+				     int subtest __maybe_unused)
+{
+	if (!topdown_sys_has_perf_metrics())
+		return TEST_OK;
+
+	TEST_ASSERT_EQUAL("all events in a group",
+			  test_sort("{instructions,topdown-retiring,slots}", 3, 2), TEST_OK);
+	TEST_ASSERT_EQUAL("all events not in a group",
+			  test_sort("instructions,topdown-retiring,slots", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event in a group but topdown metrics events outside the group",
+			  test_sort("{instructions,slots},topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event and topdown metrics events in two groups",
+			  test_sort("{instructions,slots},{topdown-retiring}", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event and metrics event are not in a group and not adjacent",
+			  test_sort("{instructions,slots},cycles,topdown-retiring", 2, 1), TEST_OK);
+
+	return TEST_OK;
+}
+
+static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unused,
+					     int subtest __maybe_unused)
+{
+	if (!topdown_sys_has_perf_metrics())
+		return TEST_OK;
+
+	TEST_ASSERT_EQUAL("all events in a group",
+			  test_sort("{instructions,topdown-retiring}", 3, 2), TEST_OK);
+	TEST_ASSERT_EQUAL("all events not in a group",
+			  test_sort("instructions,topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event in a group but topdown metrics events outside the group",
+			  test_sort("{instructions},topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event and topdown metrics events in two groups",
+			  test_sort("{instructions},{topdown-retiring}", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event and metrics event are not in a group and not adjacent",
+			  test_sort("{instructions},cycles,topdown-retiring", 2, 1), TEST_OK);
+
+	return TEST_OK;
+}
+
+static struct test_case x86_topdown_tests[] = {
+	TEST_CASE("topdown events", x86_topdown),
+	TEST_CASE("topdown sorting", x86_topdown_sorting),
+	TEST_CASE("topdown slots injection", x86_topdown_slots_injection),
+	{ .name = NULL, }
+};
+
+struct test_suite suite__x86_topdown = {
+	.desc = "x86 topdown",
+	.test_cases = x86_topdown_tests,
+};
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH v1 2/2] perf arch x86 tests: Add test for topdown event sorting

[Chen, Zide]

On 3/25/2026 11:30 AM, Ian Rogers wrote:
> Add a test to capture the comment in
> tools/perf/arch/x86/util/evlist.c. Test that slots and
> topdown-retiring get appropriately sorted with respect to instructions
> when they're all specified together. When the PMU requires topdown
> event grouping (indicated by the pressence of the slots event) metric
> events should be after slots, which should be the group leader.
> 
> Add a related test that when the slots event isn't given it is
> injected into the appropriate group.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/arch/x86/tests/topdown.c | 137 +++++++++++++++++++++++++++-
>  1 file changed, 136 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> index 3ee4e5e71be3..aca7faa16fc7 100644
> --- a/tools/perf/arch/x86/tests/topdown.c
> +++ b/tools/perf/arch/x86/tests/topdown.c
> @@ -75,4 +75,139 @@ static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest
>  	return ret;
>  }
>  
> -DEFINE_SUITE("x86 topdown", x86_topdown);
> +static int test_sort(const char *str, int expected_slots_group_size,
> +		     int expected_instructions_group_size)
> +{
> +	struct evlist *evlist;
> +	struct parse_events_error err;
> +	struct evsel *evsel;
> +	int ret;
> +
> +	evlist = evlist__new();
> +	if (!evlist)
> +		return TEST_FAIL;
> +
> +	parse_events_error__init(&err);
> +	ret = parse_events(evlist, str, &err);
> +	if (ret) {
> +		pr_debug("parse_events failed for %s\n", str);
> +		goto out_err;
> +	}
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		if (evsel__is_group_leader(evsel)) {
> +			if (strstr(evsel->name, "slots")) {
> +				/*
> +				 * Slots as a leader means the PMU is for a perf
> +				 * metric group as the slots event isn't present
> +				 * when not.
> +				 */
> +				TEST_ASSERT_EQUAL("slots group size", evsel->core.nr_members,
> +						  expected_slots_group_size);
> +				if (expected_slots_group_size == 3) {
> +					struct evsel *next = evsel__next(evsel);
> +					struct evsel *next2 = evsel__next(next);
> +
> +					TEST_ASSERT_VAL("slots second event is instructions",
> +							strstr(next->name, "instructions")
> +							!= NULL);
> +					TEST_ASSERT_VAL("slots third event is topdown-retiring",
> +							strstr(next2->name, "topdown-retiring")
> +							!= NULL);
> +				} else if (expected_slots_group_size == 2) {
> +					struct evsel *next = evsel__next(evsel);
> +
> +					TEST_ASSERT_VAL("slots second event is topdown-retiring",
> +							strstr(next->name, "topdown-retiring")
> +							!= NULL);
> +				}
> +			} else if (strstr(evsel->name, "instructions")) {
> +				TEST_ASSERT_EQUAL("instructions group size", evsel->core.nr_members,
> +						  expected_instructions_group_size);
> +				if (expected_instructions_group_size == 2) {
> +					/*
> +					 * The instructions event leads a group
> +					 * with a topdown-retiring event,
> +					 * neither of which need reordering for
> +					 * perf metric event support.
> +					 */
> +					struct evsel *next = evsel__next(evsel);
> +
> +					TEST_ASSERT_VAL("instructions second event is topdown-retiring",
> +							strstr(next->name, "topdown-retiring")
> +							!= NULL);
> +				}
> +			} else if (strstr(evsel->name, "topdown-retiring")) {
> +				/*
> +				 * A perf metric event where the PMU doesn't
> +				 * require slots as a leader.
> +				 */
> +				TEST_ASSERT_EQUAL("topdown-retiring group size",
> +						  evsel->core.nr_members, 1);
> +			} else if (strstr(evsel->name, "cycles")) {
> +				TEST_ASSERT_EQUAL("cycles group size", evsel->core.nr_members, 1);
> +			}
> +		}
> +	}
> +
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	return TEST_OK;
> +
> +out_err:
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	return TEST_FAIL;
> +}
> +
> +static int test__x86_topdown_sorting(struct test_suite *test __maybe_unused,
> +				     int subtest __maybe_unused)
> +{
> +	if (!topdown_sys_has_perf_metrics())
> +		return TEST_OK;

I'm wondering if it makes more sense to return TEST_SKIP? As well as for
other calls to topdown_sys_has_perf_metrics().

Other than that, Tested-by: Zide Chen <zide.chen@intel.com>



> +	TEST_ASSERT_EQUAL("all events in a group",
> +			  test_sort("{instructions,topdown-retiring,slots}", 3, 2), TEST_OK);
> +	TEST_ASSERT_EQUAL("all events not in a group",
> +			  test_sort("instructions,topdown-retiring,slots", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("slots event in a group but topdown metrics events outside the group",
> +			  test_sort("{instructions,slots},topdown-retiring", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("slots event and topdown metrics events in two groups",
> +			  test_sort("{instructions,slots},{topdown-retiring}", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("slots event and metrics event are not in a group and not adjacent",
> +			  test_sort("{instructions,slots},cycles,topdown-retiring", 2, 1), TEST_OK);
> +
> +	return TEST_OK;
> +}
> +
> +static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unused,
> +					     int subtest __maybe_unused)
> +{
> +	if (!topdown_sys_has_perf_metrics())
> +		return TEST_OK;
> +
> +	TEST_ASSERT_EQUAL("all events in a group",
> +			  test_sort("{instructions,topdown-retiring}", 3, 2), TEST_OK);
> +	TEST_ASSERT_EQUAL("all events not in a group",
> +			  test_sort("instructions,topdown-retiring", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("event in a group but topdown metrics events outside the group",
> +			  test_sort("{instructions},topdown-retiring", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("event and topdown metrics events in two groups",
> +			  test_sort("{instructions},{topdown-retiring}", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("event and metrics event are not in a group and not adjacent",
> +			  test_sort("{instructions},cycles,topdown-retiring", 2, 1), TEST_OK);
> +
> +	return TEST_OK;
> +}
> +
> +static struct test_case x86_topdown_tests[] = {
> +	TEST_CASE("topdown events", x86_topdown),
> +	TEST_CASE("topdown sorting", x86_topdown_sorting),
> +	TEST_CASE("topdown slots injection", x86_topdown_slots_injection),
> +	{ .name = NULL, }
> +};
> +
> +struct test_suite suite__x86_topdown = {
> +	.desc = "x86 topdown",
> +	.test_cases = x86_topdown_tests,
> +};

Re: [PATCH v1 2/2] perf arch x86 tests: Add test for topdown event sorting

[Namhyung Kim]

On Mon, Mar 30, 2026 at 02:53:45PM -0700, Chen, Zide wrote:
> 
> 
> On 3/25/2026 11:30 AM, Ian Rogers wrote:
> > Add a test to capture the comment in
> > tools/perf/arch/x86/util/evlist.c. Test that slots and
> > topdown-retiring get appropriately sorted with respect to instructions
> > when they're all specified together. When the PMU requires topdown
> > event grouping (indicated by the pressence of the slots event) metric
> > events should be after slots, which should be the group leader.
> > 
> > Add a related test that when the slots event isn't given it is
> > injected into the appropriate group.
> > 
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/arch/x86/tests/topdown.c | 137 +++++++++++++++++++++++++++-
> >  1 file changed, 136 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> > index 3ee4e5e71be3..aca7faa16fc7 100644
> > --- a/tools/perf/arch/x86/tests/topdown.c
> > +++ b/tools/perf/arch/x86/tests/topdown.c
> > @@ -75,4 +75,139 @@ static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest
> >  	return ret;
> >  }
> >  
> > -DEFINE_SUITE("x86 topdown", x86_topdown);
> > +static int test_sort(const char *str, int expected_slots_group_size,
> > +		     int expected_instructions_group_size)
> > +{
> > +	struct evlist *evlist;
> > +	struct parse_events_error err;
> > +	struct evsel *evsel;
> > +	int ret;
> > +
> > +	evlist = evlist__new();
> > +	if (!evlist)
> > +		return TEST_FAIL;
> > +
> > +	parse_events_error__init(&err);
> > +	ret = parse_events(evlist, str, &err);
> > +	if (ret) {
> > +		pr_debug("parse_events failed for %s\n", str);
> > +		goto out_err;
> > +	}
> > +
> > +	evlist__for_each_entry(evlist, evsel) {
> > +		if (evsel__is_group_leader(evsel)) {

Same thing.  Please reduce the indentation.


> > +			if (strstr(evsel->name, "slots")) {
> > +				/*
> > +				 * Slots as a leader means the PMU is for a perf
> > +				 * metric group as the slots event isn't present
> > +				 * when not.
> > +				 */
> > +				TEST_ASSERT_EQUAL("slots group size", evsel->core.nr_members,
> > +						  expected_slots_group_size);
> > +				if (expected_slots_group_size == 3) {
> > +					struct evsel *next = evsel__next(evsel);
> > +					struct evsel *next2 = evsel__next(next);
> > +
> > +					TEST_ASSERT_VAL("slots second event is instructions",
> > +							strstr(next->name, "instructions")
> > +							!= NULL);
> > +					TEST_ASSERT_VAL("slots third event is topdown-retiring",
> > +							strstr(next2->name, "topdown-retiring")
> > +							!= NULL);
> > +				} else if (expected_slots_group_size == 2) {
> > +					struct evsel *next = evsel__next(evsel);
> > +
> > +					TEST_ASSERT_VAL("slots second event is topdown-retiring",
> > +							strstr(next->name, "topdown-retiring")
> > +							!= NULL);
> > +				}
> > +			} else if (strstr(evsel->name, "instructions")) {
> > +				TEST_ASSERT_EQUAL("instructions group size", evsel->core.nr_members,
> > +						  expected_instructions_group_size);
> > +				if (expected_instructions_group_size == 2) {
> > +					/*
> > +					 * The instructions event leads a group
> > +					 * with a topdown-retiring event,
> > +					 * neither of which need reordering for
> > +					 * perf metric event support.
> > +					 */
> > +					struct evsel *next = evsel__next(evsel);
> > +
> > +					TEST_ASSERT_VAL("instructions second event is topdown-retiring",
> > +							strstr(next->name, "topdown-retiring")
> > +							!= NULL);
> > +				}
> > +			} else if (strstr(evsel->name, "topdown-retiring")) {
> > +				/*
> > +				 * A perf metric event where the PMU doesn't
> > +				 * require slots as a leader.
> > +				 */
> > +				TEST_ASSERT_EQUAL("topdown-retiring group size",
> > +						  evsel->core.nr_members, 1);
> > +			} else if (strstr(evsel->name, "cycles")) {
> > +				TEST_ASSERT_EQUAL("cycles group size", evsel->core.nr_members, 1);
> > +			}
> > +		}
> > +	}
> > +
> > +	evlist__delete(evlist);
> > +	parse_events_error__exit(&err);
> > +	return TEST_OK;
> > +
> > +out_err:
> > +	evlist__delete(evlist);
> > +	parse_events_error__exit(&err);
> > +	return TEST_FAIL;

Also, please share the code and use a different return value.

> > +}
> > +
> > +static int test__x86_topdown_sorting(struct test_suite *test __maybe_unused,
> > +				     int subtest __maybe_unused)
> > +{
> > +	if (!topdown_sys_has_perf_metrics())
> > +		return TEST_OK;
> 
> I'm wondering if it makes more sense to return TEST_SKIP? As well as for
> other calls to topdown_sys_has_perf_metrics().

Makes sense.

Thanks,
Namhyung

> 
> Other than that, Tested-by: Zide Chen <zide.chen@intel.com>
> 
> 
> 
> > +	TEST_ASSERT_EQUAL("all events in a group",
> > +			  test_sort("{instructions,topdown-retiring,slots}", 3, 2), TEST_OK);
> > +	TEST_ASSERT_EQUAL("all events not in a group",
> > +			  test_sort("instructions,topdown-retiring,slots", 2, 1), TEST_OK);
> > +	TEST_ASSERT_EQUAL("slots event in a group but topdown metrics events outside the group",
> > +			  test_sort("{instructions,slots},topdown-retiring", 2, 1), TEST_OK);
> > +	TEST_ASSERT_EQUAL("slots event and topdown metrics events in two groups",
> > +			  test_sort("{instructions,slots},{topdown-retiring}", 2, 1), TEST_OK);
> > +	TEST_ASSERT_EQUAL("slots event and metrics event are not in a group and not adjacent",
> > +			  test_sort("{instructions,slots},cycles,topdown-retiring", 2, 1), TEST_OK);
> > +
> > +	return TEST_OK;
> > +}
> > +
> > +static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unused,
> > +					     int subtest __maybe_unused)
> > +{
> > +	if (!topdown_sys_has_perf_metrics())
> > +		return TEST_OK;
> > +
> > +	TEST_ASSERT_EQUAL("all events in a group",
> > +			  test_sort("{instructions,topdown-retiring}", 3, 2), TEST_OK);
> > +	TEST_ASSERT_EQUAL("all events not in a group",
> > +			  test_sort("instructions,topdown-retiring", 2, 1), TEST_OK);
> > +	TEST_ASSERT_EQUAL("event in a group but topdown metrics events outside the group",
> > +			  test_sort("{instructions},topdown-retiring", 2, 1), TEST_OK);
> > +	TEST_ASSERT_EQUAL("event and topdown metrics events in two groups",
> > +			  test_sort("{instructions},{topdown-retiring}", 2, 1), TEST_OK);
> > +	TEST_ASSERT_EQUAL("event and metrics event are not in a group and not adjacent",
> > +			  test_sort("{instructions},cycles,topdown-retiring", 2, 1), TEST_OK);
> > +
> > +	return TEST_OK;
> > +}
> > +
> > +static struct test_case x86_topdown_tests[] = {
> > +	TEST_CASE("topdown events", x86_topdown),
> > +	TEST_CASE("topdown sorting", x86_topdown_sorting),
> > +	TEST_CASE("topdown slots injection", x86_topdown_slots_injection),
> > +	{ .name = NULL, }
> > +};
> > +
> > +struct test_suite suite__x86_topdown = {
> > +	.desc = "x86 topdown",
> > +	.test_cases = x86_topdown_tests,
> > +};
> 

[PATCH v2 0/2] perf tests: Add tests for uncore and perf metric event sorting

[Ian Rogers]

A thread changing event sorting highlighted a lack of testing for the
more complicated uncore and x86 perf metric event sorting:
https://lore.kernel.org/linux-perf-users/CAP-5=fWRgDo7UnJAD4C--d=mVPRhOEWZVyU7nVM1YEp3jncAgg@mail.gmail.com/

v2: Address indentation and other nits from Namhyung. Add Zide Chen's
    tested-by tags.

v1: https://lore.kernel.org/lkml/20260325183045.1229502-1-irogers@google.com/

Ian Rogers (2):
  perf tests: Add test for uncore event sorting
  perf arch x86 tests: Add test for topdown event sorting

 tools/perf/arch/x86/tests/topdown.c     | 129 +++++++++++++++++++++++-
 tools/perf/tests/Build                  |   1 +
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/uncore-event-sorting.c | 122 ++++++++++++++++++++++
 5 files changed, 253 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/tests/uncore-event-sorting.c

-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH v2 1/2] perf tests: Add test for uncore event sorting

[Ian Rogers]

Add a test for uncore event sorting matching multiple PMUs. Uncore
PMUs may have a common prefix, like the PMUs uncore_imc_free_running_0
and uncore_imc_free_running_1 have a prefix of
uncore_imc_free_running. Parsing an event group like
"{data_read,data_write}" for those PMUs should result with two groups
"{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_0/data_write/},
{uncore_imc_free_running_1/data_read/,uncore_imc_free_running_1/data_write/}"
which means the evsels need resorting as when initially parsed the
evsels are ordered with mixed PMUs:
"{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_1/data_read/,
uncore_imc_free_running_0/data_write/,uncore_imc_free_running_1/data_write/}".

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Zide Chen <zide.chen@intel.com>
---
 tools/perf/tests/Build                  |   1 +
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/uncore-event-sorting.c | 122 ++++++++++++++++++++++++
 4 files changed, 125 insertions(+)
 create mode 100644 tools/perf/tests/uncore-event-sorting.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index c2a67ce45941..66944a4f4968 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -3,6 +3,7 @@
 perf-test-y += builtin-test.o
 perf-test-y += tests-scripts.o
 perf-test-y += parse-events.o
+perf-test-y += uncore-event-sorting.o
 perf-test-y += dso-data.o
 perf-test-y += vmlinux-kallsyms.o
 perf-test-y += openat-syscall.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 06507066213b..f2c135891477 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
 	&suite__basic_mmap,
 	&suite__mem,
 	&suite__parse_events,
+	&suite__uncore_event_sorting,
 	&suite__expr,
 	&suite__PERF_RECORD,
 	&suite__pmu,
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index f5f1238d1f7f..ee00518bf36f 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -177,6 +177,7 @@ DECLARE_SUITE(sigtrap);
 DECLARE_SUITE(event_groups);
 DECLARE_SUITE(symbols);
 DECLARE_SUITE(util);
+DECLARE_SUITE(uncore_event_sorting);
 DECLARE_SUITE(subcmd_help);
 DECLARE_SUITE(kallsyms_split);
 
diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
new file mode 100644
index 000000000000..7e61e46283d5
--- /dev/null
+++ b/tools/perf/tests/uncore-event-sorting.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "tests.h"
+#include "debug.h"
+#include "parse-events.h"
+#include "pmu.h"
+#include "pmus.h"
+#include "evlist.h"
+#include <string.h>
+
+struct match_state {
+	char *event1;
+	char *event2;
+};
+
+static int event_cb(void *state, struct pmu_event_info *info)
+{
+	struct match_state *m = state;
+
+	if (!m->event1) {
+		m->event1 = strdup(info->name);
+	} else if (!m->event2) {
+		if (strcmp(m->event1, info->name)) {
+			m->event2 = strdup(info->name);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
+				      int subtest __maybe_unused)
+{
+	struct evlist *evlist;
+	struct parse_events_error err;
+	struct evsel *evsel;
+	struct perf_pmu *pmu = NULL;
+	char *pmu_prefix = NULL;
+	struct match_state m = { NULL, NULL };
+	char buf[1024];
+	int ret;
+
+	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+		size_t len;
+		struct perf_pmu *sibling;
+
+		if (pmu->is_core)
+			continue;
+
+		len = pmu_name_len_no_suffix(pmu->name);
+		if (len == strlen(pmu->name))
+			continue;
+
+		sibling = pmu;
+		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
+			if (sibling->is_core)
+				continue;
+			if (pmu_name_len_no_suffix(sibling->name) == len &&
+			    !strncmp(pmu->name, sibling->name, len))
+				break;
+		}
+
+		if (!sibling)
+			continue;
+
+		m.event1 = m.event2 = NULL;
+		perf_pmu__for_each_event(pmu, false, &m, event_cb);
+
+		if (m.event1 && m.event2) {
+			pmu_prefix = strndup(pmu->name, len);
+			break;
+		}
+		free(m.event1);
+	}
+
+	if (!pmu_prefix) {
+		pr_debug("No suitable uncore PMU found\n");
+		return TEST_SKIP;
+	}
+
+	evlist = evlist__new();
+	if (!evlist)
+		return TEST_FAIL;
+
+	snprintf(buf, sizeof(buf), "{%s/%s/,%s/%s/}",
+		 pmu_prefix, m.event1, pmu_prefix, m.event2);
+	pr_debug("Parsing: %s\n", buf);
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, buf, &err);
+	if (ret) {
+		pr_debug("parse_events failed\n");
+		ret = TEST_FAIL;
+		goto out_err;
+	}
+
+	TEST_ASSERT_VAL("Number of events is > 0", evlist->core.nr_entries > 0);
+	TEST_ASSERT_EQUAL("Number of events is a multiple of 2", evlist->core.nr_entries % 2, 0);
+
+	evlist__for_each_entry(evlist, evsel) {
+		struct evsel *next;
+
+		if (!evsel__is_group_leader(evsel))
+			continue;
+
+		next = evsel__next(evsel);
+		TEST_ASSERT_EQUAL("Group size is 2", evsel->core.nr_members, 2);
+		TEST_ASSERT_VAL("PMU match", evsel->pmu == next->pmu);
+		TEST_ASSERT_VAL("First event name", strstr(evsel__name(evsel), m.event1) != NULL);
+		TEST_ASSERT_VAL("Second event name", strstr(evsel__name(next), m.event2) != NULL);
+	}
+	ret = TEST_OK;
+
+out_err:
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	free(pmu_prefix);
+	free(m.event1);
+	free(m.event2);
+	return ret;
+}
+
+DEFINE_SUITE("Uncore event sorting", uncore_event_sorting);
-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH v2 2/2] perf arch x86 tests: Add test for topdown event sorting

[Ian Rogers]

Add a test to capture the comment in
tools/perf/arch/x86/util/evlist.c. Test that slots and
topdown-retiring get appropriately sorted with respect to instructions
when they're all specified together. When the PMU requires topdown
event grouping (indicated by the pressence of the slots event) metric
events should be after slots, which should be the group leader.

Add a related test that when the slots event isn't given it is
injected into the appropriate group.

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Zide Chen <zide.chen@intel.com>
---
 tools/perf/arch/x86/tests/topdown.c | 129 +++++++++++++++++++++++++++-
 1 file changed, 128 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
index 3ee4e5e71be3..b1a9fa28bdfd 100644
--- a/tools/perf/arch/x86/tests/topdown.c
+++ b/tools/perf/arch/x86/tests/topdown.c
@@ -75,4 +75,131 @@ static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest
 	return ret;
 }
 
-DEFINE_SUITE("x86 topdown", x86_topdown);
+static int test_sort(const char *str, int expected_slots_group_size,
+		     int expected_instructions_group_size)
+{
+	struct evlist *evlist;
+	struct parse_events_error err;
+	struct evsel *evsel;
+	int ret = TEST_FAIL;
+
+	evlist = evlist__new();
+	if (!evlist)
+		return TEST_FAIL;
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, str, &err);
+	if (ret) {
+		pr_debug("parse_events failed for %s\n", str);
+		goto out_err;
+	}
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (!evsel__is_group_leader(evsel))
+			continue;
+
+		if (strstr(evsel->name, "slots")) {
+			/*
+			 * Slots as a leader means the PMU is for a perf metric
+			 * group as the slots event isn't present when not.
+			 */
+			TEST_ASSERT_EQUAL("slots group size", evsel->core.nr_members,
+					expected_slots_group_size);
+			if (expected_slots_group_size == 3) {
+				struct evsel *next = evsel__next(evsel);
+				struct evsel *next2 = evsel__next(next);
+
+				TEST_ASSERT_VAL("slots second event is instructions",
+						strstr(next->name, "instructions")
+						!= NULL);
+				TEST_ASSERT_VAL("slots third event is topdown-retiring",
+						strstr(next2->name, "topdown-retiring") != NULL);
+			} else if (expected_slots_group_size == 2) {
+				struct evsel *next = evsel__next(evsel);
+
+				TEST_ASSERT_VAL("slots second event is topdown-retiring",
+						strstr(next->name, "topdown-retiring") != NULL);
+			}
+		} else if (strstr(evsel->name, "instructions")) {
+			TEST_ASSERT_EQUAL("instructions group size", evsel->core.nr_members,
+					  expected_instructions_group_size);
+			if (expected_instructions_group_size == 2) {
+				/*
+				 * The instructions event leads a group with a
+				 * topdown-retiring event, neither of which need
+				 * reordering for perf metric event support.
+				 */
+				struct evsel *next = evsel__next(evsel);
+
+				TEST_ASSERT_VAL("instructions second event is topdown-retiring",
+						strstr(next->name, "topdown-retiring") != NULL);
+			}
+		} else if (strstr(evsel->name, "topdown-retiring")) {
+			/*
+			 * A perf metric event where the PMU doesn't require
+			 * slots as a leader.
+			 */
+			TEST_ASSERT_EQUAL("topdown-retiring group size", evsel->core.nr_members, 1);
+		} else if (strstr(evsel->name, "cycles")) {
+			TEST_ASSERT_EQUAL("cycles group size", evsel->core.nr_members, 1);
+		}
+	}
+
+	ret = TEST_OK;
+out_err:
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	return ret;
+}
+
+static int test__x86_topdown_sorting(struct test_suite *test __maybe_unused,
+				     int subtest __maybe_unused)
+{
+	if (!topdown_sys_has_perf_metrics())
+		return TEST_OK;
+
+	TEST_ASSERT_EQUAL("all events in a group",
+			  test_sort("{instructions,topdown-retiring,slots}", 3, 2), TEST_OK);
+	TEST_ASSERT_EQUAL("all events not in a group",
+			  test_sort("instructions,topdown-retiring,slots", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event in a group but topdown metrics events outside the group",
+			  test_sort("{instructions,slots},topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event and topdown metrics events in two groups",
+			  test_sort("{instructions,slots},{topdown-retiring}", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event and metrics event are not in a group and not adjacent",
+			  test_sort("{instructions,slots},cycles,topdown-retiring", 2, 1), TEST_OK);
+
+	return TEST_OK;
+}
+
+static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unused,
+					     int subtest __maybe_unused)
+{
+	if (!topdown_sys_has_perf_metrics())
+		return TEST_OK;
+
+	TEST_ASSERT_EQUAL("all events in a group",
+			  test_sort("{instructions,topdown-retiring}", 3, 2), TEST_OK);
+	TEST_ASSERT_EQUAL("all events not in a group",
+			  test_sort("instructions,topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event in a group but topdown metrics events outside the group",
+			  test_sort("{instructions},topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event and topdown metrics events in two groups",
+			  test_sort("{instructions},{topdown-retiring}", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event and metrics event are not in a group and not adjacent",
+			  test_sort("{instructions},cycles,topdown-retiring", 2, 1), TEST_OK);
+
+	return TEST_OK;
+}
+
+static struct test_case x86_topdown_tests[] = {
+	TEST_CASE("topdown events", x86_topdown),
+	TEST_CASE("topdown sorting", x86_topdown_sorting),
+	TEST_CASE("topdown slots injection", x86_topdown_slots_injection),
+	{ .name = NULL, }
+};
+
+struct test_suite suite__x86_topdown = {
+	.desc = "x86 topdown",
+	.test_cases = x86_topdown_tests,
+};
-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH v3 0/2] Add tests for uncore and perf metric event sorting

[Ian Rogers]

A thread changing event sorting highlighted a lack of testing for the
more complicated uncore and x86 perf metric event sorting:
https://lore.kernel.org/linux-perf-users/CAP-5=fWRgDo7UnJAD4C--d=mVPRhOEWZVyU7nVM1YEp3jncAgg@mail.gmail.com/

v3: Address sashiko nits on using evsel__name and ensuring slots is injected.

v2: Address indentation and other nits from Namhyung. Add Zide Chen's
    tested-by tags.
    https://lore.kernel.org/lkml/20260331165207.4016392-1-irogers@google.com/

v1: https://lore.kernel.org/lkml/20260325183045.1229502-1-irogers@google.com/

Ian Rogers (2):
  perf tests: Add test for uncore event sorting
  perf arch x86 tests: Add test for topdown event sorting

 tools/perf/arch/x86/tests/topdown.c     | 134 +++++++++++++++++++++++-
 tools/perf/tests/Build                  |   1 +
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/uncore-event-sorting.c | 122 +++++++++++++++++++++
 5 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/tests/uncore-event-sorting.c

-- 
2.53.0.1118.gaef5881109-goog

[PATCH v3 1/2] perf tests: Add test for uncore event sorting

[Ian Rogers]

Add a test for uncore event sorting matching multiple PMUs. Uncore
PMUs may have a common prefix, like the PMUs uncore_imc_free_running_0
and uncore_imc_free_running_1 have a prefix of
uncore_imc_free_running. Parsing an event group like
"{data_read,data_write}" for those PMUs should result with two groups
"{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_0/data_write/},
{uncore_imc_free_running_1/data_read/,uncore_imc_free_running_1/data_write/}"
which means the evsels need resorting as when initially parsed the
evsels are ordered with mixed PMUs:
"{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_1/data_read/,
uncore_imc_free_running_0/data_write/,uncore_imc_free_running_1/data_write/}".

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Zide Chen <zide.chen@intel.com>
---
 tools/perf/tests/Build                  |   1 +
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/uncore-event-sorting.c | 122 ++++++++++++++++++++++++
 4 files changed, 125 insertions(+)
 create mode 100644 tools/perf/tests/uncore-event-sorting.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index c2a67ce45941..66944a4f4968 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -3,6 +3,7 @@
 perf-test-y += builtin-test.o
 perf-test-y += tests-scripts.o
 perf-test-y += parse-events.o
+perf-test-y += uncore-event-sorting.o
 perf-test-y += dso-data.o
 perf-test-y += vmlinux-kallsyms.o
 perf-test-y += openat-syscall.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 06507066213b..f2c135891477 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
 	&suite__basic_mmap,
 	&suite__mem,
 	&suite__parse_events,
+	&suite__uncore_event_sorting,
 	&suite__expr,
 	&suite__PERF_RECORD,
 	&suite__pmu,
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index f5f1238d1f7f..ee00518bf36f 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -177,6 +177,7 @@ DECLARE_SUITE(sigtrap);
 DECLARE_SUITE(event_groups);
 DECLARE_SUITE(symbols);
 DECLARE_SUITE(util);
+DECLARE_SUITE(uncore_event_sorting);
 DECLARE_SUITE(subcmd_help);
 DECLARE_SUITE(kallsyms_split);
 
diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
new file mode 100644
index 000000000000..7e61e46283d5
--- /dev/null
+++ b/tools/perf/tests/uncore-event-sorting.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "tests.h"
+#include "debug.h"
+#include "parse-events.h"
+#include "pmu.h"
+#include "pmus.h"
+#include "evlist.h"
+#include <string.h>
+
+struct match_state {
+	char *event1;
+	char *event2;
+};
+
+static int event_cb(void *state, struct pmu_event_info *info)
+{
+	struct match_state *m = state;
+
+	if (!m->event1) {
+		m->event1 = strdup(info->name);
+	} else if (!m->event2) {
+		if (strcmp(m->event1, info->name)) {
+			m->event2 = strdup(info->name);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
+				      int subtest __maybe_unused)
+{
+	struct evlist *evlist;
+	struct parse_events_error err;
+	struct evsel *evsel;
+	struct perf_pmu *pmu = NULL;
+	char *pmu_prefix = NULL;
+	struct match_state m = { NULL, NULL };
+	char buf[1024];
+	int ret;
+
+	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+		size_t len;
+		struct perf_pmu *sibling;
+
+		if (pmu->is_core)
+			continue;
+
+		len = pmu_name_len_no_suffix(pmu->name);
+		if (len == strlen(pmu->name))
+			continue;
+
+		sibling = pmu;
+		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
+			if (sibling->is_core)
+				continue;
+			if (pmu_name_len_no_suffix(sibling->name) == len &&
+			    !strncmp(pmu->name, sibling->name, len))
+				break;
+		}
+
+		if (!sibling)
+			continue;
+
+		m.event1 = m.event2 = NULL;
+		perf_pmu__for_each_event(pmu, false, &m, event_cb);
+
+		if (m.event1 && m.event2) {
+			pmu_prefix = strndup(pmu->name, len);
+			break;
+		}
+		free(m.event1);
+	}
+
+	if (!pmu_prefix) {
+		pr_debug("No suitable uncore PMU found\n");
+		return TEST_SKIP;
+	}
+
+	evlist = evlist__new();
+	if (!evlist)
+		return TEST_FAIL;
+
+	snprintf(buf, sizeof(buf), "{%s/%s/,%s/%s/}",
+		 pmu_prefix, m.event1, pmu_prefix, m.event2);
+	pr_debug("Parsing: %s\n", buf);
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, buf, &err);
+	if (ret) {
+		pr_debug("parse_events failed\n");
+		ret = TEST_FAIL;
+		goto out_err;
+	}
+
+	TEST_ASSERT_VAL("Number of events is > 0", evlist->core.nr_entries > 0);
+	TEST_ASSERT_EQUAL("Number of events is a multiple of 2", evlist->core.nr_entries % 2, 0);
+
+	evlist__for_each_entry(evlist, evsel) {
+		struct evsel *next;
+
+		if (!evsel__is_group_leader(evsel))
+			continue;
+
+		next = evsel__next(evsel);
+		TEST_ASSERT_EQUAL("Group size is 2", evsel->core.nr_members, 2);
+		TEST_ASSERT_VAL("PMU match", evsel->pmu == next->pmu);
+		TEST_ASSERT_VAL("First event name", strstr(evsel__name(evsel), m.event1) != NULL);
+		TEST_ASSERT_VAL("Second event name", strstr(evsel__name(next), m.event2) != NULL);
+	}
+	ret = TEST_OK;
+
+out_err:
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	free(pmu_prefix);
+	free(m.event1);
+	free(m.event2);
+	return ret;
+}
+
+DEFINE_SUITE("Uncore event sorting", uncore_event_sorting);
-- 
2.53.0.1118.gaef5881109-goog

Re: [PATCH v3 1/2] perf tests: Add test for uncore event sorting

[Namhyung Kim]

On Tue, Mar 31, 2026 at 11:54:18AM -0700, Ian Rogers wrote:
> Add a test for uncore event sorting matching multiple PMUs. Uncore
> PMUs may have a common prefix, like the PMUs uncore_imc_free_running_0
> and uncore_imc_free_running_1 have a prefix of
> uncore_imc_free_running. Parsing an event group like
> "{data_read,data_write}" for those PMUs should result with two groups
> "{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_0/data_write/},
> {uncore_imc_free_running_1/data_read/,uncore_imc_free_running_1/data_write/}"
> which means the evsels need resorting as when initially parsed the
> evsels are ordered with mixed PMUs:
> "{uncore_imc_free_running_0/data_read/,uncore_imc_free_running_1/data_read/,
> uncore_imc_free_running_0/data_write/,uncore_imc_free_running_1/data_write/}".

I got this:

  --- start ---                                                                   
  test child forked, pid 1168839                                                  
  Using CPUID AuthenticAMD-23-31-0                                                
  Parsing: {amd_iommu/amd_iommu_0/ign_rd_wr_mmio_1ff8h//,amd_iommu/amd_iommu_0/cmd_processed_inv//}
  parse_events failed                                                             
  ---- end(-1) ----
    6: Uncore event sorting                                            : FAILED!

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> Tested-by: Zide Chen <zide.chen@intel.com>
> ---
>  tools/perf/tests/Build                  |   1 +
>  tools/perf/tests/builtin-test.c         |   1 +
>  tools/perf/tests/tests.h                |   1 +
>  tools/perf/tests/uncore-event-sorting.c | 122 ++++++++++++++++++++++++
>  4 files changed, 125 insertions(+)
>  create mode 100644 tools/perf/tests/uncore-event-sorting.c
> 
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index c2a67ce45941..66944a4f4968 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -3,6 +3,7 @@
>  perf-test-y += builtin-test.o
>  perf-test-y += tests-scripts.o
>  perf-test-y += parse-events.o
> +perf-test-y += uncore-event-sorting.o
>  perf-test-y += dso-data.o
>  perf-test-y += vmlinux-kallsyms.o
>  perf-test-y += openat-syscall.o
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 06507066213b..f2c135891477 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
>  	&suite__basic_mmap,
>  	&suite__mem,
>  	&suite__parse_events,
> +	&suite__uncore_event_sorting,
>  	&suite__expr,
>  	&suite__PERF_RECORD,
>  	&suite__pmu,
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index f5f1238d1f7f..ee00518bf36f 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -177,6 +177,7 @@ DECLARE_SUITE(sigtrap);
>  DECLARE_SUITE(event_groups);
>  DECLARE_SUITE(symbols);
>  DECLARE_SUITE(util);
> +DECLARE_SUITE(uncore_event_sorting);
>  DECLARE_SUITE(subcmd_help);
>  DECLARE_SUITE(kallsyms_split);
>  
> diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
> new file mode 100644
> index 000000000000..7e61e46283d5
> --- /dev/null
> +++ b/tools/perf/tests/uncore-event-sorting.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +#include "tests.h"
> +#include "debug.h"
> +#include "parse-events.h"
> +#include "pmu.h"
> +#include "pmus.h"
> +#include "evlist.h"
> +#include <string.h>
> +
> +struct match_state {
> +	char *event1;
> +	char *event2;
> +};
> +
> +static int event_cb(void *state, struct pmu_event_info *info)
> +{
> +	struct match_state *m = state;
> +
> +	if (!m->event1) {
> +		m->event1 = strdup(info->name);
> +	} else if (!m->event2) {
> +		if (strcmp(m->event1, info->name)) {
> +			m->event2 = strdup(info->name);
> +			return 1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
> +				      int subtest __maybe_unused)
> +{
> +	struct evlist *evlist;
> +	struct parse_events_error err;
> +	struct evsel *evsel;
> +	struct perf_pmu *pmu = NULL;
> +	char *pmu_prefix = NULL;
> +	struct match_state m = { NULL, NULL };
> +	char buf[1024];
> +	int ret;
> +
> +	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> +		size_t len;
> +		struct perf_pmu *sibling;
> +
> +		if (pmu->is_core)
> +			continue;
> +
> +		len = pmu_name_len_no_suffix(pmu->name);
> +		if (len == strlen(pmu->name))
> +			continue;
> +
> +		sibling = pmu;
> +		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
> +			if (sibling->is_core)
> +				continue;
> +			if (pmu_name_len_no_suffix(sibling->name) == len &&
> +			    !strncmp(pmu->name, sibling->name, len))
> +				break;
> +		}
> +
> +		if (!sibling)
> +			continue;
> +
> +		m.event1 = m.event2 = NULL;
> +		perf_pmu__for_each_event(pmu, false, &m, event_cb);
> +
> +		if (m.event1 && m.event2) {
> +			pmu_prefix = strndup(pmu->name, len);
> +			break;
> +		}
> +		free(m.event1);
> +	}
> +
> +	if (!pmu_prefix) {
> +		pr_debug("No suitable uncore PMU found\n");
> +		return TEST_SKIP;
> +	}
> +
> +	evlist = evlist__new();
> +	if (!evlist)
> +		return TEST_FAIL;
> +
> +	snprintf(buf, sizeof(buf), "{%s/%s/,%s/%s/}",
> +		 pmu_prefix, m.event1, pmu_prefix, m.event2);
> +	pr_debug("Parsing: %s\n", buf);
> +
> +	parse_events_error__init(&err);
> +	ret = parse_events(evlist, buf, &err);
> +	if (ret) {
> +		pr_debug("parse_events failed\n");
> +		ret = TEST_FAIL;
> +		goto out_err;
> +	}
> +
> +	TEST_ASSERT_VAL("Number of events is > 0", evlist->core.nr_entries > 0);
> +	TEST_ASSERT_EQUAL("Number of events is a multiple of 2", evlist->core.nr_entries % 2, 0);
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		struct evsel *next;
> +
> +		if (!evsel__is_group_leader(evsel))
> +			continue;
> +
> +		next = evsel__next(evsel);
> +		TEST_ASSERT_EQUAL("Group size is 2", evsel->core.nr_members, 2);
> +		TEST_ASSERT_VAL("PMU match", evsel->pmu == next->pmu);
> +		TEST_ASSERT_VAL("First event name", strstr(evsel__name(evsel), m.event1) != NULL);
> +		TEST_ASSERT_VAL("Second event name", strstr(evsel__name(next), m.event2) != NULL);
> +	}
> +	ret = TEST_OK;
> +
> +out_err:
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	free(pmu_prefix);
> +	free(m.event1);
> +	free(m.event2);
> +	return ret;
> +}
> +
> +DEFINE_SUITE("Uncore event sorting", uncore_event_sorting);
> -- 
> 2.53.0.1118.gaef5881109-goog
> 

[PATCH v3 2/2] perf arch x86 tests: Add test for topdown event sorting

[Ian Rogers]

Add a test to capture the comment in
tools/perf/arch/x86/util/evlist.c. Test that slots and
topdown-retiring get appropriately sorted with respect to instructions
when they're all specified together. When the PMU requires topdown
event grouping (indicated by the pressence of the slots event) metric
events should be after slots, which should be the group leader.

Add a related test that when the slots event isn't given it is
injected into the appropriate group.

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Zide Chen <zide.chen@intel.com>
---
 tools/perf/arch/x86/tests/topdown.c | 134 +++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
index 3ee4e5e71be3..ee8d673a0e7b 100644
--- a/tools/perf/arch/x86/tests/topdown.c
+++ b/tools/perf/arch/x86/tests/topdown.c
@@ -75,4 +75,136 @@ static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest
 	return ret;
 }
 
-DEFINE_SUITE("x86 topdown", x86_topdown);
+static int test_sort(const char *str, int expected_slots_group_size,
+		     int expected_instructions_group_size)
+{
+	struct evlist *evlist;
+	struct parse_events_error err;
+	struct evsel *evsel;
+	int ret = TEST_FAIL;
+	bool slots_seen = false;
+
+	evlist = evlist__new();
+	if (!evlist)
+		return TEST_FAIL;
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, str, &err);
+	if (ret) {
+		pr_debug("parse_events failed for %s\n", str);
+		goto out_err;
+	}
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (!evsel__is_group_leader(evsel))
+			continue;
+
+		if (strstr(evsel__name(evsel), "slots")) {
+			/*
+			 * Slots as a leader means the PMU is for a perf metric
+			 * group as the slots event isn't present when not.
+			 */
+			slots_seen = true;
+			TEST_ASSERT_EQUAL("slots group size", evsel->core.nr_members,
+					expected_slots_group_size);
+			if (expected_slots_group_size == 3) {
+				struct evsel *next = evsel__next(evsel);
+				struct evsel *next2 = evsel__next(next);
+
+				TEST_ASSERT_VAL("slots second event is instructions",
+						strstr(evsel__name(next), "instructions")
+						!= NULL);
+				TEST_ASSERT_VAL("slots third event is topdown-retiring",
+						strstr(evsel__name(next2), "topdown-retiring")
+						!= NULL);
+			} else if (expected_slots_group_size == 2) {
+				struct evsel *next = evsel__next(evsel);
+
+				TEST_ASSERT_VAL("slots second event is topdown-retiring",
+						strstr(evsel__name(next), "topdown-retiring")
+						!= NULL);
+			}
+		} else if (strstr(evsel__name(evsel), "instructions")) {
+			TEST_ASSERT_EQUAL("instructions group size", evsel->core.nr_members,
+					  expected_instructions_group_size);
+			if (expected_instructions_group_size == 2) {
+				/*
+				 * The instructions event leads a group with a
+				 * topdown-retiring event, neither of which need
+				 * reordering for perf metric event support.
+				 */
+				struct evsel *next = evsel__next(evsel);
+
+				TEST_ASSERT_VAL("instructions second event is topdown-retiring",
+						strstr(evsel__name(next), "topdown-retiring")
+						!= NULL);
+			}
+		} else if (strstr(evsel__name(evsel), "topdown-retiring")) {
+			/*
+			 * A perf metric event where the PMU doesn't require
+			 * slots as a leader.
+			 */
+			TEST_ASSERT_EQUAL("topdown-retiring group size", evsel->core.nr_members, 1);
+		} else if (strstr(evsel__name(evsel), "cycles")) {
+			TEST_ASSERT_EQUAL("cycles group size", evsel->core.nr_members, 1);
+		}
+	}
+	TEST_ASSERT_VAL("slots seen", slots_seen);
+	ret = TEST_OK;
+out_err:
+	evlist__delete(evlist);
+	parse_events_error__exit(&err);
+	return ret;
+}
+
+static int test__x86_topdown_sorting(struct test_suite *test __maybe_unused,
+				     int subtest __maybe_unused)
+{
+	if (!topdown_sys_has_perf_metrics())
+		return TEST_OK;
+
+	TEST_ASSERT_EQUAL("all events in a group",
+			  test_sort("{instructions,topdown-retiring,slots}", 3, 2), TEST_OK);
+	TEST_ASSERT_EQUAL("all events not in a group",
+			  test_sort("instructions,topdown-retiring,slots", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event in a group but topdown metrics events outside the group",
+			  test_sort("{instructions,slots},topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event and topdown metrics events in two groups",
+			  test_sort("{instructions,slots},{topdown-retiring}", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("slots event and metrics event are not in a group and not adjacent",
+			  test_sort("{instructions,slots},cycles,topdown-retiring", 2, 1), TEST_OK);
+
+	return TEST_OK;
+}
+
+static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unused,
+					     int subtest __maybe_unused)
+{
+	if (!topdown_sys_has_perf_metrics())
+		return TEST_OK;
+
+	TEST_ASSERT_EQUAL("all events in a group",
+			  test_sort("{instructions,topdown-retiring}", 3, 2), TEST_OK);
+	TEST_ASSERT_EQUAL("all events not in a group",
+			  test_sort("instructions,topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event in a group but topdown metrics events outside the group",
+			  test_sort("{instructions},topdown-retiring", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event and topdown metrics events in two groups",
+			  test_sort("{instructions},{topdown-retiring}", 2, 1), TEST_OK);
+	TEST_ASSERT_EQUAL("event and metrics event are not in a group and not adjacent",
+			  test_sort("{instructions},cycles,topdown-retiring", 2, 1), TEST_OK);
+
+	return TEST_OK;
+}
+
+static struct test_case x86_topdown_tests[] = {
+	TEST_CASE("topdown events", x86_topdown),
+	TEST_CASE("topdown sorting", x86_topdown_sorting),
+	TEST_CASE("topdown slots injection", x86_topdown_slots_injection),
+	{ .name = NULL, }
+};
+
+struct test_suite suite__x86_topdown = {
+	.desc = "x86 topdown",
+	.test_cases = x86_topdown_tests,
+};
-- 
2.53.0.1118.gaef5881109-goog

Re: [PATCH v3 2/2] perf arch x86 tests: Add test for topdown event sorting

[Namhyung Kim]

On Tue, Mar 31, 2026 at 11:54:19AM -0700, Ian Rogers wrote:
> Add a test to capture the comment in
> tools/perf/arch/x86/util/evlist.c. Test that slots and
> topdown-retiring get appropriately sorted with respect to instructions
> when they're all specified together. When the PMU requires topdown
> event grouping (indicated by the pressence of the slots event) metric
> events should be after slots, which should be the group leader.
> 
> Add a related test that when the slots event isn't given it is
> injected into the appropriate group.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> Tested-by: Zide Chen <zide.chen@intel.com>
> ---
>  tools/perf/arch/x86/tests/topdown.c | 134 +++++++++++++++++++++++++++-
>  1 file changed, 133 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> index 3ee4e5e71be3..ee8d673a0e7b 100644
> --- a/tools/perf/arch/x86/tests/topdown.c
> +++ b/tools/perf/arch/x86/tests/topdown.c
> @@ -75,4 +75,136 @@ static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest
>  	return ret;
>  }
>  
> -DEFINE_SUITE("x86 topdown", x86_topdown);
> +static int test_sort(const char *str, int expected_slots_group_size,
> +		     int expected_instructions_group_size)
> +{
> +	struct evlist *evlist;
> +	struct parse_events_error err;
> +	struct evsel *evsel;
> +	int ret = TEST_FAIL;
> +	bool slots_seen = false;
> +
> +	evlist = evlist__new();
> +	if (!evlist)
> +		return TEST_FAIL;
> +
> +	parse_events_error__init(&err);
> +	ret = parse_events(evlist, str, &err);
> +	if (ret) {
> +		pr_debug("parse_events failed for %s\n", str);

It should set ret to TEST_FAIL.  I'll add that this time.

Thanks,
Namhyung


> +		goto out_err;
> +	}
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		if (!evsel__is_group_leader(evsel))
> +			continue;
> +
> +		if (strstr(evsel__name(evsel), "slots")) {
> +			/*
> +			 * Slots as a leader means the PMU is for a perf metric
> +			 * group as the slots event isn't present when not.
> +			 */
> +			slots_seen = true;
> +			TEST_ASSERT_EQUAL("slots group size", evsel->core.nr_members,
> +					expected_slots_group_size);
> +			if (expected_slots_group_size == 3) {
> +				struct evsel *next = evsel__next(evsel);
> +				struct evsel *next2 = evsel__next(next);
> +
> +				TEST_ASSERT_VAL("slots second event is instructions",
> +						strstr(evsel__name(next), "instructions")
> +						!= NULL);
> +				TEST_ASSERT_VAL("slots third event is topdown-retiring",
> +						strstr(evsel__name(next2), "topdown-retiring")
> +						!= NULL);
> +			} else if (expected_slots_group_size == 2) {
> +				struct evsel *next = evsel__next(evsel);
> +
> +				TEST_ASSERT_VAL("slots second event is topdown-retiring",
> +						strstr(evsel__name(next), "topdown-retiring")
> +						!= NULL);
> +			}
> +		} else if (strstr(evsel__name(evsel), "instructions")) {
> +			TEST_ASSERT_EQUAL("instructions group size", evsel->core.nr_members,
> +					  expected_instructions_group_size);
> +			if (expected_instructions_group_size == 2) {
> +				/*
> +				 * The instructions event leads a group with a
> +				 * topdown-retiring event, neither of which need
> +				 * reordering for perf metric event support.
> +				 */
> +				struct evsel *next = evsel__next(evsel);
> +
> +				TEST_ASSERT_VAL("instructions second event is topdown-retiring",
> +						strstr(evsel__name(next), "topdown-retiring")
> +						!= NULL);
> +			}
> +		} else if (strstr(evsel__name(evsel), "topdown-retiring")) {
> +			/*
> +			 * A perf metric event where the PMU doesn't require
> +			 * slots as a leader.
> +			 */
> +			TEST_ASSERT_EQUAL("topdown-retiring group size", evsel->core.nr_members, 1);
> +		} else if (strstr(evsel__name(evsel), "cycles")) {
> +			TEST_ASSERT_EQUAL("cycles group size", evsel->core.nr_members, 1);
> +		}
> +	}
> +	TEST_ASSERT_VAL("slots seen", slots_seen);
> +	ret = TEST_OK;
> +out_err:
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	return ret;
> +}
> +
> +static int test__x86_topdown_sorting(struct test_suite *test __maybe_unused,
> +				     int subtest __maybe_unused)
> +{
> +	if (!topdown_sys_has_perf_metrics())
> +		return TEST_OK;
> +
> +	TEST_ASSERT_EQUAL("all events in a group",
> +			  test_sort("{instructions,topdown-retiring,slots}", 3, 2), TEST_OK);
> +	TEST_ASSERT_EQUAL("all events not in a group",
> +			  test_sort("instructions,topdown-retiring,slots", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("slots event in a group but topdown metrics events outside the group",
> +			  test_sort("{instructions,slots},topdown-retiring", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("slots event and topdown metrics events in two groups",
> +			  test_sort("{instructions,slots},{topdown-retiring}", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("slots event and metrics event are not in a group and not adjacent",
> +			  test_sort("{instructions,slots},cycles,topdown-retiring", 2, 1), TEST_OK);
> +
> +	return TEST_OK;
> +}
> +
> +static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unused,
> +					     int subtest __maybe_unused)
> +{
> +	if (!topdown_sys_has_perf_metrics())
> +		return TEST_OK;
> +
> +	TEST_ASSERT_EQUAL("all events in a group",
> +			  test_sort("{instructions,topdown-retiring}", 3, 2), TEST_OK);
> +	TEST_ASSERT_EQUAL("all events not in a group",
> +			  test_sort("instructions,topdown-retiring", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("event in a group but topdown metrics events outside the group",
> +			  test_sort("{instructions},topdown-retiring", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("event and topdown metrics events in two groups",
> +			  test_sort("{instructions},{topdown-retiring}", 2, 1), TEST_OK);
> +	TEST_ASSERT_EQUAL("event and metrics event are not in a group and not adjacent",
> +			  test_sort("{instructions},cycles,topdown-retiring", 2, 1), TEST_OK);
> +
> +	return TEST_OK;
> +}
> +
> +static struct test_case x86_topdown_tests[] = {
> +	TEST_CASE("topdown events", x86_topdown),
> +	TEST_CASE("topdown sorting", x86_topdown_sorting),
> +	TEST_CASE("topdown slots injection", x86_topdown_slots_injection),
> +	{ .name = NULL, }
> +};
> +
> +struct test_suite suite__x86_topdown = {
> +	.desc = "x86 topdown",
> +	.test_cases = x86_topdown_tests,
> +};
> -- 
> 2.53.0.1118.gaef5881109-goog
>