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

[Namhyung Kim <namhyung@kernel.org>]

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
>