[PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

When libelf is not used, perf cannot not show symbol names since it
doesn't access the ELF symbol table.  But kernel is different as it
can fallback to kallsyms.

It worked w/o libelf, but recent change to use vmlinux for kernel
symbols break it.

With this change, it now can show kernel symbols again:

  # Overhead  Command  Shared Object      Symbol
  # ........  .......  .................  ........................
  #
      34.51%  swapper  [kernel.kallsyms]  [k] intel_idle
      12.54%  perf     [kernel.kallsyms]  [k] generic_exec_single
      10.11%  swapper  [kernel.kallsyms]  [k] int_sqrt
       9.83%  swapper  [kernel.kallsyms]  [k] hrtimer_interrupt
       7.25%  emacs    [kernel.kallsyms]  [k] __switch_to
       7.06%  sleep    [kernel.kallsyms]  [k] find_next_iomem_res
       7.02%  swapper  [kernel.kallsyms]  [k] run_timer_softirq
       2.55%  swapper  [kernel.kallsyms]  [k] _raw_spin_unlock_irq
       1.90%  swapper  [kernel.kallsyms]  [k] lapic_next_deadline
       1.75%  swapper  [kernel.kallsyms]  [k] native_sched_clock
       1.49%  swapper  [kernel.kallsyms]  [k] __schedule
       1.20%  swapper  [kernel.kallsyms]  [k] read_tsc
       1.10%  sleep    [kernel.kallsyms]  [k] current_kernel_time
       0.99%  swapper  [kernel.kallsyms]  [k] pick_next_task_rt
       0.36%  swapper  [kernel.kallsyms]  [k] group_sched_in
       0.24%  swapper  [kernel.kallsyms]  [k] x86_pmu_commit_txn
       0.06%  swapper  [kernel.kallsyms]  [k] intel_pmu_enable_all
       0.03%  perf     [kernel.kallsyms]  [k] intel_pmu_enable_all

Reported-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/symbol-minimal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index c9541fea9514..226cf41ed7e6 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 	unsigned char *build_id[BUILD_ID_SIZE];
 	int ret;
 
+	if (dso->kernel)
+		return 0;  /* always use kallsyms */
+
 	ret = fd__is_64_bit(ss->fd);
 	if (ret >= 0)
 		dso->is_64_bit = ret;
-- 
2.1.2

[PATCH 2/3] perf symbol: Implement a very simple ELF symbol parser

[Namhyung Kim]

It'll be used to show (userspace) symbol names when libelf isn't (or
cannot be) linked.

  # Overhead  Command     Shared Object      Symbol
  # ........  ..........  .................  .........................
  #
      37.01%  mem-memcpy  libc-2.17.so       [.] __memcpy_ssse3_back
      24.25%  perf        ld-2.17.so         [.] _dl_relocate_object
      22.16%  perf        [kernel.kallsyms]  [k] kmem_cache_alloc
      14.29%  mem-memset  libc-2.17.so       [.] __memset_sse2
       2.21%  perf        [kernel.kallsyms]  [k] flush_signal_handlers
       0.07%  perf        [kernel.kallsyms]  [k] intel_pmu_enable_all

Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/symbol-minimal.c | 143 ++++++++++++++++++++++++++++++++++++---
 tools/perf/util/symbol-minimal.h |  64 ++++++++++++++++++
 tools/perf/util/symbol.h         |  10 ++-
 3 files changed, 208 insertions(+), 9 deletions(-)
 create mode 100644 tools/perf/util/symbol-minimal.h

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 226cf41ed7e6..9da571d83a25 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -1,11 +1,13 @@
 #include "symbol.h"
 #include "util.h"
+#include "debug.h"
 
 #include <stdio.h>
 #include <fcntl.h>
 #include <string.h>
 #include <byteswap.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 
 
 static bool check_need_swap(int file_endian)
@@ -242,22 +244,81 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
 	return ret;
 }
 
+static const char *symsrc__symname(struct symsrc *ss, Elf_Shdr *strsec, int idx)
+{
+	return ss->ptr + strsec->sh_offset + idx;
+}
+
 int symsrc__init(struct symsrc *ss, struct dso *dso __maybe_unused,
 		 const char *name,
 	         enum dso_binary_type type)
 {
-	int fd = open(name, O_RDONLY);
+	int fd;
+	unsigned i;
+	Elf_Ehdr ehdr;
+	struct stat stbuf;
+	void *ptr;
+
+	fd = open(name, O_RDONLY);
 	if (fd < 0)
 		return -1;
 
 	ss->name = strdup(name);
-	if (!ss->name)
-		goto out_close;
+	if (!ss->name) {
+		close(fd);
+		return -1;
+	}
 
 	ss->fd = fd;
 	ss->type = type;
 
+	/* only consider native case (no swap) */
+	if (read(fd, &ehdr, sizeof(ehdr)) < 0)
+		goto out_close;
+
+	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) ||
+	    ehdr.e_ident[EI_VERSION] != EV_CURRENT)
+		goto out_close;
+
+	if (ehdr.e_ident[EI_CLASS] != ELFCLASS)
+		goto out_close;
+
+	if (check_need_swap(ehdr.e_ident[EI_DATA]))
+		goto out_close;
+
+	if (ehdr.e_shentsize != sizeof(Elf_Shdr))
+		goto out_close;
+
+	if (fstat(fd, &stbuf) < 0)
+		goto out_close;
+
+	ptr = mmap(NULL, stbuf.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (ptr == MAP_FAILED)
+		goto out_close;
+
+	ss->ptr = ptr;
+	ss->len = stbuf.st_size;
+	ss->sec = ptr + ehdr.e_shoff;
+
+	ss->symtab = NULL;
+	for (i = 0; i < ehdr.e_shnum; i++) {
+		Elf_Shdr *shdrp = &ss->sec[i];
+		const char *shname;
+
+		shname = symsrc__symname(ss, &ss->sec[ehdr.e_shstrndx],
+					 shdrp->sh_name);
+
+		if (!strcmp(shname, ".text"))
+			ss->adjust_offset = shdrp->sh_addr - shdrp->sh_offset;
+
+		if (shdrp->sh_type == SHT_SYMTAB) {
+			ss->symtab = shdrp;
+			ss->strtab = &ss->sec[shdrp->sh_link];
+		}
+	}
+
 	return 0;
+
 out_close:
 	close(fd);
 	return -1;
@@ -276,6 +337,7 @@ bool symsrc__has_symtab(struct symsrc *ss __maybe_unused)
 
 void symsrc__destroy(struct symsrc *ss)
 {
+	munmap(ss->ptr, ss->len);
 	zfree(&ss->name);
 	close(ss->fd);
 }
@@ -332,7 +394,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 		  symbol_filter_t filter __maybe_unused,
 		  int kmodule __maybe_unused)
 {
-	unsigned char *build_id[BUILD_ID_SIZE];
+	Elf_Ehdr *ehdr = ss->ptr;
+	Elf_Sym *symtab;
+	size_t i, nr_sym;
 	int ret;
 
 	if (dso->kernel)
@@ -342,11 +406,74 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 	if (ret >= 0)
 		dso->is_64_bit = ret;
 
-	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
-		dso__set_build_id(dso, build_id);
-		return 1;
+	if (dso->has_build_id) {
+		u8 build_id[BUILD_ID_SIZE];
+
+		if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) < 0)
+			return -1;
+
+		if (!dso__build_id_equal(dso, build_id))
+			return -1;
 	}
-	return 0;
+
+	if (map->type != MAP__FUNCTION)
+		return 0;
+
+	if (!ss->symtab)
+		return 0;
+
+	symtab = ss->ptr + ss->symtab->sh_offset;
+	nr_sym = ss->symtab->sh_size / ss->symtab->sh_entsize;
+
+	pr_debug4("loading symbols from %s\n", ss->name);
+	ret = 0;
+	for (i = 0; i < nr_sym; i++) {
+		struct symbol *s;
+		Elf_Sym *sym = &symtab[i];
+		const char *symname = symsrc__symname(ss, ss->strtab, sym->st_name);
+		unsigned long sym_addr = sym->st_value;
+
+		if (ELF_ST_TYPE(sym->st_info) != STT_FUNC)
+			continue;
+
+		if (sym->st_shndx == SHN_UNDEF)
+			continue;
+
+		if (ehdr->e_machine == EM_ARM) {
+			/* Reject ARM ELF "mapping symbols": these aren't unique
+			 * and don't identify functions, so will confuse the
+			 * profile output: */
+			if (!strcmp(symname, "$a") ||
+			    !strcmp(symname, "$d") ||
+			    !strcmp(symname, "$t"))
+				continue;
+
+			/* On ARM, symbols for thumb functions have 1 added to
+			 * the symbol address as a flag - remove it */
+			if (sym->st_value & 1)
+				sym_addr--;
+		}
+		sym_addr -= ss->adjust_offset;
+
+		pr_debug4("sym: %s (%lx+%lu)\n", symname, sym_addr, sym->st_size);
+		s = symbol__new(sym_addr, sym->st_size,
+				ELF_ST_BIND(sym->st_info), symname);
+		if (!s)
+			break;
+
+		if (filter && filter(map, s))
+			symbol__delete(s);
+		else {
+			symbols__insert(&dso->symbols[map->type], s);
+			ret++;
+		}
+	}
+
+	if (ret > 0) {
+		symbols__fixup_duplicate(&dso->symbols[map->type]);
+		symbols__fixup_end(&dso->symbols[map->type]);
+	}
+	return ret;
 }
 
 int file__read_maps(int fd __maybe_unused, bool exe __maybe_unused,
diff --git a/tools/perf/util/symbol-minimal.h b/tools/perf/util/symbol-minimal.h
new file mode 100644
index 000000000000..7b8b38426359
--- /dev/null
+++ b/tools/perf/util/symbol-minimal.h
@@ -0,0 +1,64 @@
+#ifndef __PERF_SYMBOL_MINIMAL_H
+#define __PERF_SYMBOL_MINIMAL_H
+
+#include <elf.h>
+#include <linux/bitops.h>
+
+#if BITS_PER_LONG == 32
+
+# define ELFCLASS	ELFCLASS32
+
+# define Elf_Half	Elf32_Half
+# define Elf_Word	Elf32_Word
+# define Elf_Sword	Elf32_Sword
+# define Elf_Xword	Elf32_Xword
+# define Elf_Sxword	Elf32_Sxword
+# define Elf_Addr	Elf32_Addr
+# define Elf_Off	Elf32_Off
+# define Elf_Section	Elf32_Section
+# define Elf_Versym	Elf32_Versym
+
+# define Elf_Ehdr	Elf32_Ehdr
+# define Elf_Phdr	Elf32_Phdr
+# define Elf_Shdr	Elf32_Shdr
+# define Elf_Nhdr	Elf32_Nhdr
+# define Elf_Sym	Elf32_Sym
+# define Elf_Syminfo	Elf32_Syminfo
+# define Elf_Rel	Elf32_Rel
+# define Elf_Rela	Elf32_Rela
+# define Elf_Dyn	Elf32_Dyn
+
+# define ELF_ST_BIND(val)		ELF32_ST_BIND(val)
+# define ELF_ST_TYPE(val)		ELF32_ST_TYPE(val)
+# define ELF_ST_INFO(bind, type)	ELF32_ST_INFO(bind, type)
+
+#else /* BITS_PER_LONG == 64 */
+
+# define ELFCLASS	ELFCLASS64
+
+# define Elf_Half	Elf64_Half
+# define Elf_Word	Elf64_Word
+# define Elf_Sword	Elf64_Sword
+# define Elf_Xword	Elf64_Xword
+# define Elf_Sxword	Elf64_Sxword
+# define Elf_Addr	Elf64_Addr
+# define Elf_Off	Elf64_Off
+# define Elf_Section	Elf64_Section
+# define Elf_Versym	Elf64_Versym
+
+# define Elf_Ehdr	Elf64_Ehdr
+# define Elf_Phdr	Elf64_Phdr
+# define Elf_Shdr	Elf64_Shdr
+# define Elf_Nhdr	Elf64_Nhdr
+# define Elf_Sym	Elf64_Sym
+# define Elf_Syminfo	Elf64_Syminfo
+# define Elf_Rel	Elf64_Rel
+# define Elf_Rela	Elf64_Rela
+# define Elf_Dyn	Elf64_Dyn
+
+# define ELF_ST_BIND(val)		ELF64_ST_BIND(val)
+# define ELF_ST_TYPE(val)		ELF64_ST_TYPE(val)
+# define ELF_ST_INFO(bind, type)	ELF64_ST_INFO(bind, type)
+
+#endif /* BITS_PER_LONG == 64 */
+#endif /* __PERF_SYMBOL_MINIMAL_H */
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index ded3ca7266de..1c1dc5b9b3f8 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -18,8 +18,9 @@
 #ifdef HAVE_LIBELF_SUPPORT
 #include <libelf.h>
 #include <gelf.h>
+#else
+#include "symbol-minimal.h"
 #endif
-#include <elf.h>
 
 #include "dso.h"
 
@@ -229,6 +230,13 @@ struct symsrc {
 
 	bool adjust_symbols;
 	bool is_64_bit;
+#else
+	void *ptr;
+	size_t len;
+	Elf_Shdr *sec;
+	Elf_Shdr *symtab;
+	Elf_Shdr *strtab;
+	long adjust_offset;
 #endif
 };
 
-- 
2.1.2

Re: [PATCH 2/3] perf symbol: Implement a very simple ELF symbol parser

[Arnaldo Carvalho de Melo]

Em Fri, Nov 07, 2014 at 02:20:05PM +0900, Namhyung Kim escreveu:
> It'll be used to show (userspace) symbol names when libelf isn't (or
> cannot be) linked.

Does this deals with prelink, etc?

- Arnaldo
 
>   # Overhead  Command     Shared Object      Symbol
>   # ........  ..........  .................  .........................
>   #
>       37.01%  mem-memcpy  libc-2.17.so       [.] __memcpy_ssse3_back
>       24.25%  perf        ld-2.17.so         [.] _dl_relocate_object
>       22.16%  perf        [kernel.kallsyms]  [k] kmem_cache_alloc
>       14.29%  mem-memset  libc-2.17.so       [.] __memset_sse2
>        2.21%  perf        [kernel.kallsyms]  [k] flush_signal_handlers
>        0.07%  perf        [kernel.kallsyms]  [k] intel_pmu_enable_all
> 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/symbol-minimal.c | 143 ++++++++++++++++++++++++++++++++++++---
>  tools/perf/util/symbol-minimal.h |  64 ++++++++++++++++++
>  tools/perf/util/symbol.h         |  10 ++-
>  3 files changed, 208 insertions(+), 9 deletions(-)
>  create mode 100644 tools/perf/util/symbol-minimal.h
> 
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 226cf41ed7e6..9da571d83a25 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -1,11 +1,13 @@
>  #include "symbol.h"
>  #include "util.h"
> +#include "debug.h"
>  
>  #include <stdio.h>
>  #include <fcntl.h>
>  #include <string.h>
>  #include <byteswap.h>
>  #include <sys/stat.h>
> +#include <sys/mman.h>
>  
>  
>  static bool check_need_swap(int file_endian)
> @@ -242,22 +244,81 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
>  	return ret;
>  }
>  
> +static const char *symsrc__symname(struct symsrc *ss, Elf_Shdr *strsec, int idx)
> +{
> +	return ss->ptr + strsec->sh_offset + idx;
> +}
> +
>  int symsrc__init(struct symsrc *ss, struct dso *dso __maybe_unused,
>  		 const char *name,
>  	         enum dso_binary_type type)
>  {
> -	int fd = open(name, O_RDONLY);
> +	int fd;
> +	unsigned i;
> +	Elf_Ehdr ehdr;
> +	struct stat stbuf;
> +	void *ptr;
> +
> +	fd = open(name, O_RDONLY);
>  	if (fd < 0)
>  		return -1;
>  
>  	ss->name = strdup(name);
> -	if (!ss->name)
> -		goto out_close;
> +	if (!ss->name) {
> +		close(fd);
> +		return -1;
> +	}
>  
>  	ss->fd = fd;
>  	ss->type = type;
>  
> +	/* only consider native case (no swap) */
> +	if (read(fd, &ehdr, sizeof(ehdr)) < 0)
> +		goto out_close;
> +
> +	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) ||
> +	    ehdr.e_ident[EI_VERSION] != EV_CURRENT)
> +		goto out_close;
> +
> +	if (ehdr.e_ident[EI_CLASS] != ELFCLASS)
> +		goto out_close;
> +
> +	if (check_need_swap(ehdr.e_ident[EI_DATA]))
> +		goto out_close;
> +
> +	if (ehdr.e_shentsize != sizeof(Elf_Shdr))
> +		goto out_close;
> +
> +	if (fstat(fd, &stbuf) < 0)
> +		goto out_close;
> +
> +	ptr = mmap(NULL, stbuf.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
> +	if (ptr == MAP_FAILED)
> +		goto out_close;
> +
> +	ss->ptr = ptr;
> +	ss->len = stbuf.st_size;
> +	ss->sec = ptr + ehdr.e_shoff;
> +
> +	ss->symtab = NULL;
> +	for (i = 0; i < ehdr.e_shnum; i++) {
> +		Elf_Shdr *shdrp = &ss->sec[i];
> +		const char *shname;
> +
> +		shname = symsrc__symname(ss, &ss->sec[ehdr.e_shstrndx],
> +					 shdrp->sh_name);
> +
> +		if (!strcmp(shname, ".text"))
> +			ss->adjust_offset = shdrp->sh_addr - shdrp->sh_offset;
> +
> +		if (shdrp->sh_type == SHT_SYMTAB) {
> +			ss->symtab = shdrp;
> +			ss->strtab = &ss->sec[shdrp->sh_link];
> +		}
> +	}
> +
>  	return 0;
> +
>  out_close:
>  	close(fd);
>  	return -1;
> @@ -276,6 +337,7 @@ bool symsrc__has_symtab(struct symsrc *ss __maybe_unused)
>  
>  void symsrc__destroy(struct symsrc *ss)
>  {
> +	munmap(ss->ptr, ss->len);
>  	zfree(&ss->name);
>  	close(ss->fd);
>  }
> @@ -332,7 +394,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>  		  symbol_filter_t filter __maybe_unused,
>  		  int kmodule __maybe_unused)
>  {
> -	unsigned char *build_id[BUILD_ID_SIZE];
> +	Elf_Ehdr *ehdr = ss->ptr;
> +	Elf_Sym *symtab;
> +	size_t i, nr_sym;
>  	int ret;
>  
>  	if (dso->kernel)
> @@ -342,11 +406,74 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>  	if (ret >= 0)
>  		dso->is_64_bit = ret;
>  
> -	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
> -		dso__set_build_id(dso, build_id);
> -		return 1;
> +	if (dso->has_build_id) {
> +		u8 build_id[BUILD_ID_SIZE];
> +
> +		if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) < 0)
> +			return -1;
> +
> +		if (!dso__build_id_equal(dso, build_id))
> +			return -1;
>  	}
> -	return 0;
> +
> +	if (map->type != MAP__FUNCTION)
> +		return 0;
> +
> +	if (!ss->symtab)
> +		return 0;
> +
> +	symtab = ss->ptr + ss->symtab->sh_offset;
> +	nr_sym = ss->symtab->sh_size / ss->symtab->sh_entsize;
> +
> +	pr_debug4("loading symbols from %s\n", ss->name);
> +	ret = 0;
> +	for (i = 0; i < nr_sym; i++) {
> +		struct symbol *s;
> +		Elf_Sym *sym = &symtab[i];
> +		const char *symname = symsrc__symname(ss, ss->strtab, sym->st_name);
> +		unsigned long sym_addr = sym->st_value;
> +
> +		if (ELF_ST_TYPE(sym->st_info) != STT_FUNC)
> +			continue;
> +
> +		if (sym->st_shndx == SHN_UNDEF)
> +			continue;
> +
> +		if (ehdr->e_machine == EM_ARM) {
> +			/* Reject ARM ELF "mapping symbols": these aren't unique
> +			 * and don't identify functions, so will confuse the
> +			 * profile output: */
> +			if (!strcmp(symname, "$a") ||
> +			    !strcmp(symname, "$d") ||
> +			    !strcmp(symname, "$t"))
> +				continue;
> +
> +			/* On ARM, symbols for thumb functions have 1 added to
> +			 * the symbol address as a flag - remove it */
> +			if (sym->st_value & 1)
> +				sym_addr--;
> +		}
> +		sym_addr -= ss->adjust_offset;
> +
> +		pr_debug4("sym: %s (%lx+%lu)\n", symname, sym_addr, sym->st_size);
> +		s = symbol__new(sym_addr, sym->st_size,
> +				ELF_ST_BIND(sym->st_info), symname);
> +		if (!s)
> +			break;
> +
> +		if (filter && filter(map, s))
> +			symbol__delete(s);
> +		else {
> +			symbols__insert(&dso->symbols[map->type], s);
> +			ret++;
> +		}
> +	}
> +
> +	if (ret > 0) {
> +		symbols__fixup_duplicate(&dso->symbols[map->type]);
> +		symbols__fixup_end(&dso->symbols[map->type]);
> +	}
> +	return ret;
>  }
>  
>  int file__read_maps(int fd __maybe_unused, bool exe __maybe_unused,
> diff --git a/tools/perf/util/symbol-minimal.h b/tools/perf/util/symbol-minimal.h
> new file mode 100644
> index 000000000000..7b8b38426359
> --- /dev/null
> +++ b/tools/perf/util/symbol-minimal.h
> @@ -0,0 +1,64 @@
> +#ifndef __PERF_SYMBOL_MINIMAL_H
> +#define __PERF_SYMBOL_MINIMAL_H
> +
> +#include <elf.h>
> +#include <linux/bitops.h>
> +
> +#if BITS_PER_LONG == 32
> +
> +# define ELFCLASS	ELFCLASS32
> +
> +# define Elf_Half	Elf32_Half
> +# define Elf_Word	Elf32_Word
> +# define Elf_Sword	Elf32_Sword
> +# define Elf_Xword	Elf32_Xword
> +# define Elf_Sxword	Elf32_Sxword
> +# define Elf_Addr	Elf32_Addr
> +# define Elf_Off	Elf32_Off
> +# define Elf_Section	Elf32_Section
> +# define Elf_Versym	Elf32_Versym
> +
> +# define Elf_Ehdr	Elf32_Ehdr
> +# define Elf_Phdr	Elf32_Phdr
> +# define Elf_Shdr	Elf32_Shdr
> +# define Elf_Nhdr	Elf32_Nhdr
> +# define Elf_Sym	Elf32_Sym
> +# define Elf_Syminfo	Elf32_Syminfo
> +# define Elf_Rel	Elf32_Rel
> +# define Elf_Rela	Elf32_Rela
> +# define Elf_Dyn	Elf32_Dyn
> +
> +# define ELF_ST_BIND(val)		ELF32_ST_BIND(val)
> +# define ELF_ST_TYPE(val)		ELF32_ST_TYPE(val)
> +# define ELF_ST_INFO(bind, type)	ELF32_ST_INFO(bind, type)
> +
> +#else /* BITS_PER_LONG == 64 */
> +
> +# define ELFCLASS	ELFCLASS64
> +
> +# define Elf_Half	Elf64_Half
> +# define Elf_Word	Elf64_Word
> +# define Elf_Sword	Elf64_Sword
> +# define Elf_Xword	Elf64_Xword
> +# define Elf_Sxword	Elf64_Sxword
> +# define Elf_Addr	Elf64_Addr
> +# define Elf_Off	Elf64_Off
> +# define Elf_Section	Elf64_Section
> +# define Elf_Versym	Elf64_Versym
> +
> +# define Elf_Ehdr	Elf64_Ehdr
> +# define Elf_Phdr	Elf64_Phdr
> +# define Elf_Shdr	Elf64_Shdr
> +# define Elf_Nhdr	Elf64_Nhdr
> +# define Elf_Sym	Elf64_Sym
> +# define Elf_Syminfo	Elf64_Syminfo
> +# define Elf_Rel	Elf64_Rel
> +# define Elf_Rela	Elf64_Rela
> +# define Elf_Dyn	Elf64_Dyn
> +
> +# define ELF_ST_BIND(val)		ELF64_ST_BIND(val)
> +# define ELF_ST_TYPE(val)		ELF64_ST_TYPE(val)
> +# define ELF_ST_INFO(bind, type)	ELF64_ST_INFO(bind, type)
> +
> +#endif /* BITS_PER_LONG == 64 */
> +#endif /* __PERF_SYMBOL_MINIMAL_H */
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index ded3ca7266de..1c1dc5b9b3f8 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -18,8 +18,9 @@
>  #ifdef HAVE_LIBELF_SUPPORT
>  #include <libelf.h>
>  #include <gelf.h>
> +#else
> +#include "symbol-minimal.h"
>  #endif
> -#include <elf.h>
>  
>  #include "dso.h"
>  
> @@ -229,6 +230,13 @@ struct symsrc {
>  
>  	bool adjust_symbols;
>  	bool is_64_bit;
> +#else
> +	void *ptr;
> +	size_t len;
> +	Elf_Shdr *sec;
> +	Elf_Shdr *symtab;
> +	Elf_Shdr *strtab;
> +	long adjust_offset;
>  #endif
>  };
>  
> -- 
> 2.1.2

Re: [PATCH 2/3] perf symbol: Implement a very simple ELF symbol parser

[Namhyung Kim]

Hi Arnaldo,

On Fri, 7 Nov 2014 12:26:18 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 07, 2014 at 02:20:05PM +0900, Namhyung Kim escreveu:
>> It'll be used to show (userspace) symbol names when libelf isn't (or
>> cannot be) linked.
>
> Does this deals with prelink, etc?

I believe so. :)

>  
>>   # Overhead  Command     Shared Object      Symbol
>>   # ........  ..........  .................  .........................
>>   #
>>       37.01%  mem-memcpy  libc-2.17.so       [.] __memcpy_ssse3_back

  namhyung@sejong:linux$ readelf -WS /lib64/libc-2.17.so | grep prelink
    [41] .gnu.prelink_undo PROGBITS        0000000000000000 200368 000c30 01      0   0  8

  namhyung@sejong:linux$ nm /lib64/libc-2.17.so | grep __memcpy_ssse3_back
  0000003153f46f40 t __memcpy_ssse3_back


Thanks,
Namhyung


>>       24.25%  perf        ld-2.17.so         [.] _dl_relocate_object
>>       22.16%  perf        [kernel.kallsyms]  [k] kmem_cache_alloc
>>       14.29%  mem-memset  libc-2.17.so       [.] __memset_sse2
>>        2.21%  perf        [kernel.kallsyms]  [k] flush_signal_handlers
>>        0.07%  perf        [kernel.kallsyms]  [k] intel_pmu_enable_all

Re: [PATCH 2/3] perf symbol: Implement a very simple ELF symbol parser

[Arnaldo Carvalho de Melo]

Em Mon, Nov 10, 2014 at 03:36:48PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Fri, 7 Nov 2014 12:26:18 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Nov 07, 2014 at 02:20:05PM +0900, Namhyung Kim escreveu:
> >> It'll be used to show (userspace) symbol names when libelf isn't (or
> >> cannot be) linked.
> >
> > Does this deals with prelink, etc?
> 
> I believe so. :)
> 
> >  
> >>   # Overhead  Command     Shared Object      Symbol
> >>   # ........  ..........  .................  .........................
> >>   #
> >>       37.01%  mem-memcpy  libc-2.17.so       [.] __memcpy_ssse3_back
> 
>   namhyung@sejong:linux$ readelf -WS /lib64/libc-2.17.so | grep prelink
>     [41] .gnu.prelink_undo PROGBITS        0000000000000000 200368 000c30 01      0   0  8
> 
>   namhyung@sejong:linux$ nm /lib64/libc-2.17.so | grep __memcpy_ssse3_back
>   0000003153f46f40 t __memcpy_ssse3_back

Right, in this case most of the samples seems to map to what is expected
for that workload, and the binary was prelinked. Good.

What about binaries that are not prelinked? IIRC there is code in the
full blown ELF symbol-elf.c file to detect that and act accordingly,
from a _very_ quick look I didn't saw it in this minimalistic ELF symtab
reader, hence my question.

- Arnaldo
 
> 
> Thanks,
> Namhyung
> 
> 
> >>       24.25%  perf        ld-2.17.so         [.] _dl_relocate_object
> >>       22.16%  perf        [kernel.kallsyms]  [k] kmem_cache_alloc
> >>       14.29%  mem-memset  libc-2.17.so       [.] __memset_sse2
> >>        2.21%  perf        [kernel.kallsyms]  [k] flush_signal_handlers
> >>        0.07%  perf        [kernel.kallsyms]  [k] intel_pmu_enable_all

Re: [PATCH 2/3] perf symbol: Implement a very simple ELF symbol parser

[Namhyung Kim]

On Mon, 10 Nov 2014 09:09:04 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 10, 2014 at 03:36:48PM +0900, Namhyung Kim escreveu:
>> Hi Arnaldo,
>> 
>> On Fri, 7 Nov 2014 12:26:18 -0300, Arnaldo Carvalho de Melo wrote:
>> > Em Fri, Nov 07, 2014 at 02:20:05PM +0900, Namhyung Kim escreveu:
>> >> It'll be used to show (userspace) symbol names when libelf isn't (or
>> >> cannot be) linked.
>> >
>> > Does this deals with prelink, etc?
>> 
>> I believe so. :)
>> 
>> >  
>> >>   # Overhead  Command     Shared Object      Symbol
>> >>   # ........  ..........  .................  .........................
>> >>   #
>> >>       37.01%  mem-memcpy  libc-2.17.so       [.] __memcpy_ssse3_back
>> 
>>   namhyung@sejong:linux$ readelf -WS /lib64/libc-2.17.so | grep prelink
>>     [41] .gnu.prelink_undo PROGBITS        0000000000000000 200368 000c30 01      0   0  8
>> 
>>   namhyung@sejong:linux$ nm /lib64/libc-2.17.so | grep __memcpy_ssse3_back
>>   0000003153f46f40 t __memcpy_ssse3_back
>
> Right, in this case most of the samples seems to map to what is expected
> for that workload, and the binary was prelinked. Good.
>
> What about binaries that are not prelinked? IIRC there is code in the
> full blown ELF symbol-elf.c file to detect that and act accordingly,
> from a _very_ quick look I didn't saw it in this minimalistic ELF symtab
> reader, hence my question.

AFAIK we deal with prelink'ed binary and normal binary as same way - we
only cares about the file offsets.


  $ perf top --stdio
     PerfTop:     110 irqs/sec  kernel:70.0%  exact:  0.0% [4000Hz cycles],  (all, 12 CPUs)
  --------------------------------------------------------------------------------------------
     9.42%  perf           [.] map__process_kallsym_symbol       
     8.91%  [kernel]       [k] kallsyms_expand_symbol.constprop.1
     6.80%  [kernel]       [k] memcpy                            
     6.08%  [kernel]       [k] vsnprintf                         
     ...


Thanks,
Namhyung

[PATCH 3/3] perf tools: Clean up libelf feature support code

[Namhyung Kim]

Current EXTLIBS contains -lelf by default and removes it when libelf
is not detected.  This is little bit confusing since we can now build
perf without libelf so there's no need to handle it differently than
other libraries.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Makefile.perf   | 2 --
 tools/perf/config/Makefile | 5 +++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 31a76ee92c93..06cccdceb0ae 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -495,8 +495,6 @@ ifneq ($(OUTPUT),)
 endif
 
 ifdef NO_LIBELF
-EXTLIBS := $(filter-out -lelf,$(EXTLIBS))
-
 # Remove ELF/DWARF dependent codes
 LIB_OBJS := $(filter-out $(OUTPUT)util/symbol-elf.o,$(LIB_OBJS))
 LIB_OBJS := $(filter-out $(OUTPUT)util/dwarf-aux.o,$(LIB_OBJS))
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 79f906c7124e..5d4b039fe1ed 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -150,7 +150,7 @@ CFLAGS += -std=gnu99
 # adding assembler files missing the .GNU-stack linker note.
 LDFLAGS += -Wl,-z,noexecstack
 
-EXTLIBS = -lelf -lpthread -lrt -lm -ldl
+EXTLIBS = -lpthread -lrt -lm -ldl
 
 ifneq ($(OUTPUT),)
   OUTPUT_FEATURES = $(OUTPUT)config/feature-checks/
@@ -354,6 +354,7 @@ endif # NO_LIBELF
 
 ifndef NO_LIBELF
   CFLAGS += -DHAVE_LIBELF_SUPPORT
+  EXTLIBS += -lelf
 
   ifeq ($(feature-libelf-mmap), 1)
     CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
@@ -373,7 +374,7 @@ ifndef NO_LIBELF
     else
       CFLAGS += -DHAVE_DWARF_SUPPORT $(LIBDW_CFLAGS)
       LDFLAGS += $(LIBDW_LDFLAGS)
-      EXTLIBS += -lelf -ldw
+      EXTLIBS += -ldw
     endif # PERF_HAVE_DWARF_REGS
   endif # NO_DWARF
 endif # NO_LIBELF
-- 
2.1.2

[tip:perf/core] perf tools: Clean up libelf feature support code

[tip-bot for Namhyung Kim]

Commit-ID:  5e2d4d0e88dac4003cf96aca00d63aff2314391e
Gitweb:     http://git.kernel.org/tip/5e2d4d0e88dac4003cf96aca00d63aff2314391e
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 7 Nov 2014 14:20:06 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 19 Nov 2014 12:33:46 -0300

perf tools: Clean up libelf feature support code

Current EXTLIBS contains -lelf by default and removes it when libelf is
not detected.

This is little bit confusing since we can now build perf without libelf
so there's no need to handle it differently than other libraries.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1415337606-2186-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf   | 2 --
 tools/perf/config/Makefile | 5 +++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index aecf61dc..478efa9 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -497,8 +497,6 @@ ifneq ($(OUTPUT),)
 endif
 
 ifdef NO_LIBELF
-EXTLIBS := $(filter-out -lelf,$(EXTLIBS))
-
 # Remove ELF/DWARF dependent codes
 LIB_OBJS := $(filter-out $(OUTPUT)util/symbol-elf.o,$(LIB_OBJS))
 LIB_OBJS := $(filter-out $(OUTPUT)util/dwarf-aux.o,$(LIB_OBJS))
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 79f906c..5d4b039 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -150,7 +150,7 @@ CFLAGS += -std=gnu99
 # adding assembler files missing the .GNU-stack linker note.
 LDFLAGS += -Wl,-z,noexecstack
 
-EXTLIBS = -lelf -lpthread -lrt -lm -ldl
+EXTLIBS = -lpthread -lrt -lm -ldl
 
 ifneq ($(OUTPUT),)
   OUTPUT_FEATURES = $(OUTPUT)config/feature-checks/
@@ -354,6 +354,7 @@ endif # NO_LIBELF
 
 ifndef NO_LIBELF
   CFLAGS += -DHAVE_LIBELF_SUPPORT
+  EXTLIBS += -lelf
 
   ifeq ($(feature-libelf-mmap), 1)
     CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
@@ -373,7 +374,7 @@ ifndef NO_LIBELF
     else
       CFLAGS += -DHAVE_DWARF_SUPPORT $(LIBDW_CFLAGS)
       LDFLAGS += $(LIBDW_LDFLAGS)
-      EXTLIBS += -lelf -ldw
+      EXTLIBS += -ldw
     endif # PERF_HAVE_DWARF_REGS
   endif # NO_DWARF
 endif # NO_LIBELF

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Peter Zijlstra]

On Fri, Nov 07, 2014 at 02:20:04PM +0900, Namhyung Kim wrote:
> When libelf is not used, perf cannot not show symbol names since it
> doesn't access the ELF symbol table.  But kernel is different as it
> can fallback to kallsyms.
> 
> It worked w/o libelf, but recent change to use vmlinux for kernel
> symbols break it.
> 
> With this change, it now can show kernel symbols again:
> 
>   # Overhead  Command  Shared Object      Symbol
>   # ........  .......  .................  ........................
>   #
>       34.51%  swapper  [kernel.kallsyms]  [k] intel_idle
>       12.54%  perf     [kernel.kallsyms]  [k] generic_exec_single
>       10.11%  swapper  [kernel.kallsyms]  [k] int_sqrt
>        9.83%  swapper  [kernel.kallsyms]  [k] hrtimer_interrupt
>        7.25%  emacs    [kernel.kallsyms]  [k] __switch_to
>        7.06%  sleep    [kernel.kallsyms]  [k] find_next_iomem_res
>        7.02%  swapper  [kernel.kallsyms]  [k] run_timer_softirq
>        2.55%  swapper  [kernel.kallsyms]  [k] _raw_spin_unlock_irq
>        1.90%  swapper  [kernel.kallsyms]  [k] lapic_next_deadline
>        1.75%  swapper  [kernel.kallsyms]  [k] native_sched_clock
>        1.49%  swapper  [kernel.kallsyms]  [k] __schedule
>        1.20%  swapper  [kernel.kallsyms]  [k] read_tsc
>        1.10%  sleep    [kernel.kallsyms]  [k] current_kernel_time
>        0.99%  swapper  [kernel.kallsyms]  [k] pick_next_task_rt
>        0.36%  swapper  [kernel.kallsyms]  [k] group_sched_in
>        0.24%  swapper  [kernel.kallsyms]  [k] x86_pmu_commit_txn
>        0.06%  swapper  [kernel.kallsyms]  [k] intel_pmu_enable_all
>        0.03%  perf     [kernel.kallsyms]  [k] intel_pmu_enable_all
> 
> Reported-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/symbol-minimal.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index c9541fea9514..226cf41ed7e6 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>  	unsigned char *build_id[BUILD_ID_SIZE];
>  	int ret;
>  
> +	if (dso->kernel)
> +		return 0;  /* always use kallsyms */
> +
>  	ret = fd__is_64_bit(ss->fd);
>  	if (ret >= 0)
>  		dso->is_64_bit = ret;

Why does this live in the minimal implementation; should we not always
discard ELF files with 0 symbols?

Suppose I have a vmlinux but removed all symbols from it; I want it to
fall back to kallsyms too.

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

Hi Peter,

2014-11-07 (금), 09:27 +0100, Peter Zijlstra:
> On Fri, Nov 07, 2014 at 02:20:04PM +0900, Namhyung Kim wrote:
> > When libelf is not used, perf cannot not show symbol names since it
> > doesn't access the ELF symbol table.  But kernel is different as it
> > can fallback to kallsyms.
> > 
> > It worked w/o libelf, but recent change to use vmlinux for kernel
> > symbols break it.
> > 
> > With this change, it now can show kernel symbols again:
> > 
> >   # Overhead  Command  Shared Object      Symbol
> >   # ........  .......  .................  ........................
> >   #
> >       34.51%  swapper  [kernel.kallsyms]  [k] intel_idle
> >       12.54%  perf     [kernel.kallsyms]  [k] generic_exec_single
> >       10.11%  swapper  [kernel.kallsyms]  [k] int_sqrt
> >        9.83%  swapper  [kernel.kallsyms]  [k] hrtimer_interrupt
> >        7.25%  emacs    [kernel.kallsyms]  [k] __switch_to
> >        7.06%  sleep    [kernel.kallsyms]  [k] find_next_iomem_res
> >        7.02%  swapper  [kernel.kallsyms]  [k] run_timer_softirq
> >        2.55%  swapper  [kernel.kallsyms]  [k] _raw_spin_unlock_irq
> >        1.90%  swapper  [kernel.kallsyms]  [k] lapic_next_deadline
> >        1.75%  swapper  [kernel.kallsyms]  [k] native_sched_clock
> >        1.49%  swapper  [kernel.kallsyms]  [k] __schedule
> >        1.20%  swapper  [kernel.kallsyms]  [k] read_tsc
> >        1.10%  sleep    [kernel.kallsyms]  [k] current_kernel_time
> >        0.99%  swapper  [kernel.kallsyms]  [k] pick_next_task_rt
> >        0.36%  swapper  [kernel.kallsyms]  [k] group_sched_in
> >        0.24%  swapper  [kernel.kallsyms]  [k] x86_pmu_commit_txn
> >        0.06%  swapper  [kernel.kallsyms]  [k] intel_pmu_enable_all
> >        0.03%  perf     [kernel.kallsyms]  [k] intel_pmu_enable_all
> > 
> > Reported-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/symbol-minimal.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> > index c9541fea9514..226cf41ed7e6 100644
> > --- a/tools/perf/util/symbol-minimal.c
> > +++ b/tools/perf/util/symbol-minimal.c
> > @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> >  	unsigned char *build_id[BUILD_ID_SIZE];
> >  	int ret;
> >  
> > +	if (dso->kernel)
> > +		return 0;  /* always use kallsyms */
> > +
> >  	ret = fd__is_64_bit(ss->fd);
> >  	if (ret >= 0)
> >  		dso->is_64_bit = ret;
> 
> Why does this live in the minimal implementation; should we not always
> discard ELF files with 0 symbols?
> 
> Suppose I have a vmlinux but removed all symbols from it; I want it to
> fall back to kallsyms too.

I'm not sure I understood what you said correctly.  With this change,
dso__load_kernel_sym() always ends up calling dso__load_kallsyms() since
dso__load_vmlinux() will always return 0;

So I think you'll fall back to kallsyms even though you have a vmlinux
with symbol.  This makes dso__load_sym() in the patch 2/3 simpler IMHO.

Thanks,
Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Peter Zijlstra]

On Fri, Nov 07, 2014 at 11:57:39PM +0900, Namhyung Kim wrote:

> > Why does this live in the minimal implementation; should we not always
> > discard ELF files with 0 symbols?
> > 
> > Suppose I have a vmlinux but removed all symbols from it; I want it to
> > fall back to kallsyms too.
> 
> I'm not sure I understood what you said correctly.  With this change,
> dso__load_kernel_sym() always ends up calling dso__load_kallsyms() since
> dso__load_vmlinux() will always return 0;
> 
> So I think you'll fall back to kallsyms even though you have a vmlinux
> with symbol.  This makes dso__load_sym() in the patch 2/3 simpler IMHO.

But why have it specific to the minimal elf thing? Why not discard any
DSO with 0 symbols and try the next option to acquire symbols?

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

Hi Peter,

On Fri, 7 Nov 2014 18:37:18 +0100, Peter Zijlstra wrote:
> On Fri, Nov 07, 2014 at 11:57:39PM +0900, Namhyung Kim wrote:
>
>> > Why does this live in the minimal implementation; should we not always
>> > discard ELF files with 0 symbols?
>> > 
>> > Suppose I have a vmlinux but removed all symbols from it; I want it to
>> > fall back to kallsyms too.
>> 
>> I'm not sure I understood what you said correctly.  With this change,
>> dso__load_kernel_sym() always ends up calling dso__load_kallsyms() since
>> dso__load_vmlinux() will always return 0;
>> 
>> So I think you'll fall back to kallsyms even though you have a vmlinux
>> with symbol.  This makes dso__load_sym() in the patch 2/3 simpler IMHO.
>
> But why have it specific to the minimal elf thing? Why not discard any
> DSO with 0 symbols and try the next option to acquire symbols?

Hmm.. I don't think it's specific to the minimal elf parser.  The return
value of dso__load_sym() is a number of symbols found so when it sees a
dso with 0 symbols it'll fall back to the next option IMHO (not
tested).  Did you see a problem with the current code?

Thanks,
Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Peter Zijlstra]

On Mon, Nov 10, 2014 at 03:33:11PM +0900, Namhyung Kim wrote:
> Hmm.. I don't think it's specific to the minimal elf parser.  The return
> value of dso__load_sym() is a number of symbols found so when it sees a
> dso with 0 symbols it'll fall back to the next option IMHO (not
> tested).  Did you see a problem with the current code?

So your patch:

+++ b/tools/perf/util/symbol-minimal.c
@@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
        unsigned char *build_id[BUILD_ID_SIZE];
        int ret;

+       if (dso->kernel)
+               return 0;  /* always use kallsyms */
+

changes the symbol-minimal.c file to add this exception. That is very
much specific to the minimal elf parser, or am I just seeing things?

What I was saying, why not have a util/symbol.c change that disregards
all DSOs with 0 symbols in.

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

Hi Peter,

On Mon, Nov 10, 2014 at 9:11 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Nov 10, 2014 at 03:33:11PM +0900, Namhyung Kim wrote:
>> Hmm.. I don't think it's specific to the minimal elf parser.  The return
>> value of dso__load_sym() is a number of symbols found so when it sees a
>> dso with 0 symbols it'll fall back to the next option IMHO (not
>> tested).  Did you see a problem with the current code?
>
> So your patch:
>
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>         unsigned char *build_id[BUILD_ID_SIZE];
>         int ret;
>
> +       if (dso->kernel)
> +               return 0;  /* always use kallsyms */
> +
>
> changes the symbol-minimal.c file to add this exception. That is very
> much specific to the minimal elf parser, or am I just seeing things?

What minimal parser does here is just skip kernel dsos (vmlinux) since
it didn't deal with all the details of parsing vmlinux currently.


>
> What I was saying, why not have a util/symbol.c change that disregards
> all DSOs with 0 symbols in.

The util/symbol.c doesn't need this because it can handle vmlinux
reliably.  So after reading symbol table, it'll use the dso if it
actually contains symbols or fallback to next dso if it has 0 symbols.
IOW it already disregards all dsos with 0 symbols in.

-- 
Thanks,
Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Peter Zijlstra]

On Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim wrote:
> > What I was saying, why not have a util/symbol.c change that disregards
> > all DSOs with 0 symbols in.
> 
> The util/symbol.c doesn't need this because it can handle vmlinux
> reliably.  So after reading symbol table, it'll use the dso if it
> actually contains symbols or fallback to next dso if it has 0 symbols.
> IOW it already disregards all dsos with 0 symbols in.

Well, it could not could it.. it readily proceeded with 0 symbols in my
case. It did _not_ fallback to kallsyms.

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Arnaldo Carvalho de Melo]

Em Tue, Nov 11, 2014 at 11:27:45AM +0100, Peter Zijlstra escreveu:
> On Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim wrote:
> > > What I was saying, why not have a util/symbol.c change that disregards
> > > all DSOs with 0 symbols in.

> > The util/symbol.c doesn't need this because it can handle vmlinux
> > reliably.  So after reading symbol table, it'll use the dso if it
> > actually contains symbols or fallback to next dso if it has 0 symbols.
> > IOW it already disregards all dsos with 0 symbols in.

> Well, it could not could it.. it readily proceeded with 0 symbols in my
> case. It did _not_ fallback to kallsyms.

Right, testing a fix for the problem I described on my last message.

- Arnaldo

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Arnaldo Carvalho de Melo]

Em Tue, Nov 11, 2014 at 10:03:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 11, 2014 at 11:27:45AM +0100, Peter Zijlstra escreveu:
> > On Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim wrote:
> > > > What I was saying, why not have a util/symbol.c change that disregards
> > > > all DSOs with 0 symbols in.
> 
> > > The util/symbol.c doesn't need this because it can handle vmlinux
> > > reliably.  So after reading symbol table, it'll use the dso if it
> > > actually contains symbols or fallback to next dso if it has 0 symbols.
> > > IOW it already disregards all dsos with 0 symbols in.
> 
> > Well, it could not could it.. it readily proceeded with 0 symbols in my
> > case. It did _not_ fallback to kallsyms.
> 
> Right, testing a fix for the problem I described on my last message.

Can you try the patch below?

- Arnaldo

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index c9541fea9514..b55f96d49503 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -341,7 +341,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 
 	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
 		dso__set_build_id(dso, build_id);
-		return 1;
+		return 0;
 	}
 	return 0;
 }

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Peter Zijlstra]

On Tue, Nov 11, 2014 at 11:02:00AM -0300, Arnaldo Carvalho de Melo wrote:
> Can you try the patch below?
> 

> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index c9541fea9514..b55f96d49503 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -341,7 +341,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>  
>  	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
>  		dso__set_build_id(dso, build_id);
> -		return 1;
> +		return 0;
>  	}
>  	return 0;
>  }

removing libelf-dev and recompiling to verify its broken again, then I
made this change, recompiled and tried again and that seems to work.

Control flow seems redundant though, might as well remove the entire
return stmt.

Still wondering why the generic code accepts DSOs without symbols
though. But whatever ;-)

[tip:perf/core] perf symbols: Fallback to kallsyms when using the minimal 'ELF' loader

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  162bcc17bb876772793ca070ebd6488cfdae09bf
Gitweb:     http://git.kernel.org/tip/162bcc17bb876772793ca070ebd6488cfdae09bf
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Tue, 11 Nov 2014 11:25:28 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 19 Nov 2014 12:33:46 -0300

perf symbols: Fallback to kallsyms when using the minimal 'ELF' loader

The minimal ELF loader should not return 1 when it manages to read the
vmlinux build-id, it should instead return 0, meaning that it hasn't
loaded any symbols, since it doesn't parses ELF at all.

That way, the main symbol.c routines will understand that it is
necessary to continue looking for a file with symbols, and when no
libelf is linked, that means it will eventually try kallsyms.

Reported-by: Peter Zijlstra <peterz@infradead.org>
Tested-by: Peter Zijlstra <peterz@infradead.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/20141111130326.GT18464@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-minimal.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index c9541fe..fa585c6 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -341,7 +341,6 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 
 	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
 		dso__set_build_id(dso, build_id);
-		return 1;
 	}
 	return 0;
 }

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Arnaldo Carvalho de Melo]

Em Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim escreveu:
> Hi Peter,
> 
> On Mon, Nov 10, 2014 at 9:11 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Nov 10, 2014 at 03:33:11PM +0900, Namhyung Kim wrote:
> >> Hmm.. I don't think it's specific to the minimal elf parser.  The return
> >> value of dso__load_sym() is a number of symbols found so when it sees a
> >> dso with 0 symbols it'll fall back to the next option IMHO (not
> >> tested).  Did you see a problem with the current code?
> >
> > So your patch:
> >
> > +++ b/tools/perf/util/symbol-minimal.c
> > @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> >         unsigned char *build_id[BUILD_ID_SIZE];
> >         int ret;
> >
> > +       if (dso->kernel)
> > +               return 0;  /* always use kallsyms */
> > +
> >
> > changes the symbol-minimal.c file to add this exception. That is very
> > much specific to the minimal elf parser, or am I just seeing things?
> 
> What minimal parser does here is just skip kernel dsos (vmlinux) since
> it didn't deal with all the details of parsing vmlinux currently.
> 
> 
> >
> > What I was saying, why not have a util/symbol.c change that disregards
> > all DSOs with 0 symbols in.
> 
> The util/symbol.c doesn't need this because it can handle vmlinux
> reliably.  So after reading symbol table, it'll use the dso if it

symbol.c should not be able to handle anything, since the actual ELF
loader is in symbol-elf.c or simbol-minimal.c, no?

I.e.:

	symbol.c::dso__load()
		symbol.c::dso__load_kernel_sym()
			symbol.c::dso__load_vmlinux()

And then it heads into one of the ELF loaders, right now, without your
patch, I see, when the minimal loader is used:

		symbol-minimal.c::dso__load_sym()

reads the build-id and if it works, returns 1, which is a bug, it should
return 0 in this case, possibly -1 if it doesn't read the build-id :-\

I.e. it should return 0, because that way it signals: "I can't read it,
thus no symbols were loaded, symbol.c: please go on looking for them
somewhere else, kallsyms perhaps?".

And this is the bug, probably. Now to look at your patches to see if
this is touched somehow...

> actually contains symbols or fallback to next dso if it has 0 symbols.
> IOW it already disregards all dsos with 0 symbols in.

See above.
 
> -- 
> Thanks,
> Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

Hi Arnaldo,

Sorry for late reply.  I was offline last week.


On Tue, 11 Nov 2014 10:02:29 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim escreveu:
>> Hi Peter,
>> 
>> On Mon, Nov 10, 2014 at 9:11 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Mon, Nov 10, 2014 at 03:33:11PM +0900, Namhyung Kim wrote:
>> >> Hmm.. I don't think it's specific to the minimal elf parser.  The return
>> >> value of dso__load_sym() is a number of symbols found so when it sees a
>> >> dso with 0 symbols it'll fall back to the next option IMHO (not
>> >> tested).  Did you see a problem with the current code?
>> >
>> > So your patch:
>> >
>> > +++ b/tools/perf/util/symbol-minimal.c
>> > @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>> >         unsigned char *build_id[BUILD_ID_SIZE];
>> >         int ret;
>> >
>> > +       if (dso->kernel)
>> > +               return 0;  /* always use kallsyms */
>> > +
>> >
>> > changes the symbol-minimal.c file to add this exception. That is very
>> > much specific to the minimal elf parser, or am I just seeing things?
>> 
>> What minimal parser does here is just skip kernel dsos (vmlinux) since
>> it didn't deal with all the details of parsing vmlinux currently.
>> 
>> 
>> >
>> > What I was saying, why not have a util/symbol.c change that disregards
>> > all DSOs with 0 symbols in.
>> 
>> The util/symbol.c doesn't need this because it can handle vmlinux
>> reliably.  So after reading symbol table, it'll use the dso if it
>
> symbol.c should not be able to handle anything, since the actual ELF
> loader is in symbol-elf.c or simbol-minimal.c, no?

Ah right, I was confused with symbol.c and symbol-elf.c - so I believe
the generic logic in symbol.c already handles DSOs with 0 symbol.  The
problem was symbol-minimal.c didn't return 0 for the DSOs.


>
> I.e.:
>
> 	symbol.c::dso__load()
> 		symbol.c::dso__load_kernel_sym()
> 			symbol.c::dso__load_vmlinux()
>
> And then it heads into one of the ELF loaders, right now, without your
> patch, I see, when the minimal loader is used:
>
> 		symbol-minimal.c::dso__load_sym()
>
> reads the build-id and if it works, returns 1, which is a bug, it should
> return 0 in this case, possibly -1 if it doesn't read the build-id :-\
>
> I.e. it should return 0, because that way it signals: "I can't read it,
> thus no symbols were loaded, symbol.c: please go on looking for them
> somewhere else, kallsyms perhaps?".

Right.  It's a bug for vmlinux.  But by returning 0, it'll iterate over
the candidate binaries in dso__load().  I'm afraid that this *might*
overwrite the build-id of the DSO with different one.  So I think it
also needs to add a check for valid build-id.

I'll update this patch with above.

Thanks,
Namhyung


>
> And this is the bug, probably. Now to look at your patches to see if
> this is touched somehow...
>
>> actually contains symbols or fallback to next dso if it has 0 symbols.
>> IOW it already disregards all dsos with 0 symbols in.
>
> See above.
>  
>> -- 
>> Thanks,
>> Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[David Ahern]

On 11/6/14, 10:20 PM, Namhyung Kim wrote:
> It worked w/o libelf, but recent change to use vmlinux for kernel
> symbols break it.

Do you know the recent change (commit id) that broke it?

David

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

Hi David,

On Fri, 07 Nov 2014 08:21:16 -0700, David Ahern wrote:
> On 11/6/14, 10:20 PM, Namhyung Kim wrote:
>> It worked w/o libelf, but recent change to use vmlinux for kernel
>> symbols break it.
>
> Do you know the recent change (commit id) that broke it?

Nope.  I tried to find it but failed - older versions seems also have
the problem and some of them could not run perf record.  Anyway it looks
like depending some external condition and broken for a while. :(

Thanks,
Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[David Ahern]

On 11/10/14, 12:40 AM, Namhyung Kim wrote:
> Hi David,
>
> On Fri, 07 Nov 2014 08:21:16 -0700, David Ahern wrote:
>> On 11/6/14, 10:20 PM, Namhyung Kim wrote:
>>> It worked w/o libelf, but recent change to use vmlinux for kernel
>>> symbols break it.
>>
>> Do you know the recent change (commit id) that broke it?
>
> Nope.  I tried to find it but failed - older versions seems also have
> the problem and some of them could not run perf record.  Anyway it looks
> like depending some external condition and broken for a while. :(

OK. I have a segfault issue when analyzing a mips file offbox and it is 
related to kernel symbol processing. I was not able to find past version 
of perf that worked - though limited by the file version checks added 
around v3.6.

David

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Arnaldo Carvalho de Melo]

Em Fri, Nov 07, 2014 at 02:20:04PM +0900, Namhyung Kim escreveu:
> When libelf is not used, perf cannot not show symbol names since it
> doesn't access the ELF symbol table.  But kernel is different as it
> can fallback to kallsyms.
> 
> It worked w/o libelf, but recent change to use vmlinux for kernel
> symbols break it.
> 
> With this change, it now can show kernel symbols again:

Ok, but since you added that minimal ELF symtab loading, isn't better to
try that first, i.e. if we find a vmlinux file with a build-id and with
symbols in it...

If that isn't the case, i.e. no vmlinux was found, then we fallback to
kallsyms, to check if that is available.

I.e. with your new minimalistic ELF symtab loader if we have a suitable
vmlinux but no kallsyms, we end up resolving no symbols even having that
nice vmlinux :-)

Ah, I applied the feature test one (2/3, I think)

- Arnaldo

>   # Overhead  Command  Shared Object      Symbol
>   # ........  .......  .................  ........................
>   #
>       34.51%  swapper  [kernel.kallsyms]  [k] intel_idle
>       12.54%  perf     [kernel.kallsyms]  [k] generic_exec_single
>       10.11%  swapper  [kernel.kallsyms]  [k] int_sqrt
>        9.83%  swapper  [kernel.kallsyms]  [k] hrtimer_interrupt
>        7.25%  emacs    [kernel.kallsyms]  [k] __switch_to
>        7.06%  sleep    [kernel.kallsyms]  [k] find_next_iomem_res
>        7.02%  swapper  [kernel.kallsyms]  [k] run_timer_softirq
>        2.55%  swapper  [kernel.kallsyms]  [k] _raw_spin_unlock_irq
>        1.90%  swapper  [kernel.kallsyms]  [k] lapic_next_deadline
>        1.75%  swapper  [kernel.kallsyms]  [k] native_sched_clock
>        1.49%  swapper  [kernel.kallsyms]  [k] __schedule
>        1.20%  swapper  [kernel.kallsyms]  [k] read_tsc
>        1.10%  sleep    [kernel.kallsyms]  [k] current_kernel_time
>        0.99%  swapper  [kernel.kallsyms]  [k] pick_next_task_rt
>        0.36%  swapper  [kernel.kallsyms]  [k] group_sched_in
>        0.24%  swapper  [kernel.kallsyms]  [k] x86_pmu_commit_txn
>        0.06%  swapper  [kernel.kallsyms]  [k] intel_pmu_enable_all
>        0.03%  perf     [kernel.kallsyms]  [k] intel_pmu_enable_all
> 
> Reported-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/symbol-minimal.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index c9541fea9514..226cf41ed7e6 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>  	unsigned char *build_id[BUILD_ID_SIZE];
>  	int ret;
>  
> +	if (dso->kernel)
> +		return 0;  /* always use kallsyms */
> +
>  	ret = fd__is_64_bit(ss->fd);
>  	if (ret >= 0)
>  		dso->is_64_bit = ret;
> -- 
> 2.1.2

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

On Fri, 7 Nov 2014 12:29:31 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 07, 2014 at 02:20:04PM +0900, Namhyung Kim escreveu:
>> When libelf is not used, perf cannot not show symbol names since it
>> doesn't access the ELF symbol table.  But kernel is different as it
>> can fallback to kallsyms.
>> 
>> It worked w/o libelf, but recent change to use vmlinux for kernel
>> symbols break it.
>> 
>> With this change, it now can show kernel symbols again:
>
> Ok, but since you added that minimal ELF symtab loading, isn't better to
> try that first, i.e. if we find a vmlinux file with a build-id and with
> symbols in it...
>
> If that isn't the case, i.e. no vmlinux was found, then we fallback to
> kallsyms, to check if that is available.
>
> I.e. with your new minimalistic ELF symtab loader if we have a suitable
> vmlinux but no kallsyms, we end up resolving no symbols even having that
> nice vmlinux :-)

Yeah, maybe.  But it'd add a substantial complexity also and I tried to
make it simple and small only to show usual userspace symbols.

I think that about a half of the complexity of the dso__load_sym() in
symbol-elf.c came from the kernel (and module) support.  And expecting
kallsyms on the system is not a high barrier IMHO.  So I decided to just
fall back to kallsyms for kernel dsos.  Mayby we can add the kernel
support incrementally later.

Thanks,
Namhyung

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Arnaldo Carvalho de Melo]

Em Mon, Nov 10, 2014 at 03:53:06PM +0900, Namhyung Kim escreveu:
> On Fri, 7 Nov 2014 12:29:31 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Nov 07, 2014 at 02:20:04PM +0900, Namhyung Kim escreveu:
> >> When libelf is not used, perf cannot not show symbol names since it
> >> doesn't access the ELF symbol table.  But kernel is different as it
> >> can fallback to kallsyms.

> >> It worked w/o libelf, but recent change to use vmlinux for kernel
> >> symbols break it.

> >> With this change, it now can show kernel symbols again:

> > Ok, but since you added that minimal ELF symtab loading, isn't better to
> > try that first, i.e. if we find a vmlinux file with a build-id and with
> > symbols in it...

> > If that isn't the case, i.e. no vmlinux was found, then we fallback to
> > kallsyms, to check if that is available.

> > I.e. with your new minimalistic ELF symtab loader if we have a suitable
> > vmlinux but no kallsyms, we end up resolving no symbols even having that
> > nice vmlinux :-)
> 
> Yeah, maybe.  But it'd add a substantial complexity also and I tried to
> make it simple and small only to show usual userspace symbols.
> 
> I think that about a half of the complexity of the dso__load_sym() in
> symbol-elf.c came from the kernel (and module) support.  And expecting

Right, I agree that thing grew too complex, reducing that complexity is
something we should do when we have the chance. I.e. perhaps we could
separate kernel/modules handling out of it, even if just at source code
level at first...

> kallsyms on the system is not a high barrier IMHO.  So I decided to just
> fall back to kallsyms for kernel dsos.  Mayby we can add the kernel
> support incrementally later.

The suggestion came because you did that minimalistic reader, which if
can be used for vmlinux files, would be great, as resolving vmlinux
samples is such a major usecase :-)

- Arnaldo

Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Namhyung Kim]

On Mon, 10 Nov 2014 09:11:41 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 10, 2014 at 03:53:06PM +0900, Namhyung Kim escreveu:
>> On Fri, 7 Nov 2014 12:29:31 -0300, Arnaldo Carvalho de Melo wrote:
>> > Em Fri, Nov 07, 2014 at 02:20:04PM +0900, Namhyung Kim escreveu:
>> >> When libelf is not used, perf cannot not show symbol names since it
>> >> doesn't access the ELF symbol table.  But kernel is different as it
>> >> can fallback to kallsyms.
>
>> >> It worked w/o libelf, but recent change to use vmlinux for kernel
>> >> symbols break it.
>
>> >> With this change, it now can show kernel symbols again:
>
>> > Ok, but since you added that minimal ELF symtab loading, isn't better to
>> > try that first, i.e. if we find a vmlinux file with a build-id and with
>> > symbols in it...
>
>> > If that isn't the case, i.e. no vmlinux was found, then we fallback to
>> > kallsyms, to check if that is available.
>
>> > I.e. with your new minimalistic ELF symtab loader if we have a suitable
>> > vmlinux but no kallsyms, we end up resolving no symbols even having that
>> > nice vmlinux :-)
>> 
>> Yeah, maybe.  But it'd add a substantial complexity also and I tried to
>> make it simple and small only to show usual userspace symbols.
>> 
>> I think that about a half of the complexity of the dso__load_sym() in
>> symbol-elf.c came from the kernel (and module) support.  And expecting
>
> Right, I agree that thing grew too complex, reducing that complexity is
> something we should do when we have the chance. I.e. perhaps we could
> separate kernel/modules handling out of it, even if just at source code
> level at first...

Hmm.. okay.  I'll try.

Thanks,
Namhyung


>
>> kallsyms on the system is not a high barrier IMHO.  So I decided to just
>> fall back to kallsyms for kernel dsos.  Mayby we can add the kernel
>> support incrementally later.
>
> The suggestion came because you did that minimalistic reader, which if
> can be used for vmlinux files, would be great, as resolving vmlinux
> samples is such a major usecase :-)