Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1395726 > unrolled thread

[PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind

Started byHe Kuang <hekuang@huawei.com>
First post2016-05-06 11:10 +0200
Last post2016-05-07 21:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind He Kuang <hekuang@huawei.com> - 2016-05-06 11:10 +0200
    Re: [PATCH 5/8] perf tools: Promote proper messages for  cross-platform unwind Jiri Olsa <jolsa@redhat.com> - 2016-05-07 20:50 +0200
    Re: [PATCH 5/8] perf tools: Promote proper messages for  cross-platform unwind Jiri Olsa <jolsa@redhat.com> - 2016-05-07 21:50 +0200

#1395726 — [PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind

FromHe Kuang <hekuang@huawei.com>
Date2016-05-06 11:10 +0200
Subject[PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind
Message-ID<rvJVM-2eU-31@gated-at.bofh.it>
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@huawei.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
 }
 
 static int thread__clone_map_groups(struct thread *thread,
-- 
1.8.5.2

[toc] | [next] | [standalone]


#1396345 — Re: [PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind

FromJiri Olsa <jolsa@redhat.com>
Date2016-05-07 20:50 +0200
SubjectRe: [PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind
Message-ID<rwfsC-6Re-5@gated-at.bofh.it>
In reply to#1395726
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@huawei.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

[toc] | [prev] | [next] | [standalone]


#1396357 — Re: [PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind

FromJiri Olsa <jolsa@redhat.com>
Date2016-05-07 21:50 +0200
SubjectRe: [PATCH 5/8] perf tools: Promote proper messages for cross-platform unwind
Message-ID<rwgoF-7MA-1@gated-at.bofh.it>
In reply to#1395726
On Fri, May 06, 2016 at 08:59:11AM +0000, He Kuang wrote:

SNIP

> +	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);

please put above 2 functions addition into separate patch

thanks,
jirka

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web