[PATCH v5 0/5] Add support for remote unwind

[PATCH v5 0/5] Add support for remote unwind

[He Kuang]

v4 url:
  http://thread.gmane.org/gmane.linux.kernel/2224430

Currently, perf script uses host unwind methods(local unwind) to parse
perf.data callchain info regardless of the target architecture. So we
get wrong result and no promotion when do remote unwind on other
platforms/machines.

This patchset checks whether a dso is 32-bit or 64-bit according to
elf class info for each thread to let perf use the correct remote
unwind methods instead.

Only x86 and aarch64 is added in this patchset to show the work flow,
other platforms can be added easily.

We can see the right result for unwind info on different machines, for
example: perf.data recorded on i686 qemu with '-g' option and parsed
on x86_64 machine.

before this patchset:

  hello  1219 [001] 72190.667975: probe:sys_close: (c1169d60)
                  c1169d61 sys_close ([kernel.kallsyms])
                  c189c0d7 sysenter_past_esp ([kernel.kallsyms])
                  b777aba9 [unknown] ([vdso32])

after:
(Add vdso into buildid-cache first by 'perf buildid-cache -a' and
libraries are provided in symfs dir)

  hello  1219 [001] 72190.667975: probe:sys_close: (c1169d60)
                  c1169d61 sys_close ([kernel.kallsyms])
                  c189c0d7 sysenter_past_esp ([kernel.kallsyms])
                  b777aba9 __kernel_vsyscall ([vdso32])
                  b76971cc close (/lib/libc-2.22.so)
                   804842e fib (/tmp/hello)
                   804849d main (/tmp/hello)
                  b75d746e __libc_start_main (/lib/libc-2.22.so)
                   8048341 _start (/tmp/hello)

For using remote libunwind libraries, reference this:
  http://thread.gmane.org/gmane.linux.kernel/2224430

and now we can use LIBUNWIND_DIR to specific custom dirctories
containing libunwind libs.

v5:

 - Support LIBUNWIND_DIR args for detect remote libunwind libraries.
 - Change patch 2/5 commit messages for better understanding.
 - Self test for local (un)supported, remote un(supported) cases and
   fix some bugs in v4.

v4:

 - Move reference of buildid dir to 'symfs/.debug' if --symfs is
   given.
 - Split makefile changes out from patch 'Add support for
   cross-platform unwind'.
 - Use existing code normalize_arch() for testing the arch of
   perf.data.

v3:

 - Remove --vdso option, store vdso buildid in perf.data and let perf
   fetch it automatically.
 - Use existing dso__type() function to test if dso is 32-bit or
   64-bit.

v2:

 - Explain the reason why we can omit dwarf judgement when recording
   in commit message.
 - Elaborate on why we need to add a custom vdso path option, and
   change the type name to DSO_BINARY_TYPE__VDSO.
 - Hide the build tests status for cross platform unwind.
 - Keep generic version of libunwind-debug-frame test.
 - Put 32/64-bit test functions into separate patch.
 - Extract unwind related functions to unwind-libunwind.c and add new
   file for common parts used by both local and remote unwind.
 - Eliminate most of the ifdefs in .c file.

Thanks.

He Kuang (5):
  perf tools: Use LIBUNWIND_DIR for remote libunwind feature check
  perf tools: Show warnings for unsupported cross-platform unwind
  perf callchain: Add support for cross-platform unwind
  perf callchain: Support x86 target platform
  perf callchain: Support aarch64 cross-platform

 .../arch/arm64/include/libunwind/libunwind-arch.h  |  18 ++++
 tools/perf/arch/arm64/util/unwind-libunwind.c      |   5 +-
 tools/perf/arch/common.c                           |   2 +-
 tools/perf/arch/common.h                           |   1 +
 .../arch/x86/include/libunwind/libunwind-arch.h    |  18 ++++
 tools/perf/arch/x86/util/Build                     |   2 +-
 tools/perf/arch/x86/util/unwind-libunwind.c        |  19 +++-
 tools/perf/config/Makefile                         |  49 ++++++++-
 tools/perf/util/Build                              |  13 ++-
 tools/perf/util/thread.c                           |   7 +-
 tools/perf/util/thread.h                           |  17 +++-
 tools/perf/util/unwind-libunwind.c                 |  49 +++++++--
 tools/perf/util/unwind-libunwind_common.c          | 109 +++++++++++++++++++++
 tools/perf/util/unwind.h                           |  50 ++++++++--
 14 files changed, 323 insertions(+), 36 deletions(-)
 create mode 100644 tools/perf/arch/arm64/include/libunwind/libunwind-arch.h
 create mode 100644 tools/perf/arch/x86/include/libunwind/libunwind-arch.h
 create mode 100644 tools/perf/util/unwind-libunwind_common.c

-- 
1.8.5.2

[PATCH v5 1/5] perf tools: Use LIBUNWIND_DIR for remote libunwind feature check

[He Kuang]

Pass LIBUNWIND_DIR to feature check flags for remote libunwind
tests. So perf can be able to detect remote libunwind libraries from
arbitrary directory.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/config/Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 1e46277..6f9f566 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -67,9 +67,18 @@ endif
 #
 #   make DEBUG=1 LIBUNWIND_DIR=/opt/libunwind/
 #
+
+libunwind_arch_set_flags = $(eval $(libunwind_arch_set_flags_code))
+define libunwind_arch_set_flags_code
+  FEATURE_CHECK_CFLAGS-libunwind-$(1)  = -I$(LIBUNWIND_DIR)/include
+  FEATURE_CHECK_LDFLAGS-libunwind-$(1) = -L$(LIBUNWIND_DIR)/lib
+endef
+
 ifdef LIBUNWIND_DIR
   LIBUNWIND_CFLAGS  = -I$(LIBUNWIND_DIR)/include
   LIBUNWIND_LDFLAGS = -L$(LIBUNWIND_DIR)/lib
+  LIBUNWIND_ARCHS = x86 x86_64 arm aarch64 debug-frame-arm debug-frame-aarch64
+  $(foreach libunwind_arch,$(LIBUNWIND_ARCHS),$(call libunwind_arch_set_flags,$(libunwind_arch)))
 endif
 LIBUNWIND_LDFLAGS += $(LIBUNWIND_LIBS)
 
-- 
1.8.5.2

[PATCH v5 2/5] perf tools: Show warnings for unsupported cross-platform unwind

[He Kuang]

Currently, perf script uses host unwind methods to parse perf.data
callchain info regardless of the target architecture. So we get wrong
result without any warnings when unwinding callchains of x86(32-bit)
on x86(64-bit) machine.

This patch shows warning messages when we do remote unwind x86(32-bit)
on other machines. Same thing for other platforms will be added in
next patches.

Common functions which will be used by both local unwind and remote
unwind are separated into new file 'unwind-libunwind_common.c'.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/arch/common.c                  |  2 +-
 tools/perf/arch/common.h                  |  1 +
 tools/perf/config/Makefile                |  5 +++++
 tools/perf/util/Build                     |  1 +
 tools/perf/util/thread.c                  |  2 ++
 tools/perf/util/unwind-libunwind_common.c | 34 +++++++++++++++++++++++++++++++
 tools/perf/util/unwind.h                  |  5 +++++
 7 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/util/unwind-libunwind_common.c

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index e83c8ce..fa090a9 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -102,7 +102,7 @@ static int lookup_triplets(const char *const *triplets, const char *name)
  * Return architecture name in a normalized form.
  * The conversion logic comes from the Makefile.
  */
-static const char *normalize_arch(char *arch)
+const char *normalize_arch(char *arch)
 {
 	if (!strcmp(arch, "x86_64"))
 		return "x86";
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
index 7529cfb..6b01c73 100644
--- a/tools/perf/arch/common.h
+++ b/tools/perf/arch/common.h
@@ -6,5 +6,6 @@
 extern const char *objdump_path;
 
 int perf_env__lookup_objdump(struct perf_env *env);
+const char *normalize_arch(char *arch);
 
 #endif /* ARCH_PERF_COMMON_H */
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 6f9f566..c9e1625 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -354,6 +354,11 @@ ifeq ($(ARCH),powerpc)
 endif
 
 ifndef NO_LIBUNWIND
+  ifeq ($(feature-libunwind-x86), 1)
+    $(call detected,CONFIG_LIBUNWIND_X86)
+    CFLAGS += -DHAVE_LIBUNWIND_X86_SUPPORT
+  endif
+
   ifneq ($(feature-libunwind), 1)
     msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
     NO_LIBUNWIND := 1
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 8c6c8a0..25c31fb 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -100,6 +100,7 @@ libperf-$(CONFIG_DWARF) += dwarf-aux.o
 
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
+libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
 
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
 
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 45fcb71..3043113 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -205,6 +205,8 @@ void thread__insert_map(struct thread *thread, struct map *map)
 {
 	map_groups__fixup_overlappings(thread->mg, map, stderr);
 	map_groups__insert(thread->mg, map);
+
+	unwind__get_arch(thread, map);
 }
 
 static int thread__clone_map_groups(struct thread *thread,
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
new file mode 100644
index 0000000..3946c99
--- /dev/null
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -0,0 +1,34 @@
+#include "thread.h"
+#include "session.h"
+#include "unwind.h"
+#include "symbol.h"
+#include "debug.h"
+#include "arch/common.h"
+
+void unwind__get_arch(struct thread *thread, struct map *map)
+{
+	const char *arch;
+	enum dso_type dso_type;
+
+	if (!thread->mg->machine->env)
+		return;
+
+	dso_type = dso__type(map->dso, thread->mg->machine);
+	if (dso_type == DSO__TYPE_UNKNOWN)
+		return;
+
+	if (thread->addr_space)
+		pr_debug("unwind: thread map already set, 64bit is %d, dso=%s\n",
+			 dso_type == DSO__TYPE_64BIT, map->dso->name);
+
+	arch = normalize_arch(thread->mg->machine->env->arch);
+
+	if (!strcmp(arch, "x86")) {
+		if (dso_type != DSO__TYPE_64BIT)
+#ifdef HAVE_LIBUNWIND_X86_SUPPORT
+			pr_err("unwind: target platform=%s is not implemented\n", arch);
+#else
+			pr_err("unwind: target platform=%s is not supported\n", arch);
+#endif
+	}
+}
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 12790cf..889d630 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -24,6 +24,7 @@ int libunwind__arch_reg_id(int regnum);
 int unwind__prepare_access(struct thread *thread);
 void unwind__flush_access(struct thread *thread);
 void unwind__finish_access(struct thread *thread);
+void unwind__get_arch(struct thread *thread, struct map *map);
 #else
 static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
 {
@@ -32,6 +33,8 @@ static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
 
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
+static inline void unwind__get_arch(struct thread *thread __maybe_unused,
+				    struct map *map __maybe_unused) {}
 #endif
 #else
 static inline int
@@ -51,5 +54,7 @@ static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
 
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
+static inline void unwind__get_arch(struct thread *thread __maybe_unused,
+				    struct map *map __maybe_unused) {}
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
 #endif /* __UNWIND_H */
-- 
1.8.5.2

[PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[He Kuang]

Use thread specific unwind ops to unwind cross-platform callchains.

Currently, unwind methods is suitable for local unwind, this patch
changes the fixed methods to thread/map related. Each time a map is
inserted, we find the target arch and see if this platform can be
remote unwind. We test for x86 platform and only show proper
messages. The real unwind methods are not implemented, will be
introduced in next patch.

CONFIG_LIBUNWIND/NO_LIBUNWIND are changed to
CONFIG_LOCAL_LIBUNWIND/NO_LOCAL_LIBUNWIND for retaining local unwind
features. CONFIG_LIBUNWIND stands for either local or remote or both
unwind are supported and NO_LIBUNWIND means neither local nor remote
libunwind are supported.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/arch/x86/util/Build            |  2 +-
 tools/perf/config/Makefile                | 23 +++++++++-
 tools/perf/util/Build                     |  2 +-
 tools/perf/util/thread.c                  |  5 +--
 tools/perf/util/thread.h                  | 17 ++++++--
 tools/perf/util/unwind-libunwind.c        | 49 +++++++++++++++++----
 tools/perf/util/unwind-libunwind_common.c | 71 +++++++++++++++++++++++++++++--
 tools/perf/util/unwind.h                  | 37 +++++++++++-----
 8 files changed, 173 insertions(+), 33 deletions(-)

diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
index 4659703..bc24b75 100644
--- a/tools/perf/arch/x86/util/Build
+++ b/tools/perf/arch/x86/util/Build
@@ -7,7 +7,7 @@ libperf-y += perf_regs.o
 libperf-$(CONFIG_DWARF) += dwarf-regs.o
 libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
 
-libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
+libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 
 libperf-$(CONFIG_AUXTRACE) += auxtrace.o
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index c9e1625..8ac0440 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -354,15 +354,31 @@ ifeq ($(ARCH),powerpc)
 endif
 
 ifndef NO_LIBUNWIND
+  have_libunwind =
   ifeq ($(feature-libunwind-x86), 1)
     $(call detected,CONFIG_LIBUNWIND_X86)
     CFLAGS += -DHAVE_LIBUNWIND_X86_SUPPORT
+    LDFLAGS += -lunwind-x86
+    have_libunwind = 1
   endif
 
   ifneq ($(feature-libunwind), 1)
     msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
+    NO_LOCAL_LIBUNWIND := 1
+  else
+    have_libunwind = 1
+    CFLAGS += -DHAVE_LIBUNWIND_LOCAL_SUPPORT
+    $(call detected,CONFIG_LOCAL_LIBUNWIND)
+  endif
+
+  ifneq ($(have_libunwind), 1)
     NO_LIBUNWIND := 1
+  else
+    CFLAGS += -I$(LIBUNWIND_DIR)/include
+    LDFLAGS += -L$(LIBUNWIND_DIR)/lib
   endif
+else
+  NO_LOCAL_LIBUNWIND := 1
 endif
 
 ifndef NO_LIBBPF
@@ -400,7 +416,7 @@ else
   NO_DWARF_UNWIND := 1
 endif
 
-ifndef NO_LIBUNWIND
+ifndef NO_LOCAL_LIBUNWIND
   ifeq ($(ARCH),$(filter $(ARCH),arm arm64))
     $(call feature_check,libunwind-debug-frame)
     ifneq ($(feature-libunwind-debug-frame), 1)
@@ -411,12 +427,15 @@ ifndef NO_LIBUNWIND
     # non-ARM has no dwarf_find_debug_frame() function:
     CFLAGS += -DNO_LIBUNWIND_DEBUG_FRAME
   endif
-  CFLAGS  += -DHAVE_LIBUNWIND_SUPPORT
   EXTLIBS += $(LIBUNWIND_LIBS)
   CFLAGS  += $(LIBUNWIND_CFLAGS)
   LDFLAGS += $(LIBUNWIND_LDFLAGS)
 endif
 
+ifndef NO_LIBUNWIND
+  CFLAGS  += -DHAVE_LIBUNWIND_SUPPORT
+endif
+
 ifndef NO_LIBAUDIT
   ifneq ($(feature-libaudit), 1)
     msg := $(warning No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 25c31fb..ce69721 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -99,7 +99,7 @@ libperf-$(CONFIG_DWARF) += probe-finder.o
 libperf-$(CONFIG_DWARF) += dwarf-aux.o
 
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
-libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
+libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
 
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 3043113..4e1aaf5 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -43,9 +43,6 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		thread->cpu = -1;
 		INIT_LIST_HEAD(&thread->comm_list);
 
-		if (unwind__prepare_access(thread) < 0)
-			goto err_thread;
-
 		comm_str = malloc(32);
 		if (!comm_str)
 			goto err_thread;
@@ -59,6 +56,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		list_add(&comm->list, &thread->comm_list);
 		atomic_set(&thread->refcnt, 1);
 		RB_CLEAR_NODE(&thread->rb_node);
+
+		register_null_unwind_libunwind_ops(thread);
 	}
 
 	return thread;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 45fba13..647b011 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -9,12 +9,20 @@
 #include "symbol.h"
 #include <strlist.h>
 #include <intlist.h>
-#ifdef HAVE_LIBUNWIND_SUPPORT
-#include <libunwind.h>
-#endif
 
 struct thread_stack;
 
+struct unwind_entry;
+typedef int (*unwind_entry_cb_t)(struct unwind_entry *entry, void *arg);
+struct unwind_libunwind_ops {
+	int (*prepare_access)(struct thread *thread);
+	void (*flush_access)(struct thread *thread);
+	void (*finish_access)(struct thread *thread);
+	int (*get_entries)(unwind_entry_cb_t cb, void *arg,
+			   struct thread *thread,
+			   struct perf_sample *data, int max_stack);
+};
+
 struct thread {
 	union {
 		struct rb_node	 rb_node;
@@ -36,7 +44,8 @@ struct thread {
 	void			*priv;
 	struct thread_stack	*ts;
 #ifdef HAVE_LIBUNWIND_SUPPORT
-	unw_addr_space_t	addr_space;
+	void				*addr_space;
+	struct unwind_libunwind_ops	*unwind_libunwind_ops;
 #endif
 };
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 63687d3..a6ec587 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -22,8 +22,11 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <linux/list.h>
+#ifdef REMOTE_UNWIND_LIBUNWIND
+#include "libunwind-arch.h"
+#else
 #include <libunwind.h>
-#include <libunwind-ptrace.h>
+#endif
 #include "callchain.h"
 #include "thread.h"
 #include "session.h"
@@ -34,6 +37,20 @@
 #include "debug.h"
 #include "asm/bug.h"
 
+#ifndef REMOTE_UNWIND_LIBUNWIND
+  #define LOCAL_UNWIND_LIBUNWIND
+  #undef UNWT_OBJ
+  #define UNWT_OBJ(x) _##x
+#else
+  #undef NO_LIBUNWIND_DEBUG_FRAME
+  #if defined(LIBUNWIND_ARM) && !defined(NO_LIBUNWIND_DEBUG_FRAME_ARM)
+  #elif defined(LIBUNWIND_AARCH64) &&                    \
+	  defined(NO_LIBUNWIND_DEBUG_FRAME_ARM_AARCH64)
+  #else
+    #define NO_LIBUNWIND_DEBUG_FRAME
+  #endif
+#endif
+
 extern int
 UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,
 				    unw_word_t ip,
@@ -508,7 +525,7 @@ static int access_reg(unw_addr_space_t __maybe_unused as,
 		return 0;
 	}
 
-	id = libunwind__arch_reg_id(regnum);
+	id = LIBUNWIND__ARCH_REG_ID(regnum);
 	if (id < 0)
 		return -EINVAL;
 
@@ -579,7 +596,7 @@ static unw_accessors_t accessors = {
 	.get_proc_name		= get_proc_name,
 };
 
-int unwind__prepare_access(struct thread *thread)
+static int UNWT_OBJ(_unwind__prepare_access)(struct thread *thread)
 {
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return 0;
@@ -594,7 +611,7 @@ int unwind__prepare_access(struct thread *thread)
 	return 0;
 }
 
-void unwind__flush_access(struct thread *thread)
+static void UNWT_OBJ(_unwind__flush_access)(struct thread *thread)
 {
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
@@ -602,7 +619,7 @@ void unwind__flush_access(struct thread *thread)
 	unw_flush_cache(thread->addr_space, 0, 0);
 }
 
-void unwind__finish_access(struct thread *thread)
+static void UNWT_OBJ(_unwind__finish_access)(struct thread *thread)
 {
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
@@ -662,9 +679,10 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	return ret;
 }
 
-int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
-			struct thread *thread,
-			struct perf_sample *data, int max_stack)
+static int UNWT_OBJ(_unwind__get_entries)(unwind_entry_cb_t cb, void *arg,
+					 struct thread *thread,
+					 struct perf_sample *data,
+					 int max_stack)
 {
 	struct unwind_info ui = {
 		.sample       = data,
@@ -680,3 +698,18 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 
 	return get_entries(&ui, cb, arg, max_stack);
 }
+
+struct unwind_libunwind_ops
+UNWT_OBJ(unwind_libunwind_ops) = {
+	.prepare_access = UNWT_OBJ(_unwind__prepare_access),
+	.flush_access   = UNWT_OBJ(_unwind__flush_access),
+	.finish_access  = UNWT_OBJ(_unwind__finish_access),
+	.get_entries    = UNWT_OBJ(_unwind__get_entries),
+};
+
+#ifdef LOCAL_UNWIND_LIBUNWIND
+void register_local_unwind_libunwind_ops(struct thread *thread)
+{
+	thread->unwind_libunwind_ops = &UNWT_OBJ(unwind_libunwind_ops);
+}
+#endif
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
index 3946c99..f44833b 100644
--- a/tools/perf/util/unwind-libunwind_common.c
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -5,10 +5,64 @@
 #include "debug.h"
 #include "arch/common.h"
 
+static int __null__prepare_access(struct thread *thread __maybe_unused)
+{
+	return 0;
+}
+
+static void __null__flush_access(struct thread *thread __maybe_unused)
+{
+}
+
+static void __null__finish_access(struct thread *thread __maybe_unused)
+{
+}
+
+static int __null__get_entries(unwind_entry_cb_t cb __maybe_unused,
+			       void *arg __maybe_unused,
+			       struct thread *thread __maybe_unused,
+			       struct perf_sample *data __maybe_unused,
+			       int max_stack __maybe_unused)
+{
+	return 0;
+}
+
+static struct unwind_libunwind_ops null_unwind_libunwind_ops = {
+	.prepare_access = __null__prepare_access,
+	.flush_access   = __null__flush_access,
+	.finish_access  = __null__finish_access,
+	.get_entries    = __null__get_entries,
+};
+
+void register_null_unwind_libunwind_ops(struct thread *thread)
+{
+	thread->unwind_libunwind_ops = &null_unwind_libunwind_ops;
+	if (thread->mg)
+		pr_err("unwind: target platform=%s unwind unsupported\n",
+		       thread->mg->machine->env->arch);
+}
+
+void register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
+				   struct thread *thread)
+{
+	thread->unwind_libunwind_ops = ops;
+}
+
+void unwind__flush_access(struct thread *thread)
+{
+	thread->unwind_libunwind_ops->flush_access(thread);
+}
+
+void unwind__finish_access(struct thread *thread)
+{
+	thread->unwind_libunwind_ops->finish_access(thread);
+}
+
 void unwind__get_arch(struct thread *thread, struct map *map)
 {
 	const char *arch;
 	enum dso_type dso_type;
+	int use_local_unwind = 1;
 
 	if (!thread->mg->machine->env)
 		return;
@@ -17,18 +71,27 @@ void unwind__get_arch(struct thread *thread, struct map *map)
 	if (dso_type == DSO__TYPE_UNKNOWN)
 		return;
 
-	if (thread->addr_space)
+	if (thread->addr_space) {
 		pr_debug("unwind: thread map already set, 64bit is %d, dso=%s\n",
 			 dso_type == DSO__TYPE_64BIT, map->dso->name);
+		return;
+	}
 
 	arch = normalize_arch(thread->mg->machine->env->arch);
 
 	if (!strcmp(arch, "x86")) {
-		if (dso_type != DSO__TYPE_64BIT)
+		if (dso_type != DSO__TYPE_64BIT) {
 #ifdef HAVE_LIBUNWIND_X86_SUPPORT
 			pr_err("unwind: target platform=%s is not implemented\n", arch);
-#else
-			pr_err("unwind: target platform=%s is not supported\n", arch);
 #endif
+			register_null_unwind_libunwind_ops(thread);
+			use_local_unwind = 0;
+		}
 	}
+
+	if (use_local_unwind)
+		register_local_unwind_libunwind_ops(thread);
+
+	if (thread->unwind_libunwind_ops->prepare_access(thread) < 0)
+		return;
 }
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 889d630..e170be7 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -15,26 +15,46 @@ struct unwind_entry {
 typedef int (*unwind_entry_cb_t)(struct unwind_entry *entry, void *arg);
 
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
+
+#ifndef LIBUNWIND__ARCH_REG_ID
+#define LIBUNWIND__ARCH_REG_ID libunwind__arch_reg_id
+#endif
+
 int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 			struct thread *thread,
 			struct perf_sample *data, int max_stack);
 /* libunwind specific */
 #ifdef HAVE_LIBUNWIND_SUPPORT
 int libunwind__arch_reg_id(int regnum);
-int unwind__prepare_access(struct thread *thread);
 void unwind__flush_access(struct thread *thread);
 void unwind__finish_access(struct thread *thread);
 void unwind__get_arch(struct thread *thread, struct map *map);
-#else
-static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
-{
-	return 0;
+void register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
+				   struct thread *thread);
+void register_null_unwind_libunwind_ops(struct thread *thread);
+
+#ifndef HAVE_LIBUNWIND_LOCAL_SUPPORT
+static inline void
+register_local_unwind_libunwind_ops(struct thread *thread) {
+	register_null_unwind_libunwind_ops(thread);
 }
+#else
+void register_local_unwind_libunwind_ops(struct thread *thread);
+#endif
+
+#define unwind__get_entries(cb, arg,					\
+			    thread,					\
+			    data, max_stack)				\
+	thread->unwind_libunwind_ops->get_entries(cb, arg, thread,	\
+						  data, max_stack)
 
+#else
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__get_arch(struct thread *thread __maybe_unused,
 				    struct map *map __maybe_unused) {}
+static inline void
+register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #endif
 #else
 static inline int
@@ -47,14 +67,11 @@ unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
 	return 0;
 }
 
-static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
-{
-	return 0;
-}
-
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__get_arch(struct thread *thread __maybe_unused,
 				    struct map *map __maybe_unused) {}
+static inline void
+register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
 #endif /* __UNWIND_H */
-- 
1.8.5.2

[PATCH v5 4/5] perf callchain: Support x86 target platform

[He Kuang]

Support x86(32-bit) cross platform callchain unwind.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 .../perf/arch/x86/include/libunwind/libunwind-arch.h  | 18 ++++++++++++++++++
 tools/perf/arch/x86/util/unwind-libunwind.c           | 19 ++++++++++++++-----
 tools/perf/util/Build                                 |  6 ++++++
 tools/perf/util/unwind-libunwind_common.c             |  6 ++++--
 tools/perf/util/unwind.h                              |  5 +++++
 5 files changed, 47 insertions(+), 7 deletions(-)
 create mode 100644 tools/perf/arch/x86/include/libunwind/libunwind-arch.h

diff --git a/tools/perf/arch/x86/include/libunwind/libunwind-arch.h b/tools/perf/arch/x86/include/libunwind/libunwind-arch.h
new file mode 100644
index 0000000..be8c675
--- /dev/null
+++ b/tools/perf/arch/x86/include/libunwind/libunwind-arch.h
@@ -0,0 +1,18 @@
+#ifndef _LIBUNWIND_ARCH_H
+#define _LIBUNWIND_ARCH_H
+
+#include <libunwind-x86.h>
+#include <../perf_regs.h>
+#include <../../../../../../arch/x86/include/uapi/asm/perf_regs.h>
+
+#define LIBUNWIND_X86_32
+int libunwind__x86_reg_id(int regnum);
+
+#define LIBUNWIND__ARCH_REG_ID libunwind__x86_reg_id
+
+#include <../../../x86/util/unwind-libunwind.c>
+
+#define UNWT_PREFIX	UNW_PASTE(UNW_PASTE(_U, x86), _)
+#define UNWT_OBJ(fn)	UNW_PASTE(UNWT_PREFIX, fn)
+
+#endif /* _LIBUNWIND_ARCH_H */
diff --git a/tools/perf/arch/x86/util/unwind-libunwind.c b/tools/perf/arch/x86/util/unwind-libunwind.c
index db25e93..28831d8 100644
--- a/tools/perf/arch/x86/util/unwind-libunwind.c
+++ b/tools/perf/arch/x86/util/unwind-libunwind.c
@@ -1,12 +1,18 @@
 
 #include <errno.h>
+#if defined(LIBUNWIND_X86_32)
+#include <libunwind-x86.h>
+#elif defined(LIBUNWIND_X86_64)
+#include <libunwind-x86_64.h>
+#elif defined(HAVE_LIBUNWIND_LOCAL_SUPPORT)
 #include <libunwind.h>
+#endif
 #include "perf_regs.h"
 #include "../../util/unwind.h"
 #include "../../util/debug.h"
 
-#ifdef HAVE_ARCH_X86_64_SUPPORT
-int libunwind__arch_reg_id(int regnum)
+#if !defined(REMOTE_UNWIND_LIBUNWIND) && defined(HAVE_ARCH_X86_64_SUPPORT)
+int LIBUNWIND__ARCH_REG_ID(int regnum)
 {
 	int id;
 
@@ -69,8 +75,11 @@ int libunwind__arch_reg_id(int regnum)
 
 	return id;
 }
-#else
-int libunwind__arch_reg_id(int regnum)
+#endif
+
+#if !defined(REMOTE_UNWIND_LIBUNWIND) && !defined(HAVE_ARCH_X86_64_SUPPORT) || \
+	defined(LIBUNWIND_X86_32)
+int LIBUNWIND__ARCH_REG_ID(int regnum)
 {
 	int id;
 
@@ -109,4 +118,4 @@ int libunwind__arch_reg_id(int regnum)
 
 	return id;
 }
-#endif /* HAVE_ARCH_X86_64_SUPPORT */
+#endif
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index ce69721..2373130 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -1,3 +1,5 @@
+include ../scripts/Makefile.include
+
 libperf-y += alias.o
 libperf-y += annotate.o
 libperf-y += build-id.o
@@ -101,6 +103,10 @@ libperf-$(CONFIG_DWARF) += dwarf-aux.o
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
+libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
+
+$(OUTPUT)util/unwind-libunwind_x86_32.o: util/unwind-libunwind.c arch/x86/util/unwind-libunwind.c
+	$(QUIET_CC)$(CC) $(CFLAGS) -DREMOTE_UNWIND_LIBUNWIND -Iarch/x86/include/libunwind -c -o $@ util/unwind-libunwind.c
 
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
 
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
index f44833b..619c6c0 100644
--- a/tools/perf/util/unwind-libunwind_common.c
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -82,9 +82,11 @@ void unwind__get_arch(struct thread *thread, struct map *map)
 	if (!strcmp(arch, "x86")) {
 		if (dso_type != DSO__TYPE_64BIT) {
 #ifdef HAVE_LIBUNWIND_X86_SUPPORT
-			pr_err("unwind: target platform=%s is not implemented\n", arch);
-#endif
+			register_unwind_libunwind_ops(
+				&_Ux86_unwind_libunwind_ops, thread);
+#else
 			register_null_unwind_libunwind_ops(thread);
+#endif
 			use_local_unwind = 0;
 		}
 	}
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index e170be7..7dafb6e 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -56,6 +56,11 @@ static inline void unwind__get_arch(struct thread *thread __maybe_unused,
 static inline void
 register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #endif
+
+#ifdef HAVE_LIBUNWIND_X86_SUPPORT
+extern struct unwind_libunwind_ops _Ux86_unwind_libunwind_ops;
+#endif
+
 #else
 static inline int
 unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
-- 
1.8.5.2

[PATCH v5 5/5] perf callchain: Support aarch64 cross-platform

[He Kuang]

Support aarch64 cross platform callchain unwind.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 .../perf/arch/arm64/include/libunwind/libunwind-arch.h | 18 ++++++++++++++++++
 tools/perf/arch/arm64/util/unwind-libunwind.c          |  5 ++++-
 tools/perf/config/Makefile                             | 12 ++++++++++++
 tools/perf/util/Build                                  |  4 ++++
 tools/perf/util/unwind-libunwind_common.c              | 10 ++++++++++
 tools/perf/util/unwind.h                               |  3 +++
 6 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/arch/arm64/include/libunwind/libunwind-arch.h

diff --git a/tools/perf/arch/arm64/include/libunwind/libunwind-arch.h b/tools/perf/arch/arm64/include/libunwind/libunwind-arch.h
new file mode 100644
index 0000000..47d13a6
--- /dev/null
+++ b/tools/perf/arch/arm64/include/libunwind/libunwind-arch.h
@@ -0,0 +1,18 @@
+#ifndef _LIBUNWIND_ARCH_H
+#define _LIBUNWIND_ARCH_H
+
+#include <libunwind-aarch64.h>
+#include <../perf_regs.h>
+#include <../../../../../../arch/arm64/include/uapi/asm/perf_regs.h>
+
+#define LIBUNWIND_AARCH64
+int libunwind__aarch64_reg_id(int regnum);
+
+#define LIBUNWIND__ARCH_REG_ID libunwind__aarch64_reg_id
+
+#include <../../../arm64/util/unwind-libunwind.c>
+
+#define UNWT_PREFIX	UNW_PASTE(UNW_PASTE(_U, aarch64), _)
+#define UNWT_OBJ(fn)	UNW_PASTE(UNWT_PREFIX, fn)
+
+#endif /* _LIBUNWIND_ARCH_H */
diff --git a/tools/perf/arch/arm64/util/unwind-libunwind.c b/tools/perf/arch/arm64/util/unwind-libunwind.c
index a87afa9..5b557a5 100644
--- a/tools/perf/arch/arm64/util/unwind-libunwind.c
+++ b/tools/perf/arch/arm64/util/unwind-libunwind.c
@@ -1,11 +1,14 @@
 
 #include <errno.h>
-#include <libunwind.h>
 #include "perf_regs.h"
 #include "../../util/unwind.h"
 #include "../../util/debug.h"
 
+#ifndef LIBUNWIND_AARCH64
 int libunwind__arch_reg_id(int regnum)
+#else
+int libunwind__aarch64_reg_id(int regnum)
+#endif
 {
 	switch (regnum) {
 	case UNW_AARCH64_X0:
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 8ac0440..eb7cbce 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -362,6 +362,18 @@ ifndef NO_LIBUNWIND
     have_libunwind = 1
   endif
 
+  ifeq ($(feature-libunwind-aarch64), 1)
+    $(call detected,CONFIG_LIBUNWIND_AARCH64)
+    CFLAGS += -DHAVE_LIBUNWIND_AARCH64_SUPPORT
+    LDFLAGS += -lunwind-aarch64
+    have_libunwind = 1
+    $(call feature_check,libunwind-debug-frame-aarch64)
+    ifneq ($(feature-libunwind-debug-frame-aarch64), 1)
+      msg := $(warning No debug_frame support found in libunwind-aarch64);
+      CFLAGS += -DNO_LIBUNWIND_DEBUG_FRAME_AARCH64
+    endif
+  endif
+
   ifneq ($(feature-libunwind), 1)
     msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
     NO_LOCAL_LIBUNWIND := 1
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 2373130..f1b51a2 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -104,10 +104,14 @@ libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
 libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
+libperf-$(CONFIG_LIBUNWIND_AARCH64)  += unwind-libunwind_arm64.o
 
 $(OUTPUT)util/unwind-libunwind_x86_32.o: util/unwind-libunwind.c arch/x86/util/unwind-libunwind.c
 	$(QUIET_CC)$(CC) $(CFLAGS) -DREMOTE_UNWIND_LIBUNWIND -Iarch/x86/include/libunwind -c -o $@ util/unwind-libunwind.c
 
+$(OUTPUT)util/unwind-libunwind_arm64.o: util/unwind-libunwind.c arch/arm64/util/unwind-libunwind.c
+	$(QUIET_CC)$(CC) $(CFLAGS) -DREMOTE_UNWIND_LIBUNWIND -Iarch/arm64/include/libunwind -c -o $@ util/unwind-libunwind.c
+
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
 
 libperf-y += scripting-engines/
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
index 619c6c0..d19b062 100644
--- a/tools/perf/util/unwind-libunwind_common.c
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -89,6 +89,16 @@ void unwind__get_arch(struct thread *thread, struct map *map)
 #endif
 			use_local_unwind = 0;
 		}
+	} else if (!strcmp(arch, "arm64") || !strcmp(arch, "arm")) {
+		if (dso_type == DSO__TYPE_64BIT) {
+#ifdef HAVE_LIBUNWIND_AARCH64_SUPPORT
+			register_unwind_libunwind_ops(
+				&_Uaarch64_unwind_libunwind_ops, thread);
+#else
+			register_null_unwind_libunwind_ops(thread);
+#endif
+			use_local_unwind = 0;
+		}
 	}
 
 	if (use_local_unwind)
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 7dafb6e..359f756 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -60,6 +60,9 @@ register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #ifdef HAVE_LIBUNWIND_X86_SUPPORT
 extern struct unwind_libunwind_ops _Ux86_unwind_libunwind_ops;
 #endif
+#ifdef HAVE_LIBUNWIND_AARCH64_SUPPORT
+extern struct unwind_libunwind_ops _Uaarch64_unwind_libunwind_ops;
+#endif
 
 #else
 static inline int
-- 
1.8.5.2

Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[Jiri Olsa]

On Tue, May 24, 2016 at 09:20:27AM +0000, He Kuang wrote:
> Use thread specific unwind ops to unwind cross-platform callchains.
> 
> Currently, unwind methods is suitable for local unwind, this patch
> changes the fixed methods to thread/map related. Each time a map is
> inserted, we find the target arch and see if this platform can be
> remote unwind. We test for x86 platform and only show proper
> messages. The real unwind methods are not implemented, will be
> introduced in next patch.
> 
> CONFIG_LIBUNWIND/NO_LIBUNWIND are changed to
> CONFIG_LOCAL_LIBUNWIND/NO_LOCAL_LIBUNWIND for retaining local unwind
> features. CONFIG_LIBUNWIND stands for either local or remote or both
> unwind are supported and NO_LIBUNWIND means neither local nor remote
> libunwind are supported.

hi,
I think this is too complex and error prone, I'd rather see it split
to several pieces. Basically every logicaly single piece should be
in separate patch for better readablebility and review.

I might be missing some but I'd mainly sepatate following:

  - introducing struct unwind_libunwind_ops for local unwind
  - moving unwind__prepare_access from thread_new into thread__insert_map
  - keep unwind__prepare_access name instead of unwind__get_arch
    and keep the return value, we need to bail out in case of error
  - I wouldn't use null ops.. just check for for ops == NULL in wrapper function

  - I understand we need to compile 3 objects from unwind-libunwind.c,
    how about we create 3 files like:

    util/unwind-libunwind-local.c
    util/unwind-libunwind-x86_32.c
    util/unwind-libunwind-arm64.c

    which would setup all necessary defines and include unwind-libunwind.c like:

    ---
    /* comments explaining every define ;-) */
    ...
    #define LOCAL... REMOTE..
    ...
    #include <util/unwind-libunwind-local.c>
    ...
    ----

    this way we will keep all the special setup for given unwind object
    in one place and you can also use simple rule in the Build file like
    without defining special rule:

    libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
    libperf-$(CONFIG_LIBUNWIND_AARCH64)  += unwind-libunwind_arm64.o

    the same way for the arch object:

    arch/x86/util/unwind-libunwind-local.c
    arch/x86/util/unwind-libunwind-x86_32.c


Not sure I thought everything through, but I think this way
we'll keep it more maintainable and readable..

let me know what you think

thanks,
jirka

Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[Jiri Olsa]

On Tue, May 24, 2016 at 09:20:27AM +0000, He Kuang wrote:
> Use thread specific unwind ops to unwind cross-platform callchains.
> 
> Currently, unwind methods is suitable for local unwind, this patch
> changes the fixed methods to thread/map related. Each time a map is
> inserted, we find the target arch and see if this platform can be
> remote unwind. We test for x86 platform and only show proper
> messages. The real unwind methods are not implemented, will be
> introduced in next patch.
> 
> CONFIG_LIBUNWIND/NO_LIBUNWIND are changed to
> CONFIG_LOCAL_LIBUNWIND/NO_LOCAL_LIBUNWIND for retaining local unwind
> features. CONFIG_LIBUNWIND stands for either local or remote or both
> unwind are supported and NO_LIBUNWIND means neither local nor remote
> libunwind are supported.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/x86/util/Build            |  2 +-
>  tools/perf/config/Makefile                | 23 +++++++++-
>  tools/perf/util/Build                     |  2 +-
>  tools/perf/util/thread.c                  |  5 +--
>  tools/perf/util/thread.h                  | 17 ++++++--
>  tools/perf/util/unwind-libunwind.c        | 49 +++++++++++++++++----
>  tools/perf/util/unwind-libunwind_common.c | 71 +++++++++++++++++++++++++++++--
>  tools/perf/util/unwind.h                  | 37 +++++++++++-----
>  8 files changed, 173 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
> index 4659703..bc24b75 100644
> --- a/tools/perf/arch/x86/util/Build
> +++ b/tools/perf/arch/x86/util/Build
> @@ -7,7 +7,7 @@ libperf-y += perf_regs.o
>  libperf-$(CONFIG_DWARF) += dwarf-regs.o
>  libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
>  
> -libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
> +libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
>  libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
>  
>  libperf-$(CONFIG_AUXTRACE) += auxtrace.o
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index c9e1625..8ac0440 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -354,15 +354,31 @@ ifeq ($(ARCH),powerpc)
>  endif
>  
>  ifndef NO_LIBUNWIND
> +  have_libunwind =
>    ifeq ($(feature-libunwind-x86), 1)
>      $(call detected,CONFIG_LIBUNWIND_X86)
>      CFLAGS += -DHAVE_LIBUNWIND_X86_SUPPORT
> +    LDFLAGS += -lunwind-x86
> +    have_libunwind = 1
>    endif
>  
>    ifneq ($(feature-libunwind), 1)
>      msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
> +    NO_LOCAL_LIBUNWIND := 1
> +  else
> +    have_libunwind = 1
> +    CFLAGS += -DHAVE_LIBUNWIND_LOCAL_SUPPORT
> +    $(call detected,CONFIG_LOCAL_LIBUNWIND)
> +  endif
> +
> +  ifneq ($(have_libunwind), 1)
>      NO_LIBUNWIND := 1
> +  else
> +    CFLAGS += -I$(LIBUNWIND_DIR)/include
> +    LDFLAGS += -L$(LIBUNWIND_DIR)/lib
>    endif
> +else
> +  NO_LOCAL_LIBUNWIND := 1
>  endif
>  
>  ifndef NO_LIBBPF
> @@ -400,7 +416,7 @@ else
>    NO_DWARF_UNWIND := 1
>  endif
>  
> -ifndef NO_LIBUNWIND
> +ifndef NO_LOCAL_LIBUNWIND
>    ifeq ($(ARCH),$(filter $(ARCH),arm arm64))
>      $(call feature_check,libunwind-debug-frame)
>      ifneq ($(feature-libunwind-debug-frame), 1)
> @@ -411,12 +427,15 @@ ifndef NO_LIBUNWIND
>      # non-ARM has no dwarf_find_debug_frame() function:
>      CFLAGS += -DNO_LIBUNWIND_DEBUG_FRAME
>    endif
> -  CFLAGS  += -DHAVE_LIBUNWIND_SUPPORT
>    EXTLIBS += $(LIBUNWIND_LIBS)
>    CFLAGS  += $(LIBUNWIND_CFLAGS)
>    LDFLAGS += $(LIBUNWIND_LDFLAGS)
>  endif
>  
> +ifndef NO_LIBUNWIND
> +  CFLAGS  += -DHAVE_LIBUNWIND_SUPPORT
> +endif
> +
>  ifndef NO_LIBAUDIT
>    ifneq ($(feature-libaudit), 1)
>      msg := $(warning No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev);
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 25c31fb..ce69721 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -99,7 +99,7 @@ libperf-$(CONFIG_DWARF) += probe-finder.o
>  libperf-$(CONFIG_DWARF) += dwarf-aux.o
>  
>  libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
> -libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
> +libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
>  libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
>  
>  libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 3043113..4e1aaf5 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -43,9 +43,6 @@ struct thread *thread__new(pid_t pid, pid_t tid)
>  		thread->cpu = -1;
>  		INIT_LIST_HEAD(&thread->comm_list);
>  
> -		if (unwind__prepare_access(thread) < 0)
> -			goto err_thread;
> -
>  		comm_str = malloc(32);
>  		if (!comm_str)
>  			goto err_thread;
> @@ -59,6 +56,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)
>  		list_add(&comm->list, &thread->comm_list);
>  		atomic_set(&thread->refcnt, 1);
>  		RB_CLEAR_NODE(&thread->rb_node);
> +
> +		register_null_unwind_libunwind_ops(thread);
>  	}
>  
>  	return thread;
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 45fba13..647b011 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -9,12 +9,20 @@
>  #include "symbol.h"
>  #include <strlist.h>
>  #include <intlist.h>
> -#ifdef HAVE_LIBUNWIND_SUPPORT
> -#include <libunwind.h>
> -#endif
>  
>  struct thread_stack;
>  
> +struct unwind_entry;
> +typedef int (*unwind_entry_cb_t)(struct unwind_entry *entry, void *arg);
> +struct unwind_libunwind_ops {
> +	int (*prepare_access)(struct thread *thread);
> +	void (*flush_access)(struct thread *thread);
> +	void (*finish_access)(struct thread *thread);
> +	int (*get_entries)(unwind_entry_cb_t cb, void *arg,
> +			   struct thread *thread,
> +			   struct perf_sample *data, int max_stack);
> +};
> +
>  struct thread {
>  	union {
>  		struct rb_node	 rb_node;
> @@ -36,7 +44,8 @@ struct thread {
>  	void			*priv;
>  	struct thread_stack	*ts;
>  #ifdef HAVE_LIBUNWIND_SUPPORT
> -	unw_addr_space_t	addr_space;
> +	void				*addr_space;
> +	struct unwind_libunwind_ops	*unwind_libunwind_ops;
>  #endif
>  };
>  
> diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
> index 63687d3..a6ec587 100644
> --- a/tools/perf/util/unwind-libunwind.c
> +++ b/tools/perf/util/unwind-libunwind.c
> @@ -22,8 +22,11 @@
>  #include <unistd.h>
>  #include <sys/mman.h>
>  #include <linux/list.h>
> +#ifdef REMOTE_UNWIND_LIBUNWIND
> +#include "libunwind-arch.h"
> +#else
>  #include <libunwind.h>
> -#include <libunwind-ptrace.h>
> +#endif
>  #include "callchain.h"
>  #include "thread.h"
>  #include "session.h"
> @@ -34,6 +37,20 @@
>  #include "debug.h"
>  #include "asm/bug.h"
>  
> +#ifndef REMOTE_UNWIND_LIBUNWIND
> +  #define LOCAL_UNWIND_LIBUNWIND
> +  #undef UNWT_OBJ
> +  #define UNWT_OBJ(x) _##x
> +#else
> +  #undef NO_LIBUNWIND_DEBUG_FRAME
> +  #if defined(LIBUNWIND_ARM) && !defined(NO_LIBUNWIND_DEBUG_FRAME_ARM)
> +  #elif defined(LIBUNWIND_AARCH64) &&                    \
> +	  defined(NO_LIBUNWIND_DEBUG_FRAME_ARM_AARCH64)
> +  #else
> +    #define NO_LIBUNWIND_DEBUG_FRAME
> +  #endif
> +#endif
> +
>  extern int
>  UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,
>  				    unw_word_t ip,
> @@ -508,7 +525,7 @@ static int access_reg(unw_addr_space_t __maybe_unused as,
>  		return 0;
>  	}
>  
> -	id = libunwind__arch_reg_id(regnum);
> +	id = LIBUNWIND__ARCH_REG_ID(regnum);
>  	if (id < 0)
>  		return -EINVAL;
>  
> @@ -579,7 +596,7 @@ static unw_accessors_t accessors = {
>  	.get_proc_name		= get_proc_name,
>  };
>  
> -int unwind__prepare_access(struct thread *thread)
> +static int UNWT_OBJ(_unwind__prepare_access)(struct thread *thread)
>  {
>  	if (callchain_param.record_mode != CALLCHAIN_DWARF)
>  		return 0;
> @@ -594,7 +611,7 @@ int unwind__prepare_access(struct thread *thread)
>  	return 0;
>  }
>  
> -void unwind__flush_access(struct thread *thread)
> +static void UNWT_OBJ(_unwind__flush_access)(struct thread *thread)
>  {
>  	if (callchain_param.record_mode != CALLCHAIN_DWARF)
>  		return;
> @@ -602,7 +619,7 @@ void unwind__flush_access(struct thread *thread)
>  	unw_flush_cache(thread->addr_space, 0, 0);
>  }
>  
> -void unwind__finish_access(struct thread *thread)
> +static void UNWT_OBJ(_unwind__finish_access)(struct thread *thread)
>  {
>  	if (callchain_param.record_mode != CALLCHAIN_DWARF)
>  		return;
> @@ -662,9 +679,10 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
>  	return ret;
>  }
>  
> -int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
> -			struct thread *thread,
> -			struct perf_sample *data, int max_stack)
> +static int UNWT_OBJ(_unwind__get_entries)(unwind_entry_cb_t cb, void *arg,
> +					 struct thread *thread,
> +					 struct perf_sample *data,
> +					 int max_stack)
>  {
>  	struct unwind_info ui = {
>  		.sample       = data,
> @@ -680,3 +698,18 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
>  
>  	return get_entries(&ui, cb, arg, max_stack);
>  }
> +
> +struct unwind_libunwind_ops
> +UNWT_OBJ(unwind_libunwind_ops) = {
> +	.prepare_access = UNWT_OBJ(_unwind__prepare_access),
> +	.flush_access   = UNWT_OBJ(_unwind__flush_access),
> +	.finish_access  = UNWT_OBJ(_unwind__finish_access),
> +	.get_entries    = UNWT_OBJ(_unwind__get_entries),
> +};
> +
> +#ifdef LOCAL_UNWIND_LIBUNWIND
> +void register_local_unwind_libunwind_ops(struct thread *thread)
> +{
> +	thread->unwind_libunwind_ops = &UNWT_OBJ(unwind_libunwind_ops);
> +}
> +#endif
> diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
> index 3946c99..f44833b 100644
> --- a/tools/perf/util/unwind-libunwind_common.c
> +++ b/tools/perf/util/unwind-libunwind_common.c
> @@ -5,10 +5,64 @@
>  #include "debug.h"
>  #include "arch/common.h"
>  
> +static int __null__prepare_access(struct thread *thread __maybe_unused)
> +{
> +	return 0;
> +}
> +
> +static void __null__flush_access(struct thread *thread __maybe_unused)
> +{
> +}
> +
> +static void __null__finish_access(struct thread *thread __maybe_unused)
> +{
> +}
> +
> +static int __null__get_entries(unwind_entry_cb_t cb __maybe_unused,
> +			       void *arg __maybe_unused,
> +			       struct thread *thread __maybe_unused,
> +			       struct perf_sample *data __maybe_unused,
> +			       int max_stack __maybe_unused)
> +{
> +	return 0;
> +}
> +
> +static struct unwind_libunwind_ops null_unwind_libunwind_ops = {
> +	.prepare_access = __null__prepare_access,
> +	.flush_access   = __null__flush_access,
> +	.finish_access  = __null__finish_access,
> +	.get_entries    = __null__get_entries,
> +};
> +
> +void register_null_unwind_libunwind_ops(struct thread *thread)
> +{
> +	thread->unwind_libunwind_ops = &null_unwind_libunwind_ops;
> +	if (thread->mg)
> +		pr_err("unwind: target platform=%s unwind unsupported\n",
> +		       thread->mg->machine->env->arch);
> +}
> +
> +void register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
> +				   struct thread *thread)
> +{
> +	thread->unwind_libunwind_ops = ops;
> +}
> +
> +void unwind__flush_access(struct thread *thread)
> +{
> +	thread->unwind_libunwind_ops->flush_access(thread);
> +}
> +
> +void unwind__finish_access(struct thread *thread)
> +{
> +	thread->unwind_libunwind_ops->finish_access(thread);
> +}
> +
>  void unwind__get_arch(struct thread *thread, struct map *map)
>  {
>  	const char *arch;
>  	enum dso_type dso_type;
> +	int use_local_unwind = 1;
>  
>  	if (!thread->mg->machine->env)
>  		return;
> @@ -17,18 +71,27 @@ void unwind__get_arch(struct thread *thread, struct map *map)
>  	if (dso_type == DSO__TYPE_UNKNOWN)
>  		return;
>  
> -	if (thread->addr_space)
> +	if (thread->addr_space) {
>  		pr_debug("unwind: thread map already set, 64bit is %d, dso=%s\n",
>  			 dso_type == DSO__TYPE_64BIT, map->dso->name);
> +		return;
> +	}
>  
>  	arch = normalize_arch(thread->mg->machine->env->arch);
>  
>  	if (!strcmp(arch, "x86")) {
> -		if (dso_type != DSO__TYPE_64BIT)
> +		if (dso_type != DSO__TYPE_64BIT) {
>  #ifdef HAVE_LIBUNWIND_X86_SUPPORT
>  			pr_err("unwind: target platform=%s is not implemented\n", arch);
> -#else
> -			pr_err("unwind: target platform=%s is not supported\n", arch);
>  #endif
> +			register_null_unwind_libunwind_ops(thread);
> +			use_local_unwind = 0;
> +		}
>  	}
> +
> +	if (use_local_unwind)
> +		register_local_unwind_libunwind_ops(thread);
> +
> +	if (thread->unwind_libunwind_ops->prepare_access(thread) < 0)
> +		return;
>  }
> diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
> index 889d630..e170be7 100644
> --- a/tools/perf/util/unwind.h
> +++ b/tools/perf/util/unwind.h
> @@ -15,26 +15,46 @@ struct unwind_entry {
>  typedef int (*unwind_entry_cb_t)(struct unwind_entry *entry, void *arg);
>  
>  #ifdef HAVE_DWARF_UNWIND_SUPPORT
> +
> +#ifndef LIBUNWIND__ARCH_REG_ID
> +#define LIBUNWIND__ARCH_REG_ID libunwind__arch_reg_id
> +#endif
> +
>  int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
>  			struct thread *thread,
>  			struct perf_sample *data, int max_stack);
>  /* libunwind specific */
>  #ifdef HAVE_LIBUNWIND_SUPPORT
>  int libunwind__arch_reg_id(int regnum);
> -int unwind__prepare_access(struct thread *thread);
>  void unwind__flush_access(struct thread *thread);
>  void unwind__finish_access(struct thread *thread);
>  void unwind__get_arch(struct thread *thread, struct map *map);
> -#else
> -static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
> -{
> -	return 0;
> +void register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
> +				   struct thread *thread);
> +void register_null_unwind_libunwind_ops(struct thread *thread);
> +
> +#ifndef HAVE_LIBUNWIND_LOCAL_SUPPORT
> +static inline void
> +register_local_unwind_libunwind_ops(struct thread *thread) {
> +	register_null_unwind_libunwind_ops(thread);
>  }
> +#else
> +void register_local_unwind_libunwind_ops(struct thread *thread);
> +#endif
> +
> +#define unwind__get_entries(cb, arg,					\
> +			    thread,					\
> +			    data, max_stack)				\
> +	thread->unwind_libunwind_ops->get_entries(cb, arg, thread,	\
> +						  data, max_stack)

I think this should go along with other ops wrappers like
unwind__flush_access/unwind__finish_access.. in unwind-libunwind_common.c

jirka

Re: [PATCH v5 4/5] perf callchain: Support x86 target platform

[Jiri Olsa]

On Tue, May 24, 2016 at 09:20:28AM +0000, He Kuang wrote:
> Support x86(32-bit) cross platform callchain unwind.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  .../perf/arch/x86/include/libunwind/libunwind-arch.h  | 18 ++++++++++++++++++
>  tools/perf/arch/x86/util/unwind-libunwind.c           | 19 ++++++++++++++-----
>  tools/perf/util/Build                                 |  6 ++++++
>  tools/perf/util/unwind-libunwind_common.c             |  6 ++++--
>  tools/perf/util/unwind.h                              |  5 +++++
>  5 files changed, 47 insertions(+), 7 deletions(-)
>  create mode 100644 tools/perf/arch/x86/include/libunwind/libunwind-arch.h
> 
> diff --git a/tools/perf/arch/x86/include/libunwind/libunwind-arch.h b/tools/perf/arch/x86/include/libunwind/libunwind-arch.h
> new file mode 100644
> index 0000000..be8c675
> --- /dev/null
> +++ b/tools/perf/arch/x86/include/libunwind/libunwind-arch.h
> @@ -0,0 +1,18 @@
> +#ifndef _LIBUNWIND_ARCH_H
> +#define _LIBUNWIND_ARCH_H
> +
> +#include <libunwind-x86.h>
> +#include <../perf_regs.h>
> +#include <../../../../../../arch/x86/include/uapi/asm/perf_regs.h>
> +
> +#define LIBUNWIND_X86_32
> +int libunwind__x86_reg_id(int regnum);
> +
> +#define LIBUNWIND__ARCH_REG_ID libunwind__x86_reg_id
> +
> +#include <../../../x86/util/unwind-libunwind.c>
> +
> +#define UNWT_PREFIX	UNW_PASTE(UNW_PASTE(_U, x86), _)
> +#define UNWT_OBJ(fn)	UNW_PASTE(UNWT_PREFIX, fn)

is there a reason for using libunwind define for the symbol prefix?
what's the '_U' and all those '__' for? why dont we use simple macro
for arch prefix?

this explanation would be great to have in those wrapper obects
I meantioned in earlier email:
    util/unwind-libunwind-local.c
    util/unwind-libunwind-x86_32.c
    util/unwind-libunwind-arm64.c


thanks,
jirka

Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[Hekuang]

hi

在 2016/5/27 1:42, Jiri Olsa 写道:
> On Tue, May 24, 2016 at 09:20:27AM +0000, He Kuang wrote:
>> Use thread specific unwind ops to unwind cross-platform callchains.
>>
>> Currently, unwind methods is suitable for local unwind, this patch
>> changes the fixed methods to thread/map related. Each time a map is
>> inserted, we find the target arch and see if this platform can be
>> remote unwind. We test for x86 platform and only show proper
>> messages. The real unwind methods are not implemented, will be
>> introduced in next patch.
>>
>> CONFIG_LIBUNWIND/NO_LIBUNWIND are changed to
>> CONFIG_LOCAL_LIBUNWIND/NO_LOCAL_LIBUNWIND for retaining local unwind
>> features. CONFIG_LIBUNWIND stands for either local or remote or both
>> unwind are supported and NO_LIBUNWIND means neither local nor remote
>> libunwind are supported.
> hi,
> I think this is too complex and error prone, I'd rather see it split
> to several pieces. Basically every logicaly single piece should be
> in separate patch for better readablebility and review.
>
> I might be missing some but I'd mainly sepatate following:
>
>    - introducing struct unwind_libunwind_ops for local unwind
>    - moving unwind__prepare_access from thread_new into thread__insert_map
>    - keep unwind__prepare_access name instead of unwind__get_arch
>      and keep the return value, we need to bail out in case of error
>    - I wouldn't use null ops.. just check for for ops == NULL in wrapper function

OK
>    - I understand we need to compile 3 objects from unwind-libunwind.c,
>      how about we create 3 files like:
>
>      util/unwind-libunwind-local.c
>      util/unwind-libunwind-x86_32.c
>      util/unwind-libunwind-arm64.c
>
>      which would setup all necessary defines and include unwind-libunwind.c like:
>
>      ---
>      /* comments explaining every define ;-) */
>      ...
>      #define LOCAL... REMOTE..
>      ...
>      #include <util/unwind-libunwind-local.c>
>      ...
>      ----
>
>      this way we will keep all the special setup for given unwind object
>      in one place and you can also use simple rule in the Build file like
>      without defining special rule:
>
>      libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
>      libperf-$(CONFIG_LIBUNWIND_AARCH64)  += unwind-libunwind_arm64.o
>
>      the same way for the arch object:
>
>      arch/x86/util/unwind-libunwind-local.c
>      arch/x86/util/unwind-libunwind-x86_32.c
>
>
> Not sure I thought everything through, but I think this way
> we'll keep it more maintainable and readable..
>
> let me know what you think

The only concern is that, if later we support more platforms,
there will be too much files named as 'tools/perf/util/unwind-libunwind*.c'
Is it acceptable or not?

And I thought all files belongs to specific archs should
go to folder under 'tools/perf/arch/xxx', is that right?

Thanks.
> thanks,
> jirka
>

Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[Jiri Olsa]

On Fri, May 27, 2016 at 03:13:04PM +0800, Hekuang wrote:

SNIP

> >    - I understand we need to compile 3 objects from unwind-libunwind.c,
> >      how about we create 3 files like:
> > 
> >      util/unwind-libunwind-local.c
> >      util/unwind-libunwind-x86_32.c
> >      util/unwind-libunwind-arm64.c
> > 
> >      which would setup all necessary defines and include unwind-libunwind.c like:
> > 
> >      ---
> >      /* comments explaining every define ;-) */
> >      ...
> >      #define LOCAL... REMOTE..
> >      ...
> >      #include <util/unwind-libunwind-local.c>
> >      ...
> >      ----
> > 
> >      this way we will keep all the special setup for given unwind object
> >      in one place and you can also use simple rule in the Build file like
> >      without defining special rule:
> > 
> >      libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
> >      libperf-$(CONFIG_LIBUNWIND_AARCH64)  += unwind-libunwind_arm64.o
> > 
> >      the same way for the arch object:
> > 
> >      arch/x86/util/unwind-libunwind-local.c
> >      arch/x86/util/unwind-libunwind-x86_32.c
> > 
> > 
> > Not sure I thought everything through, but I think this way
> > we'll keep it more maintainable and readable..
> > 
> > let me know what you think
> 
> The only concern is that, if later we support more platforms,
> there will be too much files named as 'tools/perf/util/unwind-libunwind*.c'
> Is it acceptable or not?
> 
> And I thought all files belongs to specific archs should
> go to folder under 'tools/perf/arch/xxx', is that right?

hum, I wouldn't worry about that.. but you're right,
let's put them under arch

thanks,
jirka

Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[Hekuang]

在 2016/5/27 15:38, Jiri Olsa 写道:
> On Fri, May 27, 2016 at 03:13:04PM +0800, Hekuang wrote:
>
> SNIP
>
>>>     - I understand we need to compile 3 objects from unwind-libunwind.c,
>>>       how about we create 3 files like:
>>>
>>>       util/unwind-libunwind-local.c
>>>       util/unwind-libunwind-x86_32.c
>>>       util/unwind-libunwind-arm64.c
>>>
>>>       which would setup all necessary defines and include unwind-libunwind.c like:
>>>
>>>       ---
>>>       /* comments explaining every define ;-) */
>>>       ...
>>>       #define LOCAL... REMOTE..
>>>       ...
>>>       #include <util/unwind-libunwind-local.c>
>>>       ...
>>>       ----
>>>
>>>       this way we will keep all the special setup for given unwind object
>>>       in one place and you can also use simple rule in the Build file like
>>>       without defining special rule:
>>>
>>>       libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
>>>       libperf-$(CONFIG_LIBUNWIND_AARCH64)  += unwind-libunwind_arm64.o
>>>
>>>       the same way for the arch object:
>>>
>>>       arch/x86/util/unwind-libunwind-local.c
>>>       arch/x86/util/unwind-libunwind-x86_32.c
>>>
>>>
>>> Not sure I thought everything through, but I think this way
>>> we'll keep it more maintainable and readable..
>>>
>>> let me know what you think
>> The only concern is that, if later we support more platforms,
>> there will be too much files named as 'tools/perf/util/unwind-libunwind*.c'
>> Is it acceptable or not?
>>
>> And I thought all files belongs to specific archs should
>> go to folder under 'tools/perf/arch/xxx', is that right?
> hum, I wouldn't worry about that.. but you're right,
> let's put them under arch

But only 'tools/perf/arch/$(host platform)' will be built, in our case,
we should built the unwind-libunwind-$(arch) as long as we have
the remote libunwind libraries. So, I think there's a conflict in the
existing build script and not easy to 'put them under arch'. That's why
I choose a complex way in my previous patch.

Do you have some suggestions?
>
> thanks,
> jirka
>

Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

[Jiri Olsa]

On Fri, May 27, 2016 at 04:02:59PM +0800, Hekuang wrote:

SNIP

> > > The only concern is that, if later we support more platforms,
> > > there will be too much files named as 'tools/perf/util/unwind-libunwind*.c'
> > > Is it acceptable or not?
> > > 
> > > And I thought all files belongs to specific archs should
> > > go to folder under 'tools/perf/arch/xxx', is that right?
> > hum, I wouldn't worry about that.. but you're right,
> > let's put them under arch
> 
> But only 'tools/perf/arch/$(host platform)' will be built, in our case,
> we should built the unwind-libunwind-$(arch) as long as we have
> the remote libunwind libraries. So, I think there's a conflict in the
> existing build script and not easy to 'put them under arch'. That's why
> I choose a complex way in my previous patch.

we dive into arch dirs via:

arch/Build:
  libperf-y += $(ARCH)/

that's not changed AFAICS..

jirka