* [PATCH 0/2] perf tests: Fix Convert perf time to TSC test for hybrid
@ 2022-07-13 12:34 Adrian Hunter
2022-07-13 12:34 ` [PATCH 1/2] perf tests: Stop Convert perf time to TSC test opening events twice Adrian Hunter
2022-07-13 12:34 ` [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
0 siblings, 2 replies; 6+ messages in thread
From: Adrian Hunter @ 2022-07-13 12:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Kan Liang, Thomas Richter, Namhyung Kim, Ian Rogers,
linux-kernel
Hi
Here are a couple more minor non-urgent fixes.
Adrian Hunter (2):
perf tests: Stop Convert perf time to TSC test opening events twice
perf tests: Fix Convert perf time to TSC test for hybrid
tools/perf/tests/perf-time-to-tsc.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
Regards
Adrian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] perf tests: Stop Convert perf time to TSC test opening events twice
2022-07-13 12:34 [PATCH 0/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
@ 2022-07-13 12:34 ` Adrian Hunter
2022-07-13 13:03 ` Liang, Kan
2022-07-13 12:34 ` [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2022-07-13 12:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Kan Liang, Thomas Richter, Namhyung Kim, Ian Rogers,
linux-kernel
Do not call evlist__open() twice.
Fixes: 5bb017d4b97a ("perf test: Fix error message for test case 71 on s390, where it is not supported")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
tools/perf/tests/perf-time-to-tsc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
index 4ad0dfbc8b21..8d6d60173693 100644
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ b/tools/perf/tests/perf-time-to-tsc.c
@@ -123,11 +123,14 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
evsel->core.attr.enable_on_exec = 0;
}
- if (evlist__open(evlist) == -ENOENT) {
- err = TEST_SKIP;
+ ret = evlist__open(evlist);
+ if (ret < 0) {
+ if (ret == -ENOENT)
+ err = TEST_SKIP;
+ else
+ pr_debug("evlist__open() failed\n");
goto out_err;
}
- CHECK__(evlist__open(evlist));
CHECK__(evlist__mmap(evlist, UINT_MAX));
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid
2022-07-13 12:34 [PATCH 0/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
2022-07-13 12:34 ` [PATCH 1/2] perf tests: Stop Convert perf time to TSC test opening events twice Adrian Hunter
@ 2022-07-13 12:34 ` Adrian Hunter
2022-07-13 13:05 ` Liang, Kan
1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2022-07-13 12:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Kan Liang, Thomas Richter, Namhyung Kim, Ian Rogers,
linux-kernel
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.
Fixes: d9da6f70eb23 ("perf tests: Support 'Convert perf time to TSC' test for hybrid")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf tests: Stop Convert perf time to TSC test opening events twice
2022-07-13 12:34 ` [PATCH 1/2] perf tests: Stop Convert perf time to TSC test opening events twice Adrian Hunter
@ 2022-07-13 13:03 ` Liang, Kan
0 siblings, 0 replies; 6+ messages in thread
From: Liang, Kan @ 2022-07-13 13:03 UTC (permalink / raw)
To: Adrian Hunter, Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Thomas Richter, Namhyung Kim, Ian Rogers, linux-kernel
On 2022-07-13 8:34 a.m., Adrian Hunter wrote:
> Do not call evlist__open() twice.
>
> Fixes: 5bb017d4b97a ("perf test: Fix error message for test case 71 on s390, where it is not supported")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>> ---
> tools/perf/tests/perf-time-to-tsc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
> index 4ad0dfbc8b21..8d6d60173693 100644
> --- a/tools/perf/tests/perf-time-to-tsc.c
> +++ b/tools/perf/tests/perf-time-to-tsc.c
> @@ -123,11 +123,14 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
> evsel->core.attr.enable_on_exec = 0;
> }
>
> - if (evlist__open(evlist) == -ENOENT) {
> - err = TEST_SKIP;
> + ret = evlist__open(evlist);
> + if (ret < 0) {
> + if (ret == -ENOENT)
> + err = TEST_SKIP;
> + else
> + pr_debug("evlist__open() failed\n");
The error message for the evlist__open is slightly different with the
old one, "evlist__open(evlist) failed!". Not sure if anyone care about it.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> goto out_err;
> }
> - CHECK__(evlist__open(evlist));
>
> CHECK__(evlist__mmap(evlist, UINT_MAX));
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid
2022-07-13 12:34 ` [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
@ 2022-07-13 13:05 ` Liang, Kan
2022-07-17 13:58 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Liang, Kan @ 2022-07-13 13:05 UTC (permalink / raw)
To: Adrian Hunter, Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Thomas Richter, Namhyung Kim, Ian Rogers, linux-kernel
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@intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
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;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid
2022-07-13 13:05 ` Liang, Kan
@ 2022-07-17 13:58 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-07-17 13:58 UTC (permalink / raw)
To: Liang, Kan
Cc: Adrian Hunter, Jiri Olsa, Thomas Richter, Namhyung Kim,
Ian Rogers, linux-kernel
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@intel.com>
>
>
> Reviewed-by: Kan Liang <kan.liang@linux.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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-17 13:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13 12:34 [PATCH 0/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
2022-07-13 12:34 ` [PATCH 1/2] perf tests: Stop Convert perf time to TSC test opening events twice Adrian Hunter
2022-07-13 13:03 ` Liang, Kan
2022-07-13 12:34 ` [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid Adrian Hunter
2022-07-13 13:05 ` Liang, Kan
2022-07-17 13:58 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).