From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758665AbcEFL42 (ORCPT ); Fri, 6 May 2016 07:56:28 -0400 Received: from mail.kernel.org ([198.145.29.136]:41179 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758650AbcEFL41 (ORCPT ); Fri, 6 May 2016 07:56:27 -0400 Date: Fri, 6 May 2016 08:56:21 -0300 From: Arnaldo Carvalho de Melo To: He Kuang Cc: peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, jolsa@redhat.com, wangnan0@huawei.com, jpoimboe@redhat.com, ak@linux.intel.com, eranian@google.com, namhyung@kernel.org, adrian.hunter@intel.com, sukadev@linux.vnet.ibm.com, masami.hiramatsu.pt@hitachi.com, tumanova@linux.vnet.ibm.com, kan.liang@intel.com, penberg@kernel.org, dsahern@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/8] perf callchain: Add support for cross-platform unwind Message-ID: <20160506115621.GV11069@kernel.org> References: <1462525154-125656-1-git-send-email-hekuang@huawei.com> <1462525154-125656-7-git-send-email-hekuang@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462525154-125656-7-git-send-email-hekuang@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 > #include > #include > +#ifdef ARCH_UNWIND_LIBUNWIND > +#include "libunwind-arch.h" > +#endif > #include > #include > #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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#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