Re: [PATCH v2] tools/perf: Fix ratio_to_prev event parsing test

[Ian Rogers]

On Thu, Mar 26, 2026 at 11:01 AM Thomas Falcon <thomas.falcon@xxxxxxxxx> wrote:
>
> test__ratio_to_prev() assumed the first event in a group is the leader,
> which is not the case when the event is expanded into two event groups
> on hybrid PMU's with auto counter reload support. This patch updates the
> test to iterate over the event group generated for each core PMU. Also
> update "wrong leader" test to check that the subordinate event has the
> correct leader instead of checking that it is not the group leader.
>
> Signed-off-by: Thomas Falcon <thomas.falcon@xxxxxxxxx>
> Fixes: 56be0fe5f62c ("perf record: Add auto counter reload parse and regression tests")
> ---
> v2: made changes suggested by Dapeng Mi
> https://lore.kernel.org/all/997b6291-7429-4eaa-8467-1fd88e100616@xxxxxxxxxxxxxxx/
> ---
>  tools/perf/tests/parse-events.c | 47 +++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 1d3cc224fbc2..4487508c5658 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1796,31 +1796,38 @@ static bool test__acr_valid(void)
>
>  static int test__ratio_to_prev(struct evlist *evlist)
>  {
> -       struct evsel *evsel;
> +       struct evsel *evsel, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries", 2 * perf_pmus__num_core_pmus() == evlist->core.nr_entries);
>
> -        evlist__for_each_entry(evlist, evsel) {
> -               if (!perf_pmu__has_format(evsel->pmu, "acr_mask"))
> +       for (int i = 0; i < num_core_entries(evlist); i++) {
> +               leader = (i == 0 ? evlist__first(evlist) : evsel__next(evsel));
> +               if (!perf_pmu__has_format(leader->pmu, "acr_mask"))

If the p-core non-ACR supporting events are first, won't this mean the
loop immediately terminates? I wonder the loop should be:
```
evlist__for_each_entry(evlist, evsel) {
               if (evsel != evsel__leader(evsel) ||
!perf_pmu__has_format(evsel->pmu, "acr_mask"))
                    continue; /* Not a group leader or the event is
for a non-ACR supporting PMU. */
```
then the regular tests on the config2, etc. Then evsel__next to get
the instructions event, etc.

Thanks,
Ian


>                         return TEST_OK;
>
> -               if (evsel == evlist__first(evlist)) {
> -                       TEST_ASSERT_VAL("wrong config2", 0 == evsel->core.attr.config2);
> -                       TEST_ASSERT_VAL("wrong leader", evsel__is_group_leader(evsel));
> -                       TEST_ASSERT_VAL("wrong core.nr_members", evsel->core.nr_members == 2);
> -                       TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 0);
> -                       TEST_ASSERT_EVSEL("unexpected event",
> -                                       evsel__match(evsel, HARDWARE, HW_CPU_CYCLES),
> -                                       evsel);
> -               } else {
> -                       TEST_ASSERT_VAL("wrong config2", 0 == evsel->core.attr.config2);
> -                       TEST_ASSERT_VAL("wrong leader", !evsel__is_group_leader(evsel));
> -                       TEST_ASSERT_VAL("wrong core.nr_members", evsel->core.nr_members == 0);
> -                       TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
> -                       TEST_ASSERT_EVSEL("unexpected event",
> -                                       evsel__match(evsel, HARDWARE, HW_INSTRUCTIONS),
> -                                       evsel);
> -               }
> +               /* cycles */
> +               TEST_ASSERT_VAL("wrong config2", 0 == leader->core.attr.config2);
> +               TEST_ASSERT_VAL("wrong leader", evsel__is_group_leader(leader));
> +               TEST_ASSERT_VAL("wrong core.nr_members", leader->core.nr_members == 2);
> +               TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(leader) == 0);
> +               TEST_ASSERT_EVSEL("unexpected event",
> +                                 evsel__match(leader, HARDWARE, HW_CPU_CYCLES),
> +                                 leader);
> +               /*
> +                * The period value gets configured within evlist__config,
> +                * while this test executes only parse events method.
> +                */
> +               TEST_ASSERT_VAL("wrong period", 0 == leader->core.attr.sample_period);
> +
> +                /* instructions/period=200000,ratio-to-prev=2.0/ */
> +               evsel = evsel__next(leader);
> +               TEST_ASSERT_VAL("wrong config2", 0 == evsel->core.attr.config2);
> +               TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
> +               TEST_ASSERT_VAL("wrong core.nr_members", evsel->core.nr_members == 0);
> +               TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
> +               TEST_ASSERT_EVSEL("unexpected event",
> +                                 evsel__match(evsel, HARDWARE, HW_INSTRUCTIONS),
> +                                 evsel);
>                 /*
>                  * The period value gets configured within evlist__config,
>                  * while this test executes only parse events method.
> --
> 2.43.0
>