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

[Ian Rogers]

On Thu, Mar 26, 2026 at 9:51 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> On Thu, Mar 26, 2026 at 7:01 PM 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. Instead, 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. Finally, do not exit immediately if a PMU
> > without auto counter reload support is found.
> >
> > Signed-off-by: Thomas Falcon <thomas.falcon@xxxxxxxxx>
> > Fixes: 56be0fe5f62c ("perf record: Add auto counter reload parse and regression tests")
>
> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Ping. Thanks,
Ian

> Thanks!
> Ian
>
> > ---
> > v3: Fixed early loop termination if PMU without auto counter reload support is
> >     encountered first, suggested by Ian Rogers
> > https://lore.kernel.org/all/CAP-5=fUFGevUhRROdybtDCXZgWgxYzfLrq_0f8r5_Yv_VfxP+A@xxxxxxxxxxxxxx/
> >
> > v2: made changes suggested by Dapeng Mi
> > https://lore.kernel.org/all/997b6291-7429-4eaa-8467-1fd88e100616@xxxxxxxxxxxxxxx/
> > ---
> >
> >  tools/perf/tests/parse-events.c | 49 +++++++++++++++++++--------------
> >  1 file changed, 28 insertions(+), 21 deletions(-)
> >
> > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> > index 1d3cc224f..05c3e899b 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"))
> > -                       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);
> > +       evlist__for_each_entry(evlist, evsel) {
> > +               if (evsel != evsel__leader(evsel) ||
> > +                   !perf_pmu__has_format(evsel->pmu, "acr_mask")) {
> > +                       continue;
> >                 }
> > +               leader = evsel;
> > +               /* cycles */
> > +               TEST_ASSERT_VAL("wrong config2", 0 == leader->core.attr.config2);
> > +               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(evsel);
> > +               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.51.0
> >