tools/perf/tests/parse-events.c | 47 +++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 20 deletions(-)
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@intel.com>
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@linux.intel.com/
---
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"))
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
On Thu, Mar 26, 2026 at 11:01 AM Thomas Falcon <thomas.falcon@intel.com> 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@intel.com>
> 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@linux.intel.com/
> ---
> 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
>
On Thu, 2026-03-26 at 14:32 -0700, Ian Rogers wrote:
> On Thu, Mar 26, 2026 at 11:01 AM Thomas Falcon
> <thomas.falcon@intel.com> 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@intel.com>
> > 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@linux.intel.com/
> > ---
> > 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.
Hi Ian, thanks for catching that! I will send a new version soon.
Thanks,
Tom
>
> 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
> >
>
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@intel.com>
Fixes: 56be0fe5f62c ("perf record: Add auto counter reload parse and regression tests")
---
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@mail.gmail.com/
v2: made changes suggested by Dapeng Mi
https://lore.kernel.org/all/997b6291-7429-4eaa-8467-1fd88e100616@linux.intel.com/
---
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
Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
On 3/27/2026 9:59 AM, Thomas Falcon 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@intel.com>
> Fixes: 56be0fe5f62c ("perf record: Add auto counter reload parse and regression tests")
> ---
> 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@mail.gmail.com/
>
> v2: made changes suggested by Dapeng Mi
> https://lore.kernel.org/all/997b6291-7429-4eaa-8467-1fd88e100616@linux.intel.com/
> ---
>
> 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.
On Thu, Mar 26, 2026 at 7:01 PM Thomas Falcon <thomas.falcon@intel.com> 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@intel.com>
> Fixes: 56be0fe5f62c ("perf record: Add auto counter reload parse and regression tests")
Reviewed-by: Ian Rogers <irogers@google.com>
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@mail.gmail.com/
>
> v2: made changes suggested by Dapeng Mi
> https://lore.kernel.org/all/997b6291-7429-4eaa-8467-1fd88e100616@linux.intel.com/
> ---
>
> 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
>
On Thu, Mar 26, 2026 at 9:51 PM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Mar 26, 2026 at 7:01 PM Thomas Falcon <thomas.falcon@intel.com> 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@intel.com>
> > Fixes: 56be0fe5f62c ("perf record: Add auto counter reload parse and regression tests")
>
> Reviewed-by: Ian Rogers <irogers@google.com>
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@mail.gmail.com/
> >
> > v2: made changes suggested by Dapeng Mi
> > https://lore.kernel.org/all/997b6291-7429-4eaa-8467-1fd88e100616@linux.intel.com/
> > ---
> >
> > 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
> >
© 2016 - 2026 Red Hat, Inc.