[PATCH] perf tools: Do not rely on errno values for precise_ip fallback

[PATCH] perf tools: Do not rely on errno values for precise_ip fallback

[Jiri Olsa]

Konstantin reported problem with default perf record command,
which fails on some AMD servers, because of the default maximum
precise config.

The current fallback mechanism counts on getting ENOTSUP errno for
precise_ip fails, but that's not the case on some AMD servers.

We can fix this by removing the errno check completely, because the
precise_ip fallback is separated. We can just try  (if requested by
evsel->precise_max) all possible precise_ip, and if one succeeds we
win, if not, we continue with standard fallback.

Reported-by: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: Kim Phillips <kim.phillips@amd.com>
Cc: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/evsel.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5ab31a4a658d..7fb4ae82f34c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1800,14 +1800,8 @@ static int perf_event_open(struct perf_evsel *evsel,
 		if (fd >= 0)
 			break;
 
-		/*
-		 * Do quick precise_ip fallback if:
-		 *  - there is precise_ip set in perf_event_attr
-		 *  - maximum precise is requested
-		 *  - sys_perf_event_open failed with ENOTSUP error,
-		 *    which is associated with wrong precise_ip
-		 */
-		if (!precise_ip || !evsel->precise_max || (errno != ENOTSUP))
+		/* Do not try less precise if not requested. */
+		if (!evsel->precise_max)
 			break;
 
 		/*
-- 
2.21.0

Re: [PATCH] perf tools: Do not rely on errno values for precise_ip fallback

[Arnaldo Carvalho de Melo]

Em Wed, Jul 03, 2019 at 10:09:49AM +0200, Jiri Olsa escreveu:
> Konstantin reported problem with default perf record command,
> which fails on some AMD servers, because of the default maximum
> precise config.
> 
> The current fallback mechanism counts on getting ENOTSUP errno for
> precise_ip fails, but that's not the case on some AMD servers.
> 
> We can fix this by removing the errno check completely, because the
> precise_ip fallback is separated. We can just try  (if requested by
> evsel->precise_max) all possible precise_ip, and if one succeeds we
> win, if not, we continue with standard fallback.

Thanks, applied.

Simple test shows that behaviour continues the same on x86_64.

- Arnaldo
 
> Reported-by: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: Kim Phillips <kim.phillips@amd.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/evsel.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 5ab31a4a658d..7fb4ae82f34c 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1800,14 +1800,8 @@ static int perf_event_open(struct perf_evsel *evsel,
>  		if (fd >= 0)
>  			break;
>  
> -		/*
> -		 * Do quick precise_ip fallback if:
> -		 *  - there is precise_ip set in perf_event_attr
> -		 *  - maximum precise is requested
> -		 *  - sys_perf_event_open failed with ENOTSUP error,
> -		 *    which is associated with wrong precise_ip
> -		 */
> -		if (!precise_ip || !evsel->precise_max || (errno != ENOTSUP))
> +		/* Do not try less precise if not requested. */
> +		if (!evsel->precise_max)
>  			break;
>  
>  		/*
> -- 
> 2.21.0

-- 

- Arnaldo

[tip:perf/core] perf evsel: Do not rely on errno values for precise_ip fallback

[tip-bot for Jiri Olsa]

Commit-ID:  cd136189370cc8a5aec0ea4b4ec517e5ee38d8a0
Gitweb:     https://git.kernel.org/tip/cd136189370cc8a5aec0ea4b4ec517e5ee38d8a0
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 3 Jul 2019 10:09:49 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 6 Jul 2019 14:30:30 -0300

perf evsel: Do not rely on errno values for precise_ip fallback

Konstantin reported problem with default perf record command, which
fails on some AMD servers, because of the default maximum precise
config.

The current fallback mechanism counts on getting ENOTSUP errno for
precise_ip fails, but that's not the case on some AMD servers.

We can fix this by removing the errno check completely, because the
precise_ip fallback is separated. We can just try  (if requested by
evsel->precise_max) all possible precise_ip, and if one succeeds we win,
if not, we continue with standard fallback.

Reported-by: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: Kim Phillips <kim.phillips@amd.com>
Link: http://lkml.kernel.org/r/20190703080949.10356-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4a5947625c5c..69beb9f80f07 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1785,14 +1785,8 @@ static int perf_event_open(struct perf_evsel *evsel,
 		if (fd >= 0)
 			break;
 
-		/*
-		 * Do quick precise_ip fallback if:
-		 *  - there is precise_ip set in perf_event_attr
-		 *  - maximum precise is requested
-		 *  - sys_perf_event_open failed with ENOTSUP error,
-		 *    which is associated with wrong precise_ip
-		 */
-		if (!precise_ip || !evsel->precise_max || (errno != ENOTSUP))
+		/* Do not try less precise if not requested. */
+		if (!evsel->precise_max)
 			break;
 
 		/*