Re: [PATCH 6/8] perf callchain: Add support for cross-platform unwind

From: Arnaldo Carvalho de Melo
Date: Fri May 06 2016 - 07:56:30 EST

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH 0/8] Add support for remote unwind"
Previous message: Thomas Gleixner: "Re: [PATCH 0/6] Intel Secure Guard Extensions"
In reply to: He Kuang: "[PATCH 6/8] perf callchain: Add support for cross-platform unwind"
Next in thread: He Kuang: "[PATCH 4/8] perf build: Add build-test for debug-frame on arm/arm64"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Fri, May 06, 2016 at 08:59:12AM +0000, He Kuang escreveu:
> Use thread specific unwind ops to unwind cross-platform callchains.
>
> Before this patch, 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. In this patch, we test for x86 platform and only show
> proper messages. The real unwind methods are not implemented, will be
> introduced in next patch.
>
> Signed-off-by: He Kuang <hekuang@xxxxxxxxxx>
> ---
> tools/perf/config/Makefile | 19 ++++++++--
> tools/perf/util/Build | 3 +-
> tools/perf/util/thread.c | 29 +++++++++++----
> tools/perf/util/thread.h | 14 +++++++-
> tools/perf/util/unwind-libunwind.c | 48 +++++++++++++++++++++----
> tools/perf/util/unwind-libunwind_common.c | 60 +++++++++++++++++++++++++++++++
> tools/perf/util/unwind.h | 22 ++++++++++++
> 7 files changed, 178 insertions(+), 17 deletions(-)
> create mode 100644 tools/perf/util/unwind-libunwind_common.c
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index a86b864..16f14b1 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -345,14 +345,24 @@ ifeq ($(ARCH),powerpc)
> endif
>
> ifndef NO_LIBUNWIND
> + have_libunwind =
> ifeq ($(feature-libunwind-x86), 1)
> - LIBUNWIND_LIBS += -lunwind-x86
> $(call detected,CONFIG_LIBUNWIND_X86)
> CFLAGS += -DHAVE_LIBUNWIND_X86_SUPPORT
> + LDFLAGS += -lunwind -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
> endif
> endif
> @@ -392,7 +402,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)
> @@ -403,12 +413,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 ea4ac03..2e21529 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -97,7 +97,8 @@ 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 e0cdcf7..cf60db1 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -41,9 +41,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;
> @@ -57,6 +54,9 @@ 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);
> +#ifdef HAVE_LIBUNWIND_SUPPORT
> + register_null_unwind_libunwind_ops(thread);
> +#endif

Could you please avoid having all those ifdefs sprinkled in the .c file?
For instance, the above ifdef/endif should be dropped and instead, at a
central place, a header probably, or at most at the start of the .c
file, you should have one ifdef block, the else should just make the
above function, for instance, be:

#ifdef HAVE_LIBUNWIND_SUPPORT
void register_null_unwind_libunwind_ops(struct thread *thread);
#else
static inline
void register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused)
{
}
#endif

> }
>
> return thread;
> @@ -82,7 +82,9 @@ void thread__delete(struct thread *thread)
> list_del(&comm->list);
> comm__free(comm);
> }
> - unwind__finish_access(thread);
> +#ifdef HAVE_LIBUNWIND_SUPPORT
> + thread->unwind_libunwind_ops->finish_access(thread);
> +#endif
>
> free(thread);
> }
> @@ -144,8 +146,10 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
> return -ENOMEM;
> list_add(&new->list, &thread->comm_list);
>
> +#ifdef HAVE_LIBUNWIND_SUPPORT
> if (exec)
> - unwind__flush_access(thread);
> + thread->unwind_libunwind_ops->flush_access(thread);
> +#endif

You see, here the original code should be left untouched and you would
change unwind__flush_access() instead to be:

#ifdef HAVE_LIBUNWIND_SUPPORT
void unwind__flush_access(struct thread *thread)
{
thread->unwind_libunwind_ops->flush_access(thread);
}
#else
void unwind__flush_access(struct thread *thread __maybe_unused)
{
}
#endif

combine the above if/else/endif with the previous and with all the
others, please.

> }
>
> thread->comm_set = true;
> @@ -208,14 +212,27 @@ void thread__insert_map(struct thread *thread, struct map *map)
> || !strcmp(arch, "x86")
> || !strcmp(arch, "i686")) {
> pr_debug("Thread map is X86, 64bit is %d\n", is_64_bit);
> - if (!is_64_bit)
> + if (!is_64_bit) {
> #ifdef HAVE_LIBUNWIND_X86_SUPPORT
> pr_err("target platform=%s is not implemented!\n",
> arch);
> #else
> pr_err("target platform=%s is not supported!\n", arch);
> #endif
> + goto err;
> + }
> + } else {
> + register_local_unwind_libunwind_ops(thread);
> }
> +
> + if (thread->unwind_libunwind_ops->prepare_access(thread) < 0)
> + return;
> +
> + return;
> +
> +err: __maybe_unused
> + register_null_unwind_libunwind_ops(thread);
> + return;
> #endif
> }
>
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index e214207..6f2d4cd 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -15,6 +15,17 @@
>
> 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 +47,8 @@ struct thread {
> void *priv;
> struct thread_stack *ts;
> #ifdef HAVE_LIBUNWIND_SUPPORT
> - unw_addr_space_t addr_space;
> + unw_addr_space_t 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..2558bf3 100644
> --- a/tools/perf/util/unwind-libunwind.c
> +++ b/tools/perf/util/unwind-libunwind.c
> @@ -22,6 +22,9 @@
> #include <unistd.h>
> #include <sys/mman.h>
> #include <linux/list.h>
> +#ifdef ARCH_UNWIND_LIBUNWIND
> +#include "libunwind-arch.h"
> +#endif
> #include <libunwind.h>
> #include <libunwind-ptrace.h>
> #include "callchain.h"
> @@ -34,6 +37,22 @@
> #include "debug.h"
> #include "asm/bug.h"
>
> +#ifndef ARCH_UNWIND_LIBUNWIND
> + #define LIBUNWIND__ARCH_REG_ID libunwind__arch_reg_id
> + #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,
> @@ -579,7 +598,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 +613,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 +621,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 +681,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 +700,19 @@ 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
> +int register_local_unwind_libunwind_ops(struct thread *thread)
> +{
> + thread->unwind_libunwind_ops = &UNWT_OBJ(unwind_libunwind_ops);
> + return 0;
> +}
> +#endif
> diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
> new file mode 100644
> index 0000000..47433a4
> --- /dev/null
> +++ b/tools/perf/util/unwind-libunwind_common.c
> @@ -0,0 +1,60 @@
> +#include <elf.h>
> +#include <gelf.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +#include <linux/list.h>
> +#include <libunwind.h>
> +#include <libunwind-ptrace.h>
> +#include "callchain.h"
> +#include "thread.h"
> +#include "session.h"
> +#include "perf_regs.h"
> +#include "unwind.h"
> +#include "symbol.h"
> +#include "util.h"
> +#include "debug.h"
> +#include "asm/bug.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,
> +};
> +
> +int register_null_unwind_libunwind_ops(struct thread *thread)
> +{
> + thread->unwind_libunwind_ops = &null_unwind_libunwind_ops;
> + return 0;
> +}
> +
> +int register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
> + struct thread *thread)
> +{
> + thread->unwind_libunwind_ops = ops;
> + return 0;
> +}
> diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
> index 12790cf..61b44a6 100644
> --- a/tools/perf/util/unwind.h
> +++ b/tools/perf/util/unwind.h
> @@ -24,6 +24,28 @@ 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);
> +int register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
> + struct thread *thread);
> +int register_null_unwind_libunwind_ops(struct thread *thread);
> +
> +#ifndef HAVE_LIBUNWIND_LOCAL_SUPPORT
> +static inline int
> +register_local_unwind_libunwind_ops(struct thread *thread) {
> + return register_null_unwind_libunwind_ops(thread);
> +}
> +#else
> +int 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 int unwind__prepare_access(struct thread *thread __maybe_unused)
> {
> --
> 1.8.5.2

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH 0/8] Add support for remote unwind"
Previous message: Thomas Gleixner: "Re: [PATCH 0/6] Intel Secure Guard Extensions"
In reply to: He Kuang: "[PATCH 6/8] perf callchain: Add support for cross-platform unwind"
Next in thread: He Kuang: "[PATCH 4/8] perf build: Add build-test for debug-frame on arm/arm64"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]