Skip to contentSkip to conversation

perf unwind: Limit warnings when asked for not supported unwind

PATCH In review Jiri Olsa 3 participants 4 messages Jun 16, 2017 – Jun 19, 2017 1 message not in the archive perf events

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

PATCH 1/1 perf unwind: Limit warnings when asked for not supported unwind

Ingo reported following warning flooding the report out:

unwind: target platform=x86 is not supported

We trigger this warning when the dwarf unwinder is asked to process architecture which wasn't compiled in, like when you get 32bit application samples on your 64bit server and you don't have the 32bit remote unwind support compiled in.

This patch limits the warning to single message for arch, and adds bits info. Above message is changed to:

unwind: target platform=x86 32bit is not supported

---
 tools/perf/util/unwind-libunwind.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
tools/perf/util/unwind-libunwind.c +24 -5
index 6d542a4e0648..1439a6bcfa07 100644
@@ -1,12 +1,24 @@
+#include <linux/err.h>
 #include "unwind.h"
 #include "thread.h"
 #include "session.h"
 #include "debug.h"
 #include "arch/common.h"
 
-struct unwind_libunwind_ops __weak *local_unwind_libunwind_ops;
-struct unwind_libunwind_ops __weak *x86_32_unwind_libunwind_ops;
-struct unwind_libunwind_ops __weak *arm64_unwind_libunwind_ops;
+enum {
+	ERR_LOCAL  = 0,
+	ERR_X86_32 = 1,
+	ERR_ARM64  = 2,
+};
+
+/*
+ * Set default error values, so we can warn appropriately when
+ * the support is not compiled in. Using negative values so we
+ * can use ERR macros.
+ */
+struct unwind_libunwind_ops __weak *local_unwind_libunwind_ops  = (void *) -ERR_LOCAL;
+struct unwind_libunwind_ops __weak *x86_32_unwind_libunwind_ops = (void *) -ERR_X86_32;
+struct unwind_libunwind_ops __weak *arm64_unwind_libunwind_ops  = (void *) -ERR_ARM64;
 
 static void unwind__register_ops(struct thread *thread,
 			  struct unwind_libunwind_ops *ops)
@@ -48,8 +60,15 @@ int unwind__prepare_access(struct thread *thread, struct map *map,
 			ops = arm64_unwind_libunwind_ops;
 	}
 
-	if (!ops) {
-		pr_err("unwind: target platform=%s is not supported\n", arch);
+	if (IS_ERR(ops)) {
+		static unsigned long warned;
+		long bit = -PTR_ERR(ops);
+
+		if (!test_and_set_bit(bit, &warned)) {
+			pr_err("unwind: target platform=%s %dbit is not supported\n",
+				arch, dso_type == DSO__TYPE_64BIT ? 64 : 32);
+		}
+
 		return -1;
 	}
 out_register:
Signature · 1 line
2.9.4
#1 lore raw

Em Fri, Jun 16, 2017 at 02:12:53PM +0200, Jiri Olsa escreveu:

Quoted 9 lines

Ingo reported following warning flooding the report out:

unwind: target platform=x86 is not supported

We trigger this warning when the dwarf unwinder is asked to process architecture which wasn't compiled in, like when you get 32bit application samples on your 64bit server and you don't have the 32bit remote unwind support compiled in.

Quoted 4 lines

This patch limits the warning to single message for arch, and adds bits info. Above message is changed to:

unwind: target platform=x86 32bit is not supported

Can we have a more informative message telling the user what is necessary to have this feature supported?

The way you phrased it looks like it is not supported at all, while what I read in the discussion is that one needs to have specific versions of support libraries installed to have this working, right?

- Arnaldo

Quoted 58 lines
---
 tools/perf/util/unwind-libunwind.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
tools/perf/util/unwind-libunwind.c +24 -5
index 6d542a4e0648..1439a6bcfa07 100644
@@ -1,12 +1,24 @@
+#include <linux/err.h>
 #include "unwind.h"
 #include "thread.h"
 #include "session.h"
 #include "debug.h"
 #include "arch/common.h"
 
-struct unwind_libunwind_ops __weak *local_unwind_libunwind_ops;
-struct unwind_libunwind_ops __weak *x86_32_unwind_libunwind_ops;
-struct unwind_libunwind_ops __weak *arm64_unwind_libunwind_ops;
+enum {
+	ERR_LOCAL  = 0,
+	ERR_X86_32 = 1,
+	ERR_ARM64  = 2,
+};
+
+/*
+ * Set default error values, so we can warn appropriately when
+ * the support is not compiled in. Using negative values so we
+ * can use ERR macros.
+ */
+struct unwind_libunwind_ops __weak *local_unwind_libunwind_ops  = (void *) -ERR_LOCAL;
+struct unwind_libunwind_ops __weak *x86_32_unwind_libunwind_ops = (void *) -ERR_X86_32;
+struct unwind_libunwind_ops __weak *arm64_unwind_libunwind_ops  = (void *) -ERR_ARM64;
 
 static void unwind__register_ops(struct thread *thread,
 			  struct unwind_libunwind_ops *ops)
@@ -48,8 +60,15 @@ int unwind__prepare_access(struct thread *thread, struct map *map,
 			ops = arm64_unwind_libunwind_ops;
 	}
 
-	if (!ops) {
-		pr_err("unwind: target platform=%s is not supported\n", arch);
+	if (IS_ERR(ops)) {
+		static unsigned long warned;
+		long bit = -PTR_ERR(ops);
+
+		if (!test_and_set_bit(bit, &warned)) {
+			pr_err("unwind: target platform=%s %dbit is not supported\n",
+				arch, dso_type == DSO__TYPE_64BIT ? 64 : 32);
+		}
+
 		return -1;
 	}
 out_register:
Signature · 1 line
2.9.4
#2 lore raw
Quoted 13 lines · Arnaldo Carvalho de Melo

Em Fri, Jun 16, 2017 at 02:12:53PM +0200, Jiri Olsa escreveu:

Quoted 12 lines

Ingo reported following warning flooding the report out:

unwind: target platform=x86 is not supported

We trigger this warning when the dwarf unwinder is asked to process architecture which wasn't compiled in, like when you get 32bit application samples on your 64bit server and you don't have the 32bit remote unwind support compiled in.

This patch limits the warning to single message for arch, and adds bits info. Above message is changed to:

Quoted 8 lines
Quoted 1 line

unwind: target platform=x86 32bit is not supported

Can we have a more informative message telling the user what is necessary to have this feature supported?

The way you phrased it looks like it is not supported at all, while what I read in the discussion is that one needs to have specific versions of support libraries installed to have this working, right?

exactly, I'll dig out from changelogs needed libunwind version and change the changelog in v2

jirka

A message that is not in the archive (sent off-list or not yet received): CA+JHD91HyFiNkRpJU+y1ppQO6t8KS3QQKEacNw_bu6s9PFH11w@mail.gmail.com

#4 lore raw
Quoted 29 lines · Arnaldo Carvalho de Melo

Thanks!

Quoted 25 lines · Jiri Olsa
Quoted 13 lines · Arnaldo Carvalho de Melo

Em Fri, Jun 16, 2017 at 02:12:53PM +0200, Jiri Olsa escreveu:

Quoted 12 lines

Ingo reported following warning flooding the report out:

unwind: target platform=x86 is not supported

We trigger this warning when the dwarf unwinder is asked to process architecture which wasn't compiled in, like when you get 32bit application samples on your 64bit server and you don't have the 32bit remote unwind support compiled in.

This patch limits the warning to single message for arch, and adds bits info. Above message is changed to:

Quoted 8 lines
Quoted 1 line

unwind: target platform=x86 32bit is not supported

Can we have a more informative message telling the user what is necessary to have this feature supported?

The way you phrased it looks like it is not supported at all, while what I read in the discussion is that one needs to have specific versions of support libraries installed to have this working, right?

exactly, I'll dig out from changelogs needed libunwind version and change the changelog in v2

He Kuang,
could you please give me details on how to enable x86 remote unwind on x86_64 server? can't find that in the libunwind's ./configure script

the feature tool detection example needs libunwind-x86.h header and -lunwind-x86.. which is provided by libunwind-devel.i686 pkg, but it conflicts with x86_64 libunwind-devel on fedora:

Error: Transaction check error:
file /usr/include/libunwind.h from install of libunwind-devel-1.2-1.fc25.i686 conflicts with file from package libunwind-devel-1.2-1.fc25.x86_64

jirka