linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH perf/core 0/3] perf-probe: Fix offline module and cross-arch support
@ 2017-01-04  3:27 Masami Hiramatsu
  2017-01-04  3:29 ` [PATCH perf/core 1/3] perf-probe: Fix --funcs to show correct symbols for offline module Masami Hiramatsu
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-04  3:27 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Jiri Olsa, Peter Zijlstra,
	Ingo Molnar, Namhyung Kim

Hello,

This series fixes some issues on offline module/cross-arch
probe support. While I was debuging arm32 kernel with
cross-arch perf-probe, I found several issues.

- perf-probe didn't show correct probe definition on
  gcc-generated symbols (on both of kernel and module).
- perf-probe didn't show available functions for offline
  module even if full path of the module was given.

This series fixes those issues.

---

Masami Hiramatsu (3):
      perf-probe: Fix --funcs to show correct symbols for offline module
      perf-probe: Fix to probe on gcc generated symbols for offline kernel
      perf-probe: Fix to probe on gcc generated functions in modules


 tools/perf/util/probe-event.c  |  111 +++++++++++++++++++++++++++-------------
 tools/perf/util/probe-finder.c |    2 -
 tools/perf/util/probe-finder.h |    2 +
 3 files changed, 79 insertions(+), 36 deletions(-)

--
Masami Hiramatsu (Linaro)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH perf/core 1/3] perf-probe: Fix --funcs to show correct symbols for offline module
  2017-01-04  3:27 [PATCH perf/core 0/3] perf-probe: Fix offline module and cross-arch support Masami Hiramatsu
@ 2017-01-04  3:29 ` Masami Hiramatsu
  2017-01-05  7:54   ` [tip:perf/urgent] perf probe: " tip-bot for Masami Hiramatsu
  2017-01-04  3:30 ` [PATCH perf/core 2/3] perf-probe: Fix to probe on gcc generated symbols for offline kernel Masami Hiramatsu
  2017-01-04  3:31 ` [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules Masami Hiramatsu
  2 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-04  3:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Jiri Olsa, Peter Zijlstra,
	Ingo Molnar, Namhyung Kim

Fix --funcs (-F) option to show correct symbols for
offline module. Since previous perf-probe uses
machine__findnew_module_map() for offline module,
even if user passes a module file (with full path)
which is for other architecture, perf-probe always
tries to load symbol map for current kernel module.

This fix uses dso__new_map() to load the map from
given binary as same as a map for user applications.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |   25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8f81096..542e647 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -163,7 +163,7 @@ static struct map *kernel_get_module_map(const char *module)
 
 	/* A file path -- this is an offline module */
 	if (module && strchr(module, '/'))
-		return machine__findnew_module_map(host_machine, 0, module);
+		return dso__new_map(module);
 
 	if (!module)
 		module = "kernel";
@@ -173,6 +173,7 @@ static struct map *kernel_get_module_map(const char *module)
 		if (strncmp(pos->dso->short_name + 1, module,
 			    pos->dso->short_name_len - 2) == 0 &&
 		    module[pos->dso->short_name_len - 2] == '\0') {
+			map__get(pos);
 			return pos;
 		}
 	}
@@ -188,15 +189,6 @@ struct map *get_target_map(const char *target, bool user)
 		return kernel_get_module_map(target);
 }
 
-static void put_target_map(struct map *map, bool user)
-{
-	if (map && user) {
-		/* Only the user map needs to be released */
-		map__put(map);
-	}
-}
-
-
 static int convert_exec_to_group(const char *exec, char **result)
 {
 	char *ptr1, *ptr2, *exec_copy;
@@ -412,7 +404,7 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
 	}
 
 out:
-	put_target_map(map, uprobes);
+	map__put(map);
 	return ret;
 
 }
@@ -2869,7 +2861,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	}
 
 out:
-	put_target_map(map, pev->uprobes);
+	map__put(map);
 	free(syms);
 	return ret;
 
@@ -3362,10 +3354,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
 		return ret;
 
 	/* Get a symbol map */
-	if (user)
-		map = dso__new_map(target);
-	else
-		map = kernel_get_module_map(target);
+	map = get_target_map(target, user);
 	if (!map) {
 		pr_err("Failed to get a map for %s\n", (target) ? : "kernel");
 		return -EINVAL;
@@ -3397,9 +3386,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
         }
 
 end:
-	if (user) {
-		map__put(map);
-	}
+	map__put(map);
 	exit_probe_symbol_maps();
 
 	return ret;

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH perf/core 2/3] perf-probe: Fix to probe on gcc generated symbols for offline kernel
  2017-01-04  3:27 [PATCH perf/core 0/3] perf-probe: Fix offline module and cross-arch support Masami Hiramatsu
  2017-01-04  3:29 ` [PATCH perf/core 1/3] perf-probe: Fix --funcs to show correct symbols for offline module Masami Hiramatsu
@ 2017-01-04  3:30 ` Masami Hiramatsu
  2017-01-05  7:54   ` [tip:perf/urgent] perf probe: " tip-bot for Masami Hiramatsu
  2017-01-04  3:31 ` [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules Masami Hiramatsu
  2 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-04  3:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Jiri Olsa, Peter Zijlstra,
	Ingo Molnar, Namhyung Kim

Fix perf-probe to show probe definition on gcc generated symbols
for offline kernel (including cross-arch kernel image).

Gcc sometimes optimizes functions and generate new symbols with
suffixes such as ".constprop.N" or ".isra.N" etc. Since those
symbol names are not recorded in DWARF, we have to find correct
generated symbols from offline ELF binary to probe on it (kallsyms
doesn't correct it).
For online kernel or uprobes we don't need it because those are
rebased on _text, or a section relative address.

E.g. Without this;
  -----
  $ perf probe -k build-arm/vmlinux -F __slab_alloc*
  __slab_alloc.constprop.9
  $ perf probe -k build-arm/vmlinux -D __slab_alloc
  p:probe/__slab_alloc __slab_alloc+0
  -----
If you put above definition on target machine, it should fail
because there is no __slab_alloc in kallsyms.

With this fix, perf probe shows correct probe definition on
__slab_alloc.constprop.9.
  -----
  $ perf probe -k build-arm/vmlinux -D __slab_alloc
  p:probe/__slab_alloc __slab_alloc.constprop.9+0
  -----

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |   48 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 542e647..4a57c8a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -610,6 +610,51 @@ static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp,
 	return ret ? : -ENOENT;
 }
 
+/*
+ * Rename DWARF symbols to ELF symbols -- gcc sometimes optimizes functions
+ * and generate new symbols with suffixes such as .constprop.N or .isra.N
+ * etc. Since those symbols are not recorded in DWARF, we have to find
+ * correct generated symbols from offline ELF binary.
+ * For online kernel or uprobes we don't need this because those are
+ * rebased on _text, or already a section relative address.
+ */
+static int
+post_process_offline_probe_trace_events(struct probe_trace_event *tevs,
+					int ntevs, const char *pathname)
+{
+	struct symbol *sym;
+	struct map *map;
+	unsigned long stext = 0;
+	u64 addr;
+	int i;
+
+	/* Prepare a map for offline binary */
+	map = dso__new_map(pathname);
+	if (!map || get_text_start_address(pathname, &stext) < 0) {
+		pr_warning("Failed to get ELF symbols for %s\n", pathname);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ntevs; i++) {
+		addr = tevs[i].point.address + tevs[i].point.offset - stext;
+		sym = map__find_symbol(map, addr);
+		if (!sym)
+			continue;
+		if (!strcmp(sym->name, tevs[i].point.symbol))
+			continue;
+		/* If we have no realname, use symbol for it */
+		if (!tevs[i].point.realname)
+			tevs[i].point.realname = tevs[i].point.symbol;
+		else
+			free(tevs[i].point.symbol);
+		tevs[i].point.symbol = strdup(sym->name);
+		tevs[i].point.offset = addr - sym->start;
+	}
+	map__put(map);
+
+	return 0;
+}
+
 static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
 					  int ntevs, const char *exec)
 {
@@ -671,7 +716,8 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 
 	/* Skip post process if the target is an offline kernel */
 	if (symbol_conf.ignore_vmlinux_buildid)
-		return 0;
+		return post_process_offline_probe_trace_events(tevs, ntevs,
+						symbol_conf.vmlinux_name);
 
 	reloc_sym = kernel_get_ref_reloc_sym();
 	if (!reloc_sym) {

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-04  3:27 [PATCH perf/core 0/3] perf-probe: Fix offline module and cross-arch support Masami Hiramatsu
  2017-01-04  3:29 ` [PATCH perf/core 1/3] perf-probe: Fix --funcs to show correct symbols for offline module Masami Hiramatsu
  2017-01-04  3:30 ` [PATCH perf/core 2/3] perf-probe: Fix to probe on gcc generated symbols for offline kernel Masami Hiramatsu
@ 2017-01-04  3:31 ` Masami Hiramatsu
  2017-01-04 14:48   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-04  3:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Jiri Olsa, Peter Zijlstra,
	Ingo Molnar, Namhyung Kim

Fix to probe on gcc generated functions on modules. Since
probing on a module is based on its symbol name, it should
be adjusted on actual symbols.

E.g. without this fix, perf probe shows probe definition
on non-exist symbol as below.
  -----
  $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -F in_range*
  in_range.isra.12
  $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
  p:probe/in_range nf_nat:in_range+0
  -----
With this fix, perf probe correctly shows a probe on
gcc-generated symbol.
  -----
  $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
  p:probe/in_range nf_nat:in_range.isra.12+0
  -----

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c  |   38 +++++++++++++++++++++++---------------
 tools/perf/util/probe-finder.c |    2 +-
 tools/perf/util/probe-finder.h |    2 ++
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4a57c8a..f75c99a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -682,15 +682,19 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
 	return ret;
 }
 
-static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
-					    int ntevs, const char *module)
+static int
+post_process_module_probe_trace_events(struct probe_trace_event *tevs,
+				       int ntevs, const char *module,
+				       struct debuginfo *dinfo)
 {
+	Dwarf_Addr text_offs;
 	int i, ret = 0;
 	char *mod_name = NULL;
 
 	if (!module)
 		return 0;
 
+	debuginfo__get_text_offset(dinfo, &text_offs);
 	mod_name = find_module_name(module);
 
 	for (i = 0; i < ntevs; i++) {
@@ -700,9 +704,15 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
 			ret = -ENOMEM;
 			break;
 		}
+		/* kernel module needs a special care to adjust addresses */
+		tevs[i].point.address -= (unsigned long)text_offs;
 	}
 
 	free(mod_name);
+
+	if (!ret)
+		ret = post_process_offline_probe_trace_events(tevs, ntevs,
+								module);
 	return ret;
 }
 
@@ -760,7 +770,7 @@ arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unuse
 static int post_process_probe_trace_events(struct perf_probe_event *pev,
 					   struct probe_trace_event *tevs,
 					   int ntevs, const char *module,
-					   bool uprobe)
+					   bool uprobe, struct debuginfo *dinfo)
 {
 	int ret;
 
@@ -768,7 +778,8 @@ static int post_process_probe_trace_events(struct perf_probe_event *pev,
 		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
 	else if (module)
 		/* Currently ref_reloc_sym based probe is not for drivers */
-		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
+		ret = post_process_module_probe_trace_events(tevs, ntevs,
+							     module, dinfo);
 	else
 		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
 
@@ -812,30 +823,27 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 		}
 	}
 
-	debuginfo__delete(dinfo);
-
 	if (ntevs > 0) {	/* Succeeded to find trace events */
 		pr_debug("Found %d probe_trace_events.\n", ntevs);
 		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
-						pev->target, pev->uprobes);
+					pev->target, pev->uprobes, dinfo);
 		if (ret < 0 || ret == ntevs) {
+			pr_debug("Post processing failed or all events are skipped. (%d)\n", ret);
 			clear_probe_trace_events(*tevs, ntevs);
 			zfree(tevs);
+			ntevs = 0;
 		}
-		if (ret != ntevs)
-			return ret < 0 ? ret : ntevs;
-		ntevs = 0;
-		/* Fall through */
 	}
 
+	debuginfo__delete(dinfo);
+
 	if (ntevs == 0)	{	/* No error but failed to find probe point. */
 		pr_warning("Probe point '%s' not found.\n",
 			   synthesize_perf_probe_point(&pev->point));
 		return -ENOENT;
-	}
-	/* Error path : ntevs < 0 */
-	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
-	if (ntevs < 0) {
+	} else if (ntevs < 0) {
+		/* Error path : ntevs < 0 */
+		pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
 		if (ntevs == -EBADF)
 			pr_warning("Warning: No dwarf info found in the vmlinux - "
 				"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index df4debe..6dede24 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1501,7 +1501,7 @@ int debuginfo__find_available_vars_at(struct debuginfo *dbg,
 }
 
 /* For the kernel module, we need a special code to get a DIE */
-static int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
+int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
 {
 	int n, i;
 	Elf32_Word shndx;
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index f1d8558..1e03bbb 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -46,6 +46,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
 int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
 				struct perf_probe_point *ppt);
 
+int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs);
+
 /* Find a line range */
 int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr);
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-04  3:31 ` [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules Masami Hiramatsu
@ 2017-01-04 14:48   ` Arnaldo Carvalho de Melo
  2017-01-05 11:20     ` Masami Hiramatsu
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-01-04 14:48 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

Em Wed, Jan 04, 2017 at 12:31:39PM +0900, Masami Hiramatsu escreveu:
> Fix to probe on gcc generated functions on modules. Since
> probing on a module is based on its symbol name, it should
> be adjusted on actual symbols.
> 
> E.g. without this fix, perf probe shows probe definition
> on non-exist symbol as below.
>   -----
>   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -F in_range*
>   in_range.isra.12
>   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
>   p:probe/in_range nf_nat:in_range+0
>   -----
> With this fix, perf probe correctly shows a probe on
> gcc-generated symbol.
>   -----
>   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
>   p:probe/in_range nf_nat:in_range.isra.12+0
>   -----

Tested this one on a x86-64 fedora25 system, applied all three.

As always, please take a look at the patch when sent to Ingo, I usually
put some committer notes there, in this case I put the test steps I
performed, using e1000e.ko and e1000_flash_cycle_ich8lan.constprop.22

BTW, it would be cool if...

Oops, I retract that, while I was testing what I was went to ask you to
implement, I saw that this last patch broke this use case:

[root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
Failed to get ELF symbols for e1000e
Probe point 'e1000_xmit_frame' not found.
  Error: Failed to add events.
[root@jouet ~]#

If I remove it, I get it back:

[root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
Added new event:
  probe:e1000_xmit_frame (on e1000_xmit_frame in e1000e)

You can now use it in all perf tools, such as:

	perf record -e probe:e1000_xmit_frame -aR sleep 1

[root@jouet ~]# perf probe -l
  probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@intel/e1000e/80003es2lan.c in e1000e)
[root@jouet ~]#

BTW, what I would find cool, besides fixing this new problem, would be that:

  perf probe e1000_xmit_frame

worked, as we can get the module name from kallsyms:

[acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
[acme@jouet linux]$ 

I applied the first two patches, and now I'm preparing a pull req to Ingo,

Thanks!

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-event.c  |   38 +++++++++++++++++++++++---------------
>  tools/perf/util/probe-finder.c |    2 +-
>  tools/perf/util/probe-finder.h |    2 ++
>  3 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4a57c8a..f75c99a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -682,15 +682,19 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
>  	return ret;
>  }
>  
> -static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> -					    int ntevs, const char *module)
> +static int
> +post_process_module_probe_trace_events(struct probe_trace_event *tevs,
> +				       int ntevs, const char *module,
> +				       struct debuginfo *dinfo)
>  {
> +	Dwarf_Addr text_offs;
>  	int i, ret = 0;
>  	char *mod_name = NULL;
>  
>  	if (!module)
>  		return 0;
>  
> +	debuginfo__get_text_offset(dinfo, &text_offs);
>  	mod_name = find_module_name(module);
>  
>  	for (i = 0; i < ntevs; i++) {
> @@ -700,9 +704,15 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>  			ret = -ENOMEM;
>  			break;
>  		}
> +		/* kernel module needs a special care to adjust addresses */
> +		tevs[i].point.address -= (unsigned long)text_offs;
>  	}
>  
>  	free(mod_name);
> +
> +	if (!ret)
> +		ret = post_process_offline_probe_trace_events(tevs, ntevs,
> +								module);
>  	return ret;
>  }
>  
> @@ -760,7 +770,7 @@ arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unuse
>  static int post_process_probe_trace_events(struct perf_probe_event *pev,
>  					   struct probe_trace_event *tevs,
>  					   int ntevs, const char *module,
> -					   bool uprobe)
> +					   bool uprobe, struct debuginfo *dinfo)
>  {
>  	int ret;
>  
> @@ -768,7 +778,8 @@ static int post_process_probe_trace_events(struct perf_probe_event *pev,
>  		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
>  	else if (module)
>  		/* Currently ref_reloc_sym based probe is not for drivers */
> -		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
> +		ret = post_process_module_probe_trace_events(tevs, ntevs,
> +							     module, dinfo);
>  	else
>  		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
>  
> @@ -812,30 +823,27 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>  		}
>  	}
>  
> -	debuginfo__delete(dinfo);
> -
>  	if (ntevs > 0) {	/* Succeeded to find trace events */
>  		pr_debug("Found %d probe_trace_events.\n", ntevs);
>  		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
> -						pev->target, pev->uprobes);
> +					pev->target, pev->uprobes, dinfo);
>  		if (ret < 0 || ret == ntevs) {
> +			pr_debug("Post processing failed or all events are skipped. (%d)\n", ret);
>  			clear_probe_trace_events(*tevs, ntevs);
>  			zfree(tevs);
> +			ntevs = 0;
>  		}
> -		if (ret != ntevs)
> -			return ret < 0 ? ret : ntevs;
> -		ntevs = 0;
> -		/* Fall through */
>  	}
>  
> +	debuginfo__delete(dinfo);
> +
>  	if (ntevs == 0)	{	/* No error but failed to find probe point. */
>  		pr_warning("Probe point '%s' not found.\n",
>  			   synthesize_perf_probe_point(&pev->point));
>  		return -ENOENT;
> -	}
> -	/* Error path : ntevs < 0 */
> -	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> -	if (ntevs < 0) {
> +	} else if (ntevs < 0) {
> +		/* Error path : ntevs < 0 */
> +		pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
>  		if (ntevs == -EBADF)
>  			pr_warning("Warning: No dwarf info found in the vmlinux - "
>  				"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index df4debe..6dede24 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1501,7 +1501,7 @@ int debuginfo__find_available_vars_at(struct debuginfo *dbg,
>  }
>  
>  /* For the kernel module, we need a special code to get a DIE */
> -static int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
>  {
>  	int n, i;
>  	Elf32_Word shndx;
> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> index f1d8558..1e03bbb 100644
> --- a/tools/perf/util/probe-finder.h
> +++ b/tools/perf/util/probe-finder.h
> @@ -46,6 +46,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
>  int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
>  				struct perf_probe_point *ppt);
>  
> +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs);
> +
>  /* Find a line range */
>  int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr);
>  

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:perf/urgent] perf probe: Fix --funcs to show correct symbols for offline module
  2017-01-04  3:29 ` [PATCH perf/core 1/3] perf-probe: Fix --funcs to show correct symbols for offline module Masami Hiramatsu
@ 2017-01-05  7:54   ` tip-bot for Masami Hiramatsu
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2017-01-05  7:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, namhyung, mingo, linux-kernel, jolsa, acme, mhiramat, tglx, peterz

Commit-ID:  eebc509b20881b92d62e317b2c073e57c5f200f0
Gitweb:     http://git.kernel.org/tip/eebc509b20881b92d62e317b2c073e57c5f200f0
Author:     Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate: Wed, 4 Jan 2017 12:29:05 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 4 Jan 2017 11:15:09 -0300

perf probe: Fix --funcs to show correct symbols for offline module

Fix --funcs (-F) option to show correct symbols for offline module.
Since previous perf-probe uses machine__findnew_module_map() for offline
module, even if user passes a module file (with full path) which is for
other architecture, perf-probe always tries to load symbol map for
current kernel module.

This fix uses dso__new_map() to load the map from given binary as same
as a map for user applications.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/148350053478.19001.15435255244512631545.stgit@devbox
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8f81096..542e647 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -163,7 +163,7 @@ static struct map *kernel_get_module_map(const char *module)
 
 	/* A file path -- this is an offline module */
 	if (module && strchr(module, '/'))
-		return machine__findnew_module_map(host_machine, 0, module);
+		return dso__new_map(module);
 
 	if (!module)
 		module = "kernel";
@@ -173,6 +173,7 @@ static struct map *kernel_get_module_map(const char *module)
 		if (strncmp(pos->dso->short_name + 1, module,
 			    pos->dso->short_name_len - 2) == 0 &&
 		    module[pos->dso->short_name_len - 2] == '\0') {
+			map__get(pos);
 			return pos;
 		}
 	}
@@ -188,15 +189,6 @@ struct map *get_target_map(const char *target, bool user)
 		return kernel_get_module_map(target);
 }
 
-static void put_target_map(struct map *map, bool user)
-{
-	if (map && user) {
-		/* Only the user map needs to be released */
-		map__put(map);
-	}
-}
-
-
 static int convert_exec_to_group(const char *exec, char **result)
 {
 	char *ptr1, *ptr2, *exec_copy;
@@ -412,7 +404,7 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
 	}
 
 out:
-	put_target_map(map, uprobes);
+	map__put(map);
 	return ret;
 
 }
@@ -2869,7 +2861,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	}
 
 out:
-	put_target_map(map, pev->uprobes);
+	map__put(map);
 	free(syms);
 	return ret;
 
@@ -3362,10 +3354,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
 		return ret;
 
 	/* Get a symbol map */
-	if (user)
-		map = dso__new_map(target);
-	else
-		map = kernel_get_module_map(target);
+	map = get_target_map(target, user);
 	if (!map) {
 		pr_err("Failed to get a map for %s\n", (target) ? : "kernel");
 		return -EINVAL;
@@ -3397,9 +3386,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
         }
 
 end:
-	if (user) {
-		map__put(map);
-	}
+	map__put(map);
 	exit_probe_symbol_maps();
 
 	return ret;

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/urgent] perf probe: Fix to probe on gcc generated symbols for offline kernel
  2017-01-04  3:30 ` [PATCH perf/core 2/3] perf-probe: Fix to probe on gcc generated symbols for offline kernel Masami Hiramatsu
@ 2017-01-05  7:54   ` tip-bot for Masami Hiramatsu
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2017-01-05  7:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, acme, linux-kernel, tglx, namhyung, hpa, mhiramat, mingo, jolsa

Commit-ID:  8a937a25a7e3c19d5fb3f9d92f605cf5fda219d8
Gitweb:     http://git.kernel.org/tip/8a937a25a7e3c19d5fb3f9d92f605cf5fda219d8
Author:     Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate: Wed, 4 Jan 2017 12:30:19 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 4 Jan 2017 11:44:22 -0300

perf probe: Fix to probe on gcc generated symbols for offline kernel

Fix perf-probe to show probe definition on gcc generated symbols for
offline kernel (including cross-arch kernel image).

gcc sometimes optimizes functions and generate new symbols with suffixes
such as ".constprop.N" or ".isra.N" etc. Since those symbol names are
not recorded in DWARF, we have to find correct generated symbols from
offline ELF binary to probe on it (kallsyms doesn't correct it).  For
online kernel or uprobes we don't need it because those are rebased on
_text, or a section relative address.

E.g. Without this:

  $ perf probe -k build-arm/vmlinux -F __slab_alloc*
  __slab_alloc.constprop.9
  $ perf probe -k build-arm/vmlinux -D __slab_alloc
  p:probe/__slab_alloc __slab_alloc+0

If you put above definition on target machine, it should fail
because there is no __slab_alloc in kallsyms.

With this fix, perf probe shows correct probe definition on
__slab_alloc.constprop.9:

  $ perf probe -k build-arm/vmlinux -D __slab_alloc
  p:probe/__slab_alloc __slab_alloc.constprop.9+0

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/148350060434.19001.11864836288580083501.stgit@devbox
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 48 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 542e647..4a57c8a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -610,6 +610,51 @@ error:
 	return ret ? : -ENOENT;
 }
 
+/*
+ * Rename DWARF symbols to ELF symbols -- gcc sometimes optimizes functions
+ * and generate new symbols with suffixes such as .constprop.N or .isra.N
+ * etc. Since those symbols are not recorded in DWARF, we have to find
+ * correct generated symbols from offline ELF binary.
+ * For online kernel or uprobes we don't need this because those are
+ * rebased on _text, or already a section relative address.
+ */
+static int
+post_process_offline_probe_trace_events(struct probe_trace_event *tevs,
+					int ntevs, const char *pathname)
+{
+	struct symbol *sym;
+	struct map *map;
+	unsigned long stext = 0;
+	u64 addr;
+	int i;
+
+	/* Prepare a map for offline binary */
+	map = dso__new_map(pathname);
+	if (!map || get_text_start_address(pathname, &stext) < 0) {
+		pr_warning("Failed to get ELF symbols for %s\n", pathname);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ntevs; i++) {
+		addr = tevs[i].point.address + tevs[i].point.offset - stext;
+		sym = map__find_symbol(map, addr);
+		if (!sym)
+			continue;
+		if (!strcmp(sym->name, tevs[i].point.symbol))
+			continue;
+		/* If we have no realname, use symbol for it */
+		if (!tevs[i].point.realname)
+			tevs[i].point.realname = tevs[i].point.symbol;
+		else
+			free(tevs[i].point.symbol);
+		tevs[i].point.symbol = strdup(sym->name);
+		tevs[i].point.offset = addr - sym->start;
+	}
+	map__put(map);
+
+	return 0;
+}
+
 static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
 					  int ntevs, const char *exec)
 {
@@ -671,7 +716,8 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 
 	/* Skip post process if the target is an offline kernel */
 	if (symbol_conf.ignore_vmlinux_buildid)
-		return 0;
+		return post_process_offline_probe_trace_events(tevs, ntevs,
+						symbol_conf.vmlinux_name);
 
 	reloc_sym = kernel_get_ref_reloc_sym();
 	if (!reloc_sym) {

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-04 14:48   ` Arnaldo Carvalho de Melo
@ 2017-01-05 11:20     ` Masami Hiramatsu
  2017-01-05 14:16       ` Masami Hiramatsu
  2017-01-05 15:30       ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-05 11:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

On Wed, 4 Jan 2017 11:48:56 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Jan 04, 2017 at 12:31:39PM +0900, Masami Hiramatsu escreveu:
> > Fix to probe on gcc generated functions on modules. Since
> > probing on a module is based on its symbol name, it should
> > be adjusted on actual symbols.
> > 
> > E.g. without this fix, perf probe shows probe definition
> > on non-exist symbol as below.
> >   -----
> >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -F in_range*
> >   in_range.isra.12
> >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> >   p:probe/in_range nf_nat:in_range+0
> >   -----
> > With this fix, perf probe correctly shows a probe on
> > gcc-generated symbol.
> >   -----
> >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> >   p:probe/in_range nf_nat:in_range.isra.12+0
> >   -----
> 
> Tested this one on a x86-64 fedora25 system, applied all three.
> 
> As always, please take a look at the patch when sent to Ingo, I usually
> put some committer notes there, in this case I put the test steps I
> performed, using e1000e.ko and e1000_flash_cycle_ich8lan.constprop.22
> 
> BTW, it would be cool if...
> 
> Oops, I retract that, while I was testing what I was went to ask you to
> implement, I saw that this last patch broke this use case:
> 
> [root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
> Failed to get ELF symbols for e1000e
> Probe point 'e1000_xmit_frame' not found.
>   Error: Failed to add events.
> [root@jouet ~]#

Oops, I missed that case.
However, when I trided to fix that on fedora25,

$ perf probe -m i915 -vD assert_plane
probe-definition(0): assert_plane
symbol:assert_plane file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
No kprobe blacklist support, ignored
Failed to get build-id from i915.
Cache open error: -1
Open Debuginfo file: /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
Try to find probe point from debuginfo.
Matched function: assert_plane [6fb806]
found inline addr: 0x886a0
Probe point found: assert_plane+0
Found 1 probe_trace_events.
text offset: 10030
Failed to get ELF symbols for /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
Post processing failed or all events are skipped. (-22)
Probe point 'assert_plane' not found.
  Error: Failed to add events. Reason: No such file or directory (Code: -2)

As far as I can see, elfutils's libdw supports compressed file, but
libelf API (perf's API) seems not supporting it. This results in
succeeding opening debuginfo, but failing ELF symbols.

> 
> If I remove it, I get it back:
> 
> [root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
> Added new event:
>   probe:e1000_xmit_frame (on e1000_xmit_frame in e1000e)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:e1000_xmit_frame -aR sleep 1
> 
> [root@jouet ~]# perf probe -l
>   probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@intel/e1000e/80003es2lan.c in e1000e)
> [root@jouet ~]#
> 
> BTW, what I would find cool, besides fixing this new problem, would be that:
> 
>   perf probe e1000_xmit_frame
> 
> worked, as we can get the module name from kallsyms:
> 
> [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> [acme@jouet linux]$ 

OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
include module symbols too?


> 
> I applied the first two patches, and now I'm preparing a pull req to Ingo,

Thanks!

> 
> Thanks!
> 
> - Arnaldo
>  
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c  |   38 +++++++++++++++++++++++---------------
> >  tools/perf/util/probe-finder.c |    2 +-
> >  tools/perf/util/probe-finder.h |    2 ++
> >  3 files changed, 26 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 4a57c8a..f75c99a 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -682,15 +682,19 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
> >  	return ret;
> >  }
> >  
> > -static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> > -					    int ntevs, const char *module)
> > +static int
> > +post_process_module_probe_trace_events(struct probe_trace_event *tevs,
> > +				       int ntevs, const char *module,
> > +				       struct debuginfo *dinfo)
> >  {
> > +	Dwarf_Addr text_offs;
> >  	int i, ret = 0;
> >  	char *mod_name = NULL;
> >  
> >  	if (!module)
> >  		return 0;
> >  
> > +	debuginfo__get_text_offset(dinfo, &text_offs);
> >  	mod_name = find_module_name(module);
> >  
> >  	for (i = 0; i < ntevs; i++) {
> > @@ -700,9 +704,15 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> >  			ret = -ENOMEM;
> >  			break;
> >  		}
> > +		/* kernel module needs a special care to adjust addresses */
> > +		tevs[i].point.address -= (unsigned long)text_offs;
> >  	}
> >  
> >  	free(mod_name);
> > +
> > +	if (!ret)
> > +		ret = post_process_offline_probe_trace_events(tevs, ntevs,
> > +								module);
> >  	return ret;
> >  }
> >  
> > @@ -760,7 +770,7 @@ arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unuse
> >  static int post_process_probe_trace_events(struct perf_probe_event *pev,
> >  					   struct probe_trace_event *tevs,
> >  					   int ntevs, const char *module,
> > -					   bool uprobe)
> > +					   bool uprobe, struct debuginfo *dinfo)
> >  {
> >  	int ret;
> >  
> > @@ -768,7 +778,8 @@ static int post_process_probe_trace_events(struct perf_probe_event *pev,
> >  		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
> >  	else if (module)
> >  		/* Currently ref_reloc_sym based probe is not for drivers */
> > -		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
> > +		ret = post_process_module_probe_trace_events(tevs, ntevs,
> > +							     module, dinfo);
> >  	else
> >  		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
> >  
> > @@ -812,30 +823,27 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> >  		}
> >  	}
> >  
> > -	debuginfo__delete(dinfo);
> > -
> >  	if (ntevs > 0) {	/* Succeeded to find trace events */
> >  		pr_debug("Found %d probe_trace_events.\n", ntevs);
> >  		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
> > -						pev->target, pev->uprobes);
> > +					pev->target, pev->uprobes, dinfo);
> >  		if (ret < 0 || ret == ntevs) {
> > +			pr_debug("Post processing failed or all events are skipped. (%d)\n", ret);
> >  			clear_probe_trace_events(*tevs, ntevs);
> >  			zfree(tevs);
> > +			ntevs = 0;
> >  		}
> > -		if (ret != ntevs)
> > -			return ret < 0 ? ret : ntevs;
> > -		ntevs = 0;
> > -		/* Fall through */
> >  	}
> >  
> > +	debuginfo__delete(dinfo);
> > +
> >  	if (ntevs == 0)	{	/* No error but failed to find probe point. */
> >  		pr_warning("Probe point '%s' not found.\n",
> >  			   synthesize_perf_probe_point(&pev->point));
> >  		return -ENOENT;
> > -	}
> > -	/* Error path : ntevs < 0 */
> > -	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> > -	if (ntevs < 0) {
> > +	} else if (ntevs < 0) {
> > +		/* Error path : ntevs < 0 */
> > +		pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> >  		if (ntevs == -EBADF)
> >  			pr_warning("Warning: No dwarf info found in the vmlinux - "
> >  				"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index df4debe..6dede24 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -1501,7 +1501,7 @@ int debuginfo__find_available_vars_at(struct debuginfo *dbg,
> >  }
> >  
> >  /* For the kernel module, we need a special code to get a DIE */
> > -static int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> > +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> >  {
> >  	int n, i;
> >  	Elf32_Word shndx;
> > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> > index f1d8558..1e03bbb 100644
> > --- a/tools/perf/util/probe-finder.h
> > +++ b/tools/perf/util/probe-finder.h
> > @@ -46,6 +46,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
> >  int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
> >  				struct perf_probe_point *ppt);
> >  
> > +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs);
> > +
> >  /* Find a line range */
> >  int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr);
> >  


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-05 11:20     ` Masami Hiramatsu
@ 2017-01-05 14:16       ` Masami Hiramatsu
  2017-01-05 15:30       ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-05 14:16 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa,
	Peter Zijlstra, Ingo Molnar, Namhyung Kim

On Thu, 5 Jan 2017 20:20:19 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Wed, 4 Jan 2017 11:48:56 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Wed, Jan 04, 2017 at 12:31:39PM +0900, Masami Hiramatsu escreveu:
> > > Fix to probe on gcc generated functions on modules. Since
> > > probing on a module is based on its symbol name, it should
> > > be adjusted on actual symbols.
> > > 
> > > E.g. without this fix, perf probe shows probe definition
> > > on non-exist symbol as below.
> > >   -----
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -F in_range*
> > >   in_range.isra.12
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> > >   p:probe/in_range nf_nat:in_range+0
> > >   -----
> > > With this fix, perf probe correctly shows a probe on
> > > gcc-generated symbol.
> > >   -----
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> > >   p:probe/in_range nf_nat:in_range.isra.12+0
> > >   -----
> > 
> > Tested this one on a x86-64 fedora25 system, applied all three.
> > 
> > As always, please take a look at the patch when sent to Ingo, I usually
> > put some committer notes there, in this case I put the test steps I
> > performed, using e1000e.ko and e1000_flash_cycle_ich8lan.constprop.22
> > 
> > BTW, it would be cool if...
> > 
> > Oops, I retract that, while I was testing what I was went to ask you to
> > implement, I saw that this last patch broke this use case:
> > 
> > [root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
> > Failed to get ELF symbols for e1000e
> > Probe point 'e1000_xmit_frame' not found.
> >   Error: Failed to add events.
> > [root@jouet ~]#
> 
> Oops, I missed that case.
> However, when I trided to fix that on fedora25,
> 
> $ perf probe -m i915 -vD assert_plane
> probe-definition(0): assert_plane
> symbol:assert_plane file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> No kprobe blacklist support, ignored
> Failed to get build-id from i915.
> Cache open error: -1
> Open Debuginfo file: /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
> Try to find probe point from debuginfo.
> Matched function: assert_plane [6fb806]
> found inline addr: 0x886a0
> Probe point found: assert_plane+0
> Found 1 probe_trace_events.
> text offset: 10030
> Failed to get ELF symbols for /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
> Post processing failed or all events are skipped. (-22)
> Probe point 'assert_plane' not found.
>   Error: Failed to add events. Reason: No such file or directory (Code: -2)
> 
> As far as I can see, elfutils's libdw supports compressed file, but
> libelf API (perf's API) seems not supporting it. This results in
> succeeding opening debuginfo, but failing ELF symbols.

Sorry, this is my fault, perf's map already supported that.
Anyway, I can get ELF headers from debuginfo. However, I'm still
trying to fix an text section offset issue in modules.

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-05 11:20     ` Masami Hiramatsu
  2017-01-05 14:16       ` Masami Hiramatsu
@ 2017-01-05 15:30       ` Arnaldo Carvalho de Melo
  2017-01-05 18:47         ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-01-05 15:30 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> On Wed, 4 Jan 2017 11:48:56 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Wed, Jan 04, 2017 at 12:31:39PM +0900, Masami Hiramatsu escreveu:
> > > Fix to probe on gcc generated functions on modules. Since
> > > probing on a module is based on its symbol name, it should
> > > be adjusted on actual symbols.
> > > 
> > > E.g. without this fix, perf probe shows probe definition
> > > on non-exist symbol as below.
> > >   -----
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -F in_range*
> > >   in_range.isra.12
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> > >   p:probe/in_range nf_nat:in_range+0
> > >   -----
> > > With this fix, perf probe correctly shows a probe on
> > > gcc-generated symbol.
> > >   -----
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> > >   p:probe/in_range nf_nat:in_range.isra.12+0
> > >   -----
> > 
> > Tested this one on a x86-64 fedora25 system, applied all three.
> > 
> > As always, please take a look at the patch when sent to Ingo, I usually
> > put some committer notes there, in this case I put the test steps I
> > performed, using e1000e.ko and e1000_flash_cycle_ich8lan.constprop.22
> > 
> > BTW, it would be cool if...
> > 
> > Oops, I retract that, while I was testing what I was went to ask you to
> > implement, I saw that this last patch broke this use case:
> > 
> > [root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
> > Failed to get ELF symbols for e1000e
> > Probe point 'e1000_xmit_frame' not found.
> >   Error: Failed to add events.
> > [root@jouet ~]#
> 
> Oops, I missed that case.
> However, when I trided to fix that on fedora25,
> 
> $ perf probe -m i915 -vD assert_plane
> probe-definition(0): assert_plane
> symbol:assert_plane file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> No kprobe blacklist support, ignored
> Failed to get build-id from i915.
> Cache open error: -1
> Open Debuginfo file: /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
> Try to find probe point from debuginfo.
> Matched function: assert_plane [6fb806]
> found inline addr: 0x886a0
> Probe point found: assert_plane+0
> Found 1 probe_trace_events.
> text offset: 10030
> Failed to get ELF symbols for /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
> Post processing failed or all events are skipped. (-22)
> Probe point 'assert_plane' not found.
>   Error: Failed to add events. Reason: No such file or directory (Code: -2)
> 
> As far as I can see, elfutils's libdw supports compressed file, but
> libelf API (perf's API) seems not supporting it. This results in
> succeeding opening debuginfo, but failing ELF symbols.
> 
> > 
> > If I remove it, I get it back:
> > 
> > [root@jouet ~]# perf probe -m e1000e e1000_xmit_frame
> > Added new event:
> >   probe:e1000_xmit_frame (on e1000_xmit_frame in e1000e)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe:e1000_xmit_frame -aR sleep 1
> > 
> > [root@jouet ~]# perf probe -l
> >   probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@intel/e1000e/80003es2lan.c in e1000e)
> > [root@jouet ~]#
> > 
> > BTW, what I would find cool, besides fixing this new problem, would be that:
> > 
> >   perf probe e1000_xmit_frame
> > 
> > worked, as we can get the module name from kallsyms:
> > 
> > [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > [acme@jouet linux]$ 
> 
> OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> include module symbols too?

Well, the machine abstraction is there for that, i.e. to get all the
abstractions related to a machine (kernel, kernel modules, threads,
maps, symbols).

We have 'struct machines' as well, to deal with VMs, etc. This can come
from the running system or from a perf.data file, when it may be for a
different architecture than the machine where analysis is done.

So, "map for kallsyms" in fact should be "maps for kallsyms", as it will
split the maps for each module, etc.

Probably what you want is:

 symbol = machine__find_kernel_function_by_name(machine, name, &map);

Then, if symbol is not NULL, map->dso->name will have your module name,
map->dso->long_name will have the .ko path, etc.

- Arnaldo

> 
> > 
> > I applied the first two patches, and now I'm preparing a pull req to Ingo,
> 
> Thanks!
> 
> > 
> > Thanks!
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > >  tools/perf/util/probe-event.c  |   38 +++++++++++++++++++++++---------------
> > >  tools/perf/util/probe-finder.c |    2 +-
> > >  tools/perf/util/probe-finder.h |    2 ++
> > >  3 files changed, 26 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 4a57c8a..f75c99a 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -682,15 +682,19 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
> > >  	return ret;
> > >  }
> > >  
> > > -static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> > > -					    int ntevs, const char *module)
> > > +static int
> > > +post_process_module_probe_trace_events(struct probe_trace_event *tevs,
> > > +				       int ntevs, const char *module,
> > > +				       struct debuginfo *dinfo)
> > >  {
> > > +	Dwarf_Addr text_offs;
> > >  	int i, ret = 0;
> > >  	char *mod_name = NULL;
> > >  
> > >  	if (!module)
> > >  		return 0;
> > >  
> > > +	debuginfo__get_text_offset(dinfo, &text_offs);
> > >  	mod_name = find_module_name(module);
> > >  
> > >  	for (i = 0; i < ntevs; i++) {
> > > @@ -700,9 +704,15 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> > >  			ret = -ENOMEM;
> > >  			break;
> > >  		}
> > > +		/* kernel module needs a special care to adjust addresses */
> > > +		tevs[i].point.address -= (unsigned long)text_offs;
> > >  	}
> > >  
> > >  	free(mod_name);
> > > +
> > > +	if (!ret)
> > > +		ret = post_process_offline_probe_trace_events(tevs, ntevs,
> > > +								module);
> > >  	return ret;
> > >  }
> > >  
> > > @@ -760,7 +770,7 @@ arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unuse
> > >  static int post_process_probe_trace_events(struct perf_probe_event *pev,
> > >  					   struct probe_trace_event *tevs,
> > >  					   int ntevs, const char *module,
> > > -					   bool uprobe)
> > > +					   bool uprobe, struct debuginfo *dinfo)
> > >  {
> > >  	int ret;
> > >  
> > > @@ -768,7 +778,8 @@ static int post_process_probe_trace_events(struct perf_probe_event *pev,
> > >  		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
> > >  	else if (module)
> > >  		/* Currently ref_reloc_sym based probe is not for drivers */
> > > -		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
> > > +		ret = post_process_module_probe_trace_events(tevs, ntevs,
> > > +							     module, dinfo);
> > >  	else
> > >  		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
> > >  
> > > @@ -812,30 +823,27 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> > >  		}
> > >  	}
> > >  
> > > -	debuginfo__delete(dinfo);
> > > -
> > >  	if (ntevs > 0) {	/* Succeeded to find trace events */
> > >  		pr_debug("Found %d probe_trace_events.\n", ntevs);
> > >  		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
> > > -						pev->target, pev->uprobes);
> > > +					pev->target, pev->uprobes, dinfo);
> > >  		if (ret < 0 || ret == ntevs) {
> > > +			pr_debug("Post processing failed or all events are skipped. (%d)\n", ret);
> > >  			clear_probe_trace_events(*tevs, ntevs);
> > >  			zfree(tevs);
> > > +			ntevs = 0;
> > >  		}
> > > -		if (ret != ntevs)
> > > -			return ret < 0 ? ret : ntevs;
> > > -		ntevs = 0;
> > > -		/* Fall through */
> > >  	}
> > >  
> > > +	debuginfo__delete(dinfo);
> > > +
> > >  	if (ntevs == 0)	{	/* No error but failed to find probe point. */
> > >  		pr_warning("Probe point '%s' not found.\n",
> > >  			   synthesize_perf_probe_point(&pev->point));
> > >  		return -ENOENT;
> > > -	}
> > > -	/* Error path : ntevs < 0 */
> > > -	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> > > -	if (ntevs < 0) {
> > > +	} else if (ntevs < 0) {
> > > +		/* Error path : ntevs < 0 */
> > > +		pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> > >  		if (ntevs == -EBADF)
> > >  			pr_warning("Warning: No dwarf info found in the vmlinux - "
> > >  				"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
> > > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > > index df4debe..6dede24 100644
> > > --- a/tools/perf/util/probe-finder.c
> > > +++ b/tools/perf/util/probe-finder.c
> > > @@ -1501,7 +1501,7 @@ int debuginfo__find_available_vars_at(struct debuginfo *dbg,
> > >  }
> > >  
> > >  /* For the kernel module, we need a special code to get a DIE */
> > > -static int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> > > +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> > >  {
> > >  	int n, i;
> > >  	Elf32_Word shndx;
> > > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> > > index f1d8558..1e03bbb 100644
> > > --- a/tools/perf/util/probe-finder.h
> > > +++ b/tools/perf/util/probe-finder.h
> > > @@ -46,6 +46,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
> > >  int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
> > >  				struct perf_probe_point *ppt);
> > >  
> > > +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs);
> > > +
> > >  /* Find a line range */
> > >  int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr);
> > >  
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-05 15:30       ` Arnaldo Carvalho de Melo
@ 2017-01-05 18:47         ` Arnaldo Carvalho de Melo
  2017-01-06  2:16           ` Masami Hiramatsu
  2017-01-06 16:22           ` Masami Hiramatsu
  0 siblings, 2 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-01-05 18:47 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

Em Thu, Jan 05, 2017 at 12:30:37PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> > On Wed, 4 Jan 2017 11:48:56 -0300
> > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > > [acme@jouet linux]$ 

> > OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> > May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> > include module symbols too?

> Probably what you want is:
 
>  symbol = machine__find_kernel_function_by_name(machine, name, &map);
 
> Then, if symbol is not NULL, map->dso->name will have your module name,
> map->dso->long_name will have the .ko path, etc.

Take a look at 'perf kallsyms' a simple toy cmd that exercises the APIs
you need:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=25c92235676d2655e3f31ffe02bd4355d838e5e9

Basically:

  struct machine *machine = machine__new_kallsyms();
  struct map *map;
  struct symbol *symbol = machine__find_kernel_function_by_name(machine, symbol_name, &map);

  Then you have symbol->{addr,end,name,etc} and map->dso->{long,short}_name

  $ perf kallsyms e1000_xmit_frame usb_stor_set_xfer_buf
  e1000_xmit_frame: [e1000e] /lib/modules/4.9.0+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko 0xffffffffc046fc10-0xffffffffc0470bb0 (0x19c80-0x1ac20)
  usb_stor_set_xfer_buf: [usb_storage] /lib/modules/4.9.0+/kernel/drivers/usb/storage/usb-storage.ko 0xffffffffc057aea0-0xffffffffc057af19 (0xf10-0xf89)

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-05 18:47         ` Arnaldo Carvalho de Melo
@ 2017-01-06  2:16           ` Masami Hiramatsu
  2017-01-06 16:22           ` Masami Hiramatsu
  1 sibling, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-06  2:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

On Thu, 5 Jan 2017 15:47:43 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Jan 05, 2017 at 12:30:37PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> > > On Wed, 4 Jan 2017 11:48:56 -0300
> > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > > > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > > > [acme@jouet linux]$ 
> 
> > > OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> > > May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> > > include module symbols too?
> 
> > Probably what you want is:
>  
> >  symbol = machine__find_kernel_function_by_name(machine, name, &map);
>  
> > Then, if symbol is not NULL, map->dso->name will have your module name,
> > map->dso->long_name will have the .ko path, etc.
> 
> Take a look at 'perf kallsyms' a simple toy cmd that exercises the APIs
> you need:
> 
> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=25c92235676d2655e3f31ffe02bd4355d838e5e9
> 
> Basically:
> 
>   struct machine *machine = machine__new_kallsyms();
>   struct map *map;
>   struct symbol *symbol = machine__find_kernel_function_by_name(machine, symbol_name, &map);
> 
>   Then you have symbol->{addr,end,name,etc} and map->dso->{long,short}_name

Thanks for the precise information :)
I'll make and send it with updated 3rd patch.

Thanks again!

> 
>   $ perf kallsyms e1000_xmit_frame usb_stor_set_xfer_buf
>   e1000_xmit_frame: [e1000e] /lib/modules/4.9.0+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko 0xffffffffc046fc10-0xffffffffc0470bb0 (0x19c80-0x1ac20)
>   usb_stor_set_xfer_buf: [usb_storage] /lib/modules/4.9.0+/kernel/drivers/usb/storage/usb-storage.ko 0xffffffffc057aea0-0xffffffffc057af19 (0xf10-0xf89)
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-05 18:47         ` Arnaldo Carvalho de Melo
  2017-01-06  2:16           ` Masami Hiramatsu
@ 2017-01-06 16:22           ` Masami Hiramatsu
  2017-01-06 18:17             ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-06 16:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

Hi Arnaldo,

On Thu, 5 Jan 2017 15:47:43 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Jan 05, 2017 at 12:30:37PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> > > On Wed, 4 Jan 2017 11:48:56 -0300
> > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > > > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > > > [acme@jouet linux]$ 
> 
> > > OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> > > May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> > > include module symbols too?
> 
> > Probably what you want is:
>  
> >  symbol = machine__find_kernel_function_by_name(machine, name, &map);
>  
> > Then, if symbol is not NULL, map->dso->name will have your module name,
> > map->dso->long_name will have the .ko path, etc.
> 
> Take a look at 'perf kallsyms' a simple toy cmd that exercises the APIs
> you need:
> 
> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=25c92235676d2655e3f31ffe02bd4355d838e5e9
> 
> Basically:
> 
>   struct machine *machine = machine__new_kallsyms();
>   struct map *map;
>   struct symbol *symbol = machine__find_kernel_function_by_name(machine, symbol_name, &map);
> 
>   Then you have symbol->{addr,end,name,etc} and map->dso->{long,short}_name
> 
>   $ perf kallsyms e1000_xmit_frame usb_stor_set_xfer_buf
>   e1000_xmit_frame: [e1000e] /lib/modules/4.9.0+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko 0xffffffffc046fc10-0xffffffffc0470bb0 (0x19c80-0x1ac20)
>   usb_stor_set_xfer_buf: [usb_storage] /lib/modules/4.9.0+/kernel/drivers/usb/storage/usb-storage.ko 0xffffffffc057aea0-0xffffffffc057af19 (0xf10-0xf89)

I've found an issue on perf kallsyms. Please see below.

When I run kallsyms without root privilege, it works well.

$ ./perf kallsyms -vv drm_malloc_ab
Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO capability to access.
drm_malloc_ab: [i915] /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz 0xffffffffc0211950-0xffffffffc02119a6 (0x309c0-0x30a16)

However, with root privilege it failed to get correct module name...

$ sudo ./perf kallsyms -vv drm_malloc_ab
Using /proc/kcore for kernel object code
drm_malloc_ab: [kernel] /proc/kcore 0xffffffffc0211950-0xffffffffc02119b0 (0x7fffc0214950-0x7fffc02149b0)

Thank you,


> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-06 16:22           ` Masami Hiramatsu
@ 2017-01-06 18:17             ` Arnaldo Carvalho de Melo
  2017-01-07  1:50               ` Masami Hiramatsu
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-01-06 18:17 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

Em Sat, Jan 07, 2017 at 01:22:23AM +0900, Masami Hiramatsu escreveu:
> On Thu, 5 Jan 2017 15:47:43 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Thu, Jan 05, 2017 at 12:30:37PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> > > > On Wed, 4 Jan 2017 11:48:56 -0300
> > > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > > [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > > > > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > > > > [acme@jouet linux]$ 
> > 
> > > > OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> > > > May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> > > > include module symbols too?
> > 
> > > Probably what you want is:
> >  
> > >  symbol = machine__find_kernel_function_by_name(machine, name, &map);
> >  
> > > Then, if symbol is not NULL, map->dso->name will have your module name,
> > > map->dso->long_name will have the .ko path, etc.
> > 
> > Take a look at 'perf kallsyms' a simple toy cmd that exercises the APIs
> > you need:
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=25c92235676d2655e3f31ffe02bd4355d838e5e9
> > 
> > Basically:
> > 
> >   struct machine *machine = machine__new_kallsyms();
> >   struct map *map;
> >   struct symbol *symbol = machine__find_kernel_function_by_name(machine, symbol_name, &map);
> > 
> >   Then you have symbol->{addr,end,name,etc} and map->dso->{long,short}_name
> > 
> >   $ perf kallsyms e1000_xmit_frame usb_stor_set_xfer_buf
> >   e1000_xmit_frame: [e1000e] /lib/modules/4.9.0+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko 0xffffffffc046fc10-0xffffffffc0470bb0 (0x19c80-0x1ac20)
> >   usb_stor_set_xfer_buf: [usb_storage] /lib/modules/4.9.0+/kernel/drivers/usb/storage/usb-storage.ko 0xffffffffc057aea0-0xffffffffc057af19 (0xf10-0xf89)
> 
> I've found an issue on perf kallsyms. Please see below.
> 
> When I run kallsyms without root privilege, it works well.
> 
> $ ./perf kallsyms -vv drm_malloc_ab
> Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO capability to access.
> drm_malloc_ab: [i915] /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz 0xffffffffc0211950-0xffffffffc02119a6 (0x309c0-0x30a16)
> 
> However, with root privilege it failed to get correct module name...
> 
> $ sudo ./perf kallsyms -vv drm_malloc_ab
> Using /proc/kcore for kernel object code
> drm_malloc_ab: [kernel] /proc/kcore 0xffffffffc0211950-0xffffffffc02119b0 (0x7fffc0214950-0x7fffc02149b0)

Right, that is something that needs fixing, please try with the patch
below.

The problem is that when using /proc/kcore it messes up the creation of
one map per module :-\

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4aef30ec8bc3..43d533b075df 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -91,7 +91,7 @@ struct machine *machine__new_kallsyms(void)
 {
 	struct machine *machine = machine__new_host();
 
-	if (machine && machine__load_kallsyms(machine, "/proc/kallsyms", MAP__FUNCTION) <= 0) {
+	if (machine && __machine__load_kallsyms(machine, "/proc/kallsyms", MAP__FUNCTION, true) <= 0) {
 		machine__delete(machine);
 		machine = NULL;
 	}

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules
  2017-01-06 18:17             ` Arnaldo Carvalho de Melo
@ 2017-01-07  1:50               ` Masami Hiramatsu
  0 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2017-01-07  1:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim

On Fri, 6 Jan 2017 15:17:11 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Sat, Jan 07, 2017 at 01:22:23AM +0900, Masami Hiramatsu escreveu:
> > On Thu, 5 Jan 2017 15:47:43 -0300
> > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > Em Thu, Jan 05, 2017 at 12:30:37PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> > > > > On Wed, 4 Jan 2017 11:48:56 -0300
> > > > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > > > [acme@jouet linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > > > > > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > > > > > [acme@jouet linux]$ 
> > > 
> > > > > OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> > > > > May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> > > > > include module symbols too?
> > > 
> > > > Probably what you want is:
> > >  
> > > >  symbol = machine__find_kernel_function_by_name(machine, name, &map);
> > >  
> > > > Then, if symbol is not NULL, map->dso->name will have your module name,
> > > > map->dso->long_name will have the .ko path, etc.
> > > 
> > > Take a look at 'perf kallsyms' a simple toy cmd that exercises the APIs
> > > you need:
> > > 
> > > https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=25c92235676d2655e3f31ffe02bd4355d838e5e9
> > > 
> > > Basically:
> > > 
> > >   struct machine *machine = machine__new_kallsyms();
> > >   struct map *map;
> > >   struct symbol *symbol = machine__find_kernel_function_by_name(machine, symbol_name, &map);
> > > 
> > >   Then you have symbol->{addr,end,name,etc} and map->dso->{long,short}_name
> > > 
> > >   $ perf kallsyms e1000_xmit_frame usb_stor_set_xfer_buf
> > >   e1000_xmit_frame: [e1000e] /lib/modules/4.9.0+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko 0xffffffffc046fc10-0xffffffffc0470bb0 (0x19c80-0x1ac20)
> > >   usb_stor_set_xfer_buf: [usb_storage] /lib/modules/4.9.0+/kernel/drivers/usb/storage/usb-storage.ko 0xffffffffc057aea0-0xffffffffc057af19 (0xf10-0xf89)
> > 
> > I've found an issue on perf kallsyms. Please see below.
> > 
> > When I run kallsyms without root privilege, it works well.
> > 
> > $ ./perf kallsyms -vv drm_malloc_ab
> > Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO capability to access.
> > drm_malloc_ab: [i915] /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz 0xffffffffc0211950-0xffffffffc02119a6 (0x309c0-0x30a16)
> > 
> > However, with root privilege it failed to get correct module name...
> > 
> > $ sudo ./perf kallsyms -vv drm_malloc_ab
> > Using /proc/kcore for kernel object code
> > drm_malloc_ab: [kernel] /proc/kcore 0xffffffffc0211950-0xffffffffc02119b0 (0x7fffc0214950-0x7fffc02149b0)
> 
> Right, that is something that needs fixing, please try with the patch
> below.
> 
> The problem is that when using /proc/kcore it messes up the creation of
> one map per module :-\

OK, it works now :)

$ sudo ./perf kallsyms drm_malloc_ab
drm_malloc_ab: [i915] /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz 0xffffffffc0188950-0xffffffffc01889a6 (0x309c0-0x30a16)

Tested-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks! 

> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 4aef30ec8bc3..43d533b075df 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -91,7 +91,7 @@ struct machine *machine__new_kallsyms(void)
>  {
>  	struct machine *machine = machine__new_host();
>  
> -	if (machine && machine__load_kallsyms(machine, "/proc/kallsyms", MAP__FUNCTION) <= 0) {
> +	if (machine && __machine__load_kallsyms(machine, "/proc/kallsyms", MAP__FUNCTION, true) <= 0) {
>  		machine__delete(machine);
>  		machine = NULL;
>  	}


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-01-07  1:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-04  3:27 [PATCH perf/core 0/3] perf-probe: Fix offline module and cross-arch support Masami Hiramatsu
2017-01-04  3:29 ` [PATCH perf/core 1/3] perf-probe: Fix --funcs to show correct symbols for offline module Masami Hiramatsu
2017-01-05  7:54   ` [tip:perf/urgent] perf probe: " tip-bot for Masami Hiramatsu
2017-01-04  3:30 ` [PATCH perf/core 2/3] perf-probe: Fix to probe on gcc generated symbols for offline kernel Masami Hiramatsu
2017-01-05  7:54   ` [tip:perf/urgent] perf probe: " tip-bot for Masami Hiramatsu
2017-01-04  3:31 ` [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated functions in modules Masami Hiramatsu
2017-01-04 14:48   ` Arnaldo Carvalho de Melo
2017-01-05 11:20     ` Masami Hiramatsu
2017-01-05 14:16       ` Masami Hiramatsu
2017-01-05 15:30       ` Arnaldo Carvalho de Melo
2017-01-05 18:47         ` Arnaldo Carvalho de Melo
2017-01-06  2:16           ` Masami Hiramatsu
2017-01-06 16:22           ` Masami Hiramatsu
2017-01-06 18:17             ` Arnaldo Carvalho de Melo
2017-01-07  1:50               ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).