[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160507184113.GF19977@krava>
Date: Sat, 7 May 2016 20:41:13 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: He Kuang <hekuang@...wei.com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
alexander.shishkin@...ux.intel.com, wangnan0@...wei.com,
jpoimboe@...hat.com, ak@...ux.intel.com, eranian@...gle.com,
namhyung@...nel.org, adrian.hunter@...el.com,
sukadev@...ux.vnet.ibm.com, masami.hiramatsu.pt@...achi.com,
tumanova@...ux.vnet.ibm.com, kan.liang@...el.com,
penberg@...nel.org, dsahern@...il.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/8] perf tools: Promote proper messages for
cross-platform unwind
On Fri, May 06, 2016 at 08:59:11AM +0000, He Kuang wrote:
> Currently, perf script uses host unwind methods to parse perf.data
> callchain info regardless of the target architecture. So we get wrong
> result and no promotion when unwinding callchains of x86(32bit) on
> x86(64bit) machine.
>
> This patch shows proper error messages when we do remote unwind
> x86(32bit) on other machines.
>
> Same thing for other platforms will be added in next patches.
>
> Signed-off-by: He Kuang <hekuang@...wei.com>
> ---
> tools/perf/config/Makefile | 6 ++++++
> tools/perf/util/symbol-elf.c | 16 +++++++++++++++
> tools/perf/util/symbol.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/symbol.h | 2 ++
> tools/perf/util/thread.c | 31 ++++++++++++++++++++++++++++
> 5 files changed, 104 insertions(+)
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index 1e46277..a86b864 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -345,6 +345,12 @@ ifeq ($(ARCH),powerpc)
> endif
>
> ifndef NO_LIBUNWIND
> + ifeq ($(feature-libunwind-x86), 1)
> + LIBUNWIND_LIBS += -lunwind-x86
> + $(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/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 3f9d679..9f290b9 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -636,6 +636,22 @@ bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
> return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
> }
>
> +int elf_is_64_bit(char *name)
> +{
> + Elf *elf;
> + int fd;
> +
> + fd = open(name, O_RDONLY);
> + if (fd < 0)
> + return -1;
> +
> + elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
> + if (elf == NULL)
> + return -1;
> +
> + return (gelf_getclass(elf) == ELFCLASS64);
> +}
> +
> int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
> enum dso_binary_type type)
> {
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 93f348f..c33aa5a 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1395,6 +1395,55 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
> }
> }
>
> +int dso_is_64_bit(struct dso *dso, struct map *map)
> +{
> + char *name;
> + u_int i;
> + bool kmod;
> + char *root_dir = (char *) "";
> + struct machine *machine;
> +
> + if (map->groups && map->groups->machine)
> + machine = map->groups->machine;
> + else
> + machine = NULL;
> +
> + if (machine)
> + root_dir = machine->root_dir;
> +
> + name = malloc(PATH_MAX);
> + if (!name)
> + return -1;
> +
> + kmod = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
> + dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP ||
> + dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
> + dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
> +
> + /*
> + * Iterate over candidate debug images.
> + * Keep track of "interesting" ones (those which have a symtab, dynsym,
> + * and/or opd section) for processing.
> + */
> + for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
> + enum dso_binary_type symtab_type = binary_type_symtab[i];
> +
> + if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type))
> + continue;
> +
> + if (dso__read_binary_type_filename(dso, symtab_type,
> + root_dir, name, PATH_MAX))
> + continue;
> +
> + if (!is_regular_file(name))
> + continue;
> +
> + return elf_is_64_bit(name);
> + }
> +
> + return -1;
> +}
> +
> int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
> {
> char *name;
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 4e6910e..d33fbf4 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -308,6 +308,8 @@ int setup_list(struct strlist **list, const char *list_str,
> const char *list_name);
> int setup_intlist(struct intlist **list, const char *list_str,
> const char *list_name);
> +int elf_is_64_bit(char *name);
> +int dso_is_64_bit(struct dso *dso, struct map *map);
>
> #ifdef HAVE_LIBELF_SUPPORT
> bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index dfd00c6..e0cdcf7 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -184,8 +184,39 @@ size_t thread__fprintf(struct thread *thread, FILE *fp)
>
> void thread__insert_map(struct thread *thread, struct map *map)
> {
> + char * __maybe_unused arch;
> + int __maybe_unused is_64_bit;
> +
> map_groups__fixup_overlappings(thread->mg, map, stderr);
> map_groups__insert(thread->mg, map);
> +
> +#ifdef HAVE_LIBUNWIND_SUPPORT
> + if (!thread->mg->machine->env)
> + return;
> +
> + is_64_bit = dso_is_64_bit(map->dso, map);
> + if (is_64_bit < 0)
> + return;
> +
> + if (thread->addr_space)
> + pr_debug("Thread map already set, 64bit is %d, dso=%s\n",
> + is_64_bit, map->dso->name);
> +
> + arch = thread->mg->machine->env->arch;
> +
> + if (!strcmp(arch, "x86_64")
> + || !strcmp(arch, "x86")
> + || !strcmp(arch, "i686")) {
> + pr_debug("Thread map is X86, 64bit is %d\n", 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
> + }
> +#endif
could you please move above code under unwind object,
like we do with unwind__prepare_access in thread__new?
also we could unite both names somehome now we have 2 callbacks
to unwind code from generic code
thanks,
jirka
Powered by blists - more mailing lists