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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The test fails to verify that events from multiple PMUs were actually parsed, allowing it to silently pass without exercising the multi-PMU sorting logic.
--

commit 7547d1b7750e7305c495d31d775f2a8e3af0325a
Author: Ian Rogers <irogers@google.com>

perf tests: Add test for uncore event sorting

This commit adds a test to verify that uncore events with a common PMU prefix
are properly resorted when parsed. It sets up a scenario matching multiple
sibling PMUs and checks that the resulting event list correctly groups the
events by PMU.

> diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/perf/tests/uncore-event-sorting.c
[ ... ]
> +	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);

If parse_events() encounters a regression and only matches a single PMU instead
of expanding to all siblings, evlist->core.nr_entries would evaluate to 2.

Since 2 is greater than 0 and a multiple of 2, wouldn't the test silently pass
without actually exercising the multi-PMU sorting logic it was designed for?

Should this test assert that evlist->core.nr_entries >= 4 to guarantee that
the mixed PMU resorting scenario is genuinely validated?

> +
> +	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);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517232849.2391357-1-irogers@google.com?part=1