[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YtQVa189ATKJ5Ey1@kernel.org>
Date: Sun, 17 Jul 2022 10:58:03 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: Adrian Hunter <adrian.hunter@...el.com>,
Jiri Olsa <jolsa@...hat.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for
hybrid
Em Wed, Jul 13, 2022 at 09:05:21AM -0400, Liang, Kan escreveu:
>
>
> On 2022-07-13 8:34 a.m., Adrian Hunter wrote:
> > The test does not always correctly determine the number of events for
> > hybrids, nor allow for more than 1 evsel when parsing.
> >
> > Fix by iterating the events actually created and getting the correct
> > evsel for the events processed.
> >
>
> Yes, we cannot always assume there are two events for hybrid.
>
> > Fixes: d9da6f70eb23 ("perf tests: Support 'Convert perf time to TSC' test for hybrid")
> > Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
>
>
> Reviewed-by: Kan Liang <kan.liang@...ux.intel.com>
Thanks, applied.
- Arnaldo
> Thanks,
> Kan
> > ---
> > tools/perf/tests/perf-time-to-tsc.c | 18 ++++--------------
> > 1 file changed, 4 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
> > index 8d6d60173693..7c7d20fc503a 100644
> > --- a/tools/perf/tests/perf-time-to-tsc.c
> > +++ b/tools/perf/tests/perf-time-to-tsc.c
> > @@ -20,8 +20,6 @@
> > #include "tsc.h"
> > #include "mmap.h"
> > #include "tests.h"
> > -#include "pmu.h"
> > -#include "pmu-hybrid.h"
> >
> > /*
> > * Except x86_64/i386 and Arm64, other archs don't support TSC in perf. Just
> > @@ -106,18 +104,8 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
> >
> > evlist__config(evlist, &opts, NULL);
> >
> > - evsel = evlist__first(evlist);
> > -
> > - evsel->core.attr.comm = 1;
> > - evsel->core.attr.disabled = 1;
> > - evsel->core.attr.enable_on_exec = 0;
> > -
> > - /*
> > - * For hybrid "cycles:u", it creates two events.
> > - * Init the second evsel here.
> > - */
> > - if (perf_pmu__has_hybrid() && perf_pmu__hybrid_mounted("cpu_atom")) {
> > - evsel = evsel__next(evsel);
> > + /* For hybrid "cycles:u", it creates two events */
> > + evlist__for_each_entry(evlist, evsel) {
> > evsel->core.attr.comm = 1;
> > evsel->core.attr.disabled = 1;
> > evsel->core.attr.enable_on_exec = 0;
> > @@ -170,10 +158,12 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
> > goto next_event;
> >
> > if (strcmp(event->comm.comm, comm1) == 0) {
> > + CHECK_NOT_NULL__(evsel = evlist__event2evsel(evlist, event));
> > CHECK__(evsel__parse_sample(evsel, event, &sample));
> > comm1_time = sample.time;
> > }
> > if (strcmp(event->comm.comm, comm2) == 0) {
> > + CHECK_NOT_NULL__(evsel = evlist__event2evsel(evlist, event));
> > CHECK__(evsel__parse_sample(evsel, event, &sample));
> > comm2_time = sample.time;
> > }
--
- Arnaldo
Powered by blists - more mailing lists