linux-kernel - Re: [PATCH 2/2] perf tests: Fix Convert perf time to TSC test for hybrid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <75aaa2a8-7956-84bb-a811-f70802753e21@linux.intel.com>
Date:   Wed, 13 Jul 2022 09:05:21 -0400
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Adrian Hunter <adrian.hunter@...el.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     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



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,
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;
>  			}