* [PATCH v2] tools/perf: Fix ratio_to_prev event parsing test
@ 2026-03-26 18:01 Thomas Falcon
2026-03-26 21:32 ` Ian Rogers
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Falcon @ 2026-03-26 18:01 UTC (permalink / raw)
To: linux-perf-users, Dapeng Mi
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tools/perf: Fix ratio_to_prev event parsing test
2026-03-26 18:01 [PATCH v2] tools/perf: Fix ratio_to_prev event parsing test Thomas Falcon
@ 2026-03-26 21:32 ` Ian Rogers
2026-03-27 0:37 ` Falcon, Thomas
2026-03-27 1:59 ` [PATCH v3] " Thomas Falcon
0 siblings, 2 replies; 8+ messages in thread
From: Ian Rogers @ 2026-03-26 21:32 UTC (permalink / raw)
To: Thomas Falcon
Cc: linux-perf-users, Dapeng Mi, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tools/perf: Fix ratio_to_prev event parsing test
2026-03-26 21:32 ` Ian Rogers
@ 2026-03-27 0:37 ` Falcon, Thomas
2026-03-27 1:59 ` [PATCH v3] " Thomas Falcon
1 sibling, 0 replies; 8+ messages in thread
From: Falcon, Thomas @ 2026-03-27 0:37 UTC (permalink / raw)
To: Rogers, Ian
Cc: james.clark@linaro.org, linux-kernel@vger.kernel.org,
alexander.shishkin@linux.intel.com, peterz@infradead.org,
mark.rutland@arm.com, linux-perf-users@vger.kernel.org,
dapeng1.mi@linux.intel.com, acme@kernel.org, namhyung@kernel.org,
jolsa@kernel.org, mingo@redhat.com, Hunter, Adrian
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
> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] tools/perf: Fix ratio_to_prev event parsing test
2026-03-26 21:32 ` Ian Rogers
2026-03-27 0:37 ` Falcon, Thomas
@ 2026-03-27 1:59 ` Thomas Falcon
2026-03-27 4:51 ` Ian Rogers
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Thomas Falcon @ 2026-03-27 1:59 UTC (permalink / raw)
To: linux-perf-users, Ian Rogers, Dapeng Mi
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] tools/perf: Fix ratio_to_prev event parsing test
2026-03-27 1:59 ` [PATCH v3] " Thomas Falcon
@ 2026-03-27 4:51 ` Ian Rogers
2026-04-01 20:08 ` Ian Rogers
2026-03-27 5:38 ` Mi, Dapeng
2026-04-02 21:57 ` Namhyung Kim
2 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2026-03-27 4:51 UTC (permalink / raw)
To: Thomas Falcon
Cc: linux-perf-users, Dapeng Mi, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] tools/perf: Fix ratio_to_prev event parsing test
2026-03-27 1:59 ` [PATCH v3] " Thomas Falcon
2026-03-27 4:51 ` Ian Rogers
@ 2026-03-27 5:38 ` Mi, Dapeng
2026-04-02 21:57 ` Namhyung Kim
2 siblings, 0 replies; 8+ messages in thread
From: Mi, Dapeng @ 2026-03-27 5:38 UTC (permalink / raw)
To: Thomas Falcon, linux-perf-users, Ian Rogers
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] tools/perf: Fix ratio_to_prev event parsing test
2026-03-27 4:51 ` Ian Rogers
@ 2026-04-01 20:08 ` Ian Rogers
0 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2026-04-01 20:08 UTC (permalink / raw)
To: Namhyung Kim, Arnaldo Carvalho de Melo
Cc: linux-perf-users, Dapeng Mi, linux-kernel, Peter Zijlstra,
Ingo Molnar, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
James Clark, Thomas Falcon
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
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] tools/perf: Fix ratio_to_prev event parsing test
2026-03-27 1:59 ` [PATCH v3] " Thomas Falcon
2026-03-27 4:51 ` Ian Rogers
2026-03-27 5:38 ` Mi, Dapeng
@ 2026-04-02 21:57 ` Namhyung Kim
2 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2026-04-02 21:57 UTC (permalink / raw)
To: linux-perf-users, Ian Rogers, Dapeng Mi, Thomas Falcon
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, James Clark
On Thu, 26 Mar 2026 20:59:27 -0500, 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.
>
> [...]
Applied to perf-tools-next, thanks!
Best regards,
Namhyung
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-04-02 21:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-26 18:01 [PATCH v2] tools/perf: Fix ratio_to_prev event parsing test Thomas Falcon
2026-03-26 21:32 ` Ian Rogers
2026-03-27 0:37 ` Falcon, Thomas
2026-03-27 1:59 ` [PATCH v3] " Thomas Falcon
2026-03-27 4:51 ` Ian Rogers
2026-04-01 20:08 ` Ian Rogers
2026-03-27 5:38 ` Mi, Dapeng
2026-04-02 21:57 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox