Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230704143628.1177124-1-james.clark@arm.com> <20230704143628.1177124-2-james.clark@arm.com>
In-Reply-To: <20230704143628.1177124-2-james.clark@arm.com>
From:   Ian Rogers <irogers@google.com>
Date:   Tue, 4 Jul 2023 10:12:56 -0700
Message-ID: <CAP-5=fWt7jrRhS3CU6H-CqWJixnX4tZY+X=+oKhKwCjYcYZWwA@mail.gmail.com>
Subject: Re: [PATCH 2/2] perf test: Fix event parsing test when
 PERF_PMU_CAP_EXTENDED_HW_TYPE isn't supported.
To:     James Clark <james.clark@arm.com>
Cc:     linux-perf-users@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Jul 4, 2023 at 7:38=E2=80=AFAM James Clark <james.clark@arm.com> wr=
ote:
>
> Arm has multiple PMU types for heterogeneous systems, but doesn't
> currently support PERF_PMU_CAP_EXTENDED_HW_TYPE. Make the tests
> support both scenarios so that they pass on Arm, and will still pass
> once PERF_PMU_CAP_EXTENDED_HW_TYPE support is added.
>
> Fixes: 27c9fcfc1e14 ("perf test: Update parse-events expectations to test=
 for multiple events")
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/tests/parse-events.c | 86 +++++++++++++++++++--------------
>  1 file changed, 50 insertions(+), 36 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-eve=
nts.c
> index 21f79aa31233..454b667e144b 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -20,6 +20,20 @@
>  #define PERF_TP_SAMPLE_TYPE (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | \
>                              PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD)
>
> +static int num_entries(void)

Thanks James! A nit, I'd prefer a function name that is a little less
ambiguous, perhaps num_core_events_per_event which is too long,
perhaps num_core_entries. Other than that:

Acked-by: Ian Rogers <irogers@google.com>

Ian

> +{
> +       /*
> +        * If the kernel supports extended type, expect events to be
> +        * opened once for each PMU type. Otherwise fall back to the lega=
cy
> +        * behavior of opening only one event even though there are multi=
ple
> +        * PMUs
> +        */
> +       if (perf_pmus__supports_extended_type())
> +               return perf_pmus__num_core_pmus();
> +
> +       return 1;
> +}
> +
>  static bool test_config(const struct evsel *evsel, __u64 expected_config=
)
>  {
>         __u32 type =3D evsel->core.attr.type;
> @@ -339,7 +353,7 @@ static int test__checkevent_symbolic_name_modifier(st=
ruct evlist *evlist)
>         struct perf_evsel *evsel;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D perf_pmus__num_cor=
e_pmus());
> +                       evlist->core.nr_entries =3D=3D num_entries());
>
>         perf_evlist__for_each_entry(&evlist->core, evsel) {
>                 TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude=
_user);
> @@ -842,11 +856,11 @@ static int test__group1(struct evlist *evlist)
>         struct evsel *evsel, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (perf_pmus__num_co=
re_pmus() * 2));
> +                       evlist->core.nr_entries =3D=3D (num_entries() * 2=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       evlist__nr_groups(evlist) =3D=3D perf_pmus__num_c=
ore_pmus());
> +                       evlist__nr_groups(evlist) =3D=3D num_entries());
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* instructions:k */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -885,7 +899,7 @@ static int test__group2(struct evlist *evlist)
>         struct evsel *evsel, *leader =3D NULL;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (2 * perf_pmus__nu=
m_core_pmus() + 1));
> +                       evlist->core.nr_entries =3D=3D (2 * num_entries()=
 + 1));
>         /*
>          * TODO: Currently the software event won't be grouped with the h=
ardware
>          * event except for 1 PMU.
> @@ -1051,11 +1065,11 @@ static int test__group4(struct evlist *evlist __m=
aybe_unused)
>         struct evsel *evsel, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (perf_pmus__num_co=
re_pmus() * 2));
> +                       evlist->core.nr_entries =3D=3D (num_entries() * 2=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       perf_pmus__num_core_pmus() =3D=3D evlist__nr_grou=
ps(evlist));
> +                       num_entries() =3D=3D evlist__nr_groups(evlist));
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles:u + p */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1096,11 +1110,11 @@ static int test__group5(struct evlist *evlist __m=
aybe_unused)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (5 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (5 * num_entries()=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       evlist__nr_groups(evlist) =3D=3D (2 * perf_pmus__=
num_core_pmus()));
> +                       evlist__nr_groups(evlist) =3D=3D (2 * num_entries=
()));
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles + G */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1131,7 +1145,7 @@ static int test__group5(struct evlist *evlist __may=
be_unused)
>                 TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel=
) =3D=3D 1);
>                 TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read)=
;
>         }
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles:G */
>                 evsel =3D leader =3D evsel__next(evsel);
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1161,7 +1175,7 @@ static int test__group5(struct evlist *evlist __may=
be_unused)
>                 TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, =
leader));
>                 TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel=
) =3D=3D 1);
>         }
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles */
>                 evsel =3D evsel__next(evsel);
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1182,11 +1196,11 @@ static int test__group_gh1(struct evlist *evlist)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (2 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (2 * num_entries()=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       evlist__nr_groups(evlist) =3D=3D perf_pmus__num_c=
ore_pmus());
> +                       evlist__nr_groups(evlist) =3D=3D num_entries());
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles + :H group modifier */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1223,11 +1237,11 @@ static int test__group_gh2(struct evlist *evlist)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (2 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (2 * num_entries()=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       evlist__nr_groups(evlist) =3D=3D perf_pmus__num_c=
ore_pmus());
> +                       evlist__nr_groups(evlist) =3D=3D num_entries());
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles + :G group modifier */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1264,11 +1278,11 @@ static int test__group_gh3(struct evlist *evlist)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (2 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (2 * num_entries()=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       evlist__nr_groups(evlist) =3D=3D perf_pmus__num_c=
ore_pmus());
> +                       evlist__nr_groups(evlist) =3D=3D num_entries());
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles:G + :u group modifier */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1305,11 +1319,11 @@ static int test__group_gh4(struct evlist *evlist)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (2 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (2 * num_entries()=
));
>         TEST_ASSERT_VAL("wrong number of groups",
> -                       evlist__nr_groups(evlist) =3D=3D perf_pmus__num_c=
ore_pmus());
> +                       evlist__nr_groups(evlist) =3D=3D num_entries());
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles:G + :uG group modifier */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1346,9 +1360,9 @@ static int test__leader_sample1(struct evlist *evli=
st)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (3 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (3 * num_entries()=
));
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles - sampling group leader */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1398,9 +1412,9 @@ static int test__leader_sample2(struct evlist *evli=
st __maybe_unused)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (2 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (2 * num_entries()=
));
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* instructions - sampling group leader */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1437,9 +1451,9 @@ static int test__checkevent_pinned_modifier(struct =
evlist *evlist)
>         struct evsel *evsel =3D NULL;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D perf_pmus__num_cor=
e_pmus());
> +                       evlist->core.nr_entries =3D=3D num_entries());
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 evsel =3D (i =3D=3D 0 ? evlist__first(evlist) : evsel__ne=
xt(evsel));
>                 TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.e=
xclude_user);
>                 TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.=
exclude_kernel);
> @@ -1455,9 +1469,9 @@ static int test__pinned_group(struct evlist *evlist=
)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (3 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D (3 * num_entries()=
));
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles - group leader */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1500,9 +1514,9 @@ static int test__exclusive_group(struct evlist *evl=
ist)
>         struct evsel *evsel =3D NULL, *leader;
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (3 * perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D 3 * num_entries())=
;
>
> -       for (int i =3D 0; i < perf_pmus__num_core_pmus(); i++) {
> +       for (int i =3D 0; i < num_entries(); i++) {
>                 /* cycles - group leader */
>                 evsel =3D leader =3D (i =3D=3D 0 ? evlist__first(evlist) =
: evsel__next(evsel));
>                 TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE =3D=3D e=
vsel->core.attr.type);
> @@ -1574,7 +1588,7 @@ static int test__checkevent_precise_max_modifier(st=
ruct evlist *evlist)
>         struct evsel *evsel =3D evlist__first(evlist);
>
>         TEST_ASSERT_VAL("wrong number of entries",
> -                       evlist->core.nr_entries =3D=3D (1 + perf_pmus__nu=
m_core_pmus()));
> +                       evlist->core.nr_entries =3D=3D 1 + num_entries())=
;
>         TEST_ASSERT_VAL("wrong type", PERF_TYPE_SOFTWARE =3D=3D evsel->co=
re.attr.type);
>         TEST_ASSERT_VAL("wrong config", test_config(evsel, PERF_COUNT_SW_=
TASK_CLOCK));
>         return TEST_OK;
> --
> 2.34.1
>