linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] perf: Add function to post process kernel trace events
@ 2016-08-09  6:23 Ravi Bangoria
  2016-08-09  6:23 ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Ravi Bangoria
  2016-08-09 19:20 ` [tip:perf/urgent] perf probe: Add function to post process kernel trace events tip-bot for Ravi Bangoria
  0 siblings, 2 replies; 16+ messages in thread
From: Ravi Bangoria @ 2016-08-09  6:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: peterz, mingo, acme, alexander.shishkin, bsingharora,
	naveen.n.rao, ananth, ravi.bangoria, mhiramat, wangnan0,
	namhyung

Instead of inline code, introduce function to post process kernel
probe trace events.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/probe-event.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 953dc1a..4e215e7 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -664,22 +664,14 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
 	return ret;
 }
 
-/* Post processing the probe events */
-static int post_process_probe_trace_events(struct probe_trace_event *tevs,
-					   int ntevs, const char *module,
-					   bool uprobe)
+static int
+post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
+				       int ntevs)
 {
 	struct ref_reloc_sym *reloc_sym;
 	char *tmp;
 	int i, skipped = 0;
 
-	if (uprobe)
-		return add_exec_to_probe_trace_events(tevs, ntevs, module);
-
-	/* Note that currently ref_reloc_sym based probe is not for drivers */
-	if (module)
-		return add_module_to_probe_trace_events(tevs, ntevs, module);
-
 	reloc_sym = kernel_get_ref_reloc_sym();
 	if (!reloc_sym) {
 		pr_warning("Relocated base symbol is not found!\n");
@@ -711,6 +703,21 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 	return skipped;
 }
 
+/* Post processing the probe events */
+static int post_process_probe_trace_events(struct probe_trace_event *tevs,
+					   int ntevs, const char *module,
+					   bool uprobe)
+{
+	if (uprobe)
+		return add_exec_to_probe_trace_events(tevs, ntevs, module);
+
+	if (module)
+		/* Currently ref_reloc_sym based probe is not for drivers */
+		return add_module_to_probe_trace_events(tevs, ntevs, module);
+
+	return post_process_kernel_probe_trace_events(tevs, ntevs);
+}
+
 /* Try to find perf_probe_event with debuginfo */
 static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 					  struct probe_trace_event **tevs)
-- 
2.7.4

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

* [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-09  6:23 [PATCH 1/2] perf: Add function to post process kernel trace events Ravi Bangoria
@ 2016-08-09  6:23 ` Ravi Bangoria
  2016-08-09 14:02   ` Masami Hiramatsu
                     ` (2 more replies)
  2016-08-09 19:20 ` [tip:perf/urgent] perf probe: Add function to post process kernel trace events tip-bot for Ravi Bangoria
  1 sibling, 3 replies; 16+ messages in thread
From: Ravi Bangoria @ 2016-08-09  6:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: peterz, mingo, acme, alexander.shishkin, bsingharora,
	naveen.n.rao, ananth, ravi.bangoria, mhiramat, wangnan0,
	namhyung

Powerpc has Global Entry Point and Local Entry Point for functions.
LEP catches call from both the GEP and the LEP. Symbol table of ELF
contains GEP and Offset from which we can calculate LEP, but debuginfo
does not have LEP info.

Currently, perf prioritize symbol table over dwarf to probe on LEP
for ppc64le. But when user tries to probe with function parameter,
we fall back to using dwarf(i.e. GEP) and when function called via
LEP, probe will never hit.

For example:
  $ objdump -d vmlinux
    ...
    do_sys_open():
    c0000000002eb4a0:       e8 00 4c 3c     addis   r2,r12,232
    c0000000002eb4a4:       60 00 42 38     addi    r2,r2,96
    c0000000002eb4a8:       a6 02 08 7c     mflr    r0
    c0000000002eb4ac:       d0 ff 41 fb     std     r26,-48(r1)

  $ sudo ./perf probe do_sys_open
  $ sudo cat /sys/kernel/debug/tracing/kprobe_events
    p:probe/do_sys_open _text+3060904

  $ sudo ./perf probe 'do_sys_open filename:string'
  $ sudo cat /sys/kernel/debug/tracing/kprobe_events
    p:probe/do_sys_open _text+3060896 filename_string=+0(%gpr4):string

For second case, perf probed on GEP. So when function will be called
via LEP, probe won't hit.

  $ sudo ./perf record -a -e probe:do_sys_open ls
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.195 MB perf.data ]

To resolve this issue, let's not prioritize symbol table, let perf
decide what it wants to use. Perf is already converting GEP to LEP
when it uses symbol table. When perf uses debuginfo, let it find
LEP offset form symbol table. This way we fall back to probe on LEP
for all cases.

After patch:
  $ sudo ./perf probe 'do_sys_open filename:string'
  $ sudo cat /sys/kernel/debug/tracing/kprobe_events
    p:probe/do_sys_open _text+3060904 filename_string=+0(%gpr4):string

  $ sudo ./perf record -a -e probe:do_sys_open ls
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.197 MB perf.data (11 samples) ]

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 27 +++++++++++++++++----
 tools/perf/util/probe-event.c               | 37 ++++++++++++++++-------------
 tools/perf/util/probe-event.h               |  6 ++++-
 3 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index c6d0f91..8d4dc97 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -54,10 +54,6 @@ int arch__compare_symbol_names(const char *namea, const char *nameb)
 #endif
 
 #if defined(_CALL_ELF) && _CALL_ELF == 2
-bool arch__prefers_symtab(void)
-{
-	return true;
-}
 
 #ifdef HAVE_LIBELF_SUPPORT
 void arch__sym_update(struct symbol *s, GElf_Sym *sym)
@@ -100,4 +96,27 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 			tev->point.offset += lep_offset;
 	}
 }
+
+void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
+					   int ntevs)
+{
+	struct probe_trace_event *tev;
+	struct map *map;
+	struct symbol *sym = NULL;
+	struct rb_node *tmp;
+	int i = 0;
+
+	map = get_target_map(pev->target, pev->uprobes);
+	if (!map || map__load(map, NULL) < 0)
+		return;
+
+	for (i = 0; i < ntevs; i++) {
+		tev = &pev->tevs[i];
+		map__for_each_symbol(map, sym, tmp) {
+			if (map->unmap_ip(map, sym->start) == tev->point.address)
+				arch__fix_tev_from_maps(pev, tev, map, sym);
+		}
+	}
+}
+
 #endif
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4e215e7..5efa535 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -178,7 +178,7 @@ static struct map *kernel_get_module_map(const char *module)
 	return NULL;
 }
 
-static struct map *get_target_map(const char *target, bool user)
+struct map *get_target_map(const char *target, bool user)
 {
 	/* Init maps of given executable or kernel */
 	if (user)
@@ -703,19 +703,32 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 	return skipped;
 }
 
+void __weak
+arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unused,
+				      int ntevs __maybe_unused)
+{
+}
+
 /* Post processing the probe events */
-static int post_process_probe_trace_events(struct probe_trace_event *tevs,
+static int post_process_probe_trace_events(struct perf_probe_event *pev,
+					   struct probe_trace_event *tevs,
 					   int ntevs, const char *module,
 					   bool uprobe)
 {
-	if (uprobe)
-		return add_exec_to_probe_trace_events(tevs, ntevs, module);
+	int ret;
 
-	if (module)
+	if (uprobe)
+		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
+	else if (module)
 		/* Currently ref_reloc_sym based probe is not for drivers */
-		return add_module_to_probe_trace_events(tevs, ntevs, module);
+		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
+	else
+		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
 
-	return post_process_kernel_probe_trace_events(tevs, ntevs);
+	if (ret >= 0)
+		arch__post_process_probe_trace_events(pev, ntevs);
+
+	return ret;
 }
 
 /* Try to find perf_probe_event with debuginfo */
@@ -756,7 +769,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
 		pr_debug("Found %d probe_trace_events.\n", ntevs);
-		ret = post_process_probe_trace_events(*tevs, ntevs,
+		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
 						pev->target, pev->uprobes);
 		if (ret < 0 || ret == ntevs) {
 			clear_probe_trace_events(*tevs, ntevs);
@@ -2943,8 +2956,6 @@ errout:
 	return err;
 }
 
-bool __weak arch__prefers_symtab(void) { return false; }
-
 /* Concatinate two arrays */
 static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b)
 {
@@ -3165,12 +3176,6 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 	if (ret > 0 || pev->sdt)	/* SDT can be found only in the cache */
 		return ret == 0 ? -ENOENT : ret; /* Found in probe cache */
 
-	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
-		ret = find_probe_trace_events_from_map(pev, tevs);
-		if (ret > 0)
-			return ret; /* Found in symbol table */
-	}
-
 	/* Convert perf_probe_event with debuginfo */
 	ret = try_to_find_probe_trace_events(pev, tevs);
 	if (ret != 0)
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e18ea9f..f4f45db 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -158,7 +158,6 @@ int show_line_range(struct line_range *lr, const char *module, bool user);
 int show_available_vars(struct perf_probe_event *pevs, int npevs,
 			struct strfilter *filter);
 int show_available_funcs(const char *module, struct strfilter *filter, bool user);
-bool arch__prefers_symtab(void);
 void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 			     struct probe_trace_event *tev, struct map *map,
 			     struct symbol *sym);
@@ -173,4 +172,9 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
 int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
 			    struct perf_probe_arg *pvar);
 
+struct map *get_target_map(const char *target, bool user);
+
+void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
+					   int ntevs);
+
 #endif /*_PROBE_EVENT_H */
-- 
2.7.4

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-09  6:23 ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Ravi Bangoria
@ 2016-08-09 14:02   ` Masami Hiramatsu
  2016-08-09 15:09     ` Arnaldo Carvalho de Melo
  2016-08-09 19:21   ` [tip:perf/urgent] perf probe " tip-bot for Ravi Bangoria
  2016-08-10 23:54   ` [PATCH 2/2] perf " Anton Blanchard
  2 siblings, 1 reply; 16+ messages in thread
From: Masami Hiramatsu @ 2016-08-09 14:02 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: linux-kernel, peterz, mingo, acme, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung

On Tue,  9 Aug 2016 01:23:25 -0500
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Powerpc has Global Entry Point and Local Entry Point for functions.
> LEP catches call from both the GEP and the LEP. Symbol table of ELF
> contains GEP and Offset from which we can calculate LEP, but debuginfo
> does not have LEP info.
> 
> Currently, perf prioritize symbol table over dwarf to probe on LEP
> for ppc64le. But when user tries to probe with function parameter,
> we fall back to using dwarf(i.e. GEP) and when function called via
> LEP, probe will never hit.
> 
> For example:
>   $ objdump -d vmlinux
>     ...
>     do_sys_open():
>     c0000000002eb4a0:       e8 00 4c 3c     addis   r2,r12,232
>     c0000000002eb4a4:       60 00 42 38     addi    r2,r2,96
>     c0000000002eb4a8:       a6 02 08 7c     mflr    r0
>     c0000000002eb4ac:       d0 ff 41 fb     std     r26,-48(r1)
> 
>   $ sudo ./perf probe do_sys_open
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/do_sys_open _text+3060904
> 
>   $ sudo ./perf probe 'do_sys_open filename:string'
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/do_sys_open _text+3060896 filename_string=+0(%gpr4):string
> 
> For second case, perf probed on GEP. So when function will be called
> via LEP, probe won't hit.
> 
>   $ sudo ./perf record -a -e probe:do_sys_open ls
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.195 MB perf.data ]
> 
> To resolve this issue, let's not prioritize symbol table, let perf
> decide what it wants to use. Perf is already converting GEP to LEP
> when it uses symbol table. When perf uses debuginfo, let it find
> LEP offset form symbol table. This way we fall back to probe on LEP
> for all cases.
> 
> After patch:
>   $ sudo ./perf probe 'do_sys_open filename:string'
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/do_sys_open _text+3060904 filename_string=+0(%gpr4):string
> 
>   $ sudo ./perf record -a -e probe:do_sys_open ls
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.197 MB perf.data (11 samples) ]

OK, I see.
And I'd like to add note below.

 This reverts commit d5c2e2c17ae1 ("perf probe ppc64le: Prefer symbol
 table lookup over DWARF").

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

for this series. 

Thanks,

> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/arch/powerpc/util/sym-handling.c | 27 +++++++++++++++++----
>  tools/perf/util/probe-event.c               | 37 ++++++++++++++++-------------
>  tools/perf/util/probe-event.h               |  6 ++++-
>  3 files changed, 49 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
> index c6d0f91..8d4dc97 100644
> --- a/tools/perf/arch/powerpc/util/sym-handling.c
> +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> @@ -54,10 +54,6 @@ int arch__compare_symbol_names(const char *namea, const char *nameb)
>  #endif
>  
>  #if defined(_CALL_ELF) && _CALL_ELF == 2
> -bool arch__prefers_symtab(void)
> -{
> -	return true;
> -}
>  
>  #ifdef HAVE_LIBELF_SUPPORT
>  void arch__sym_update(struct symbol *s, GElf_Sym *sym)
> @@ -100,4 +96,27 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
>  			tev->point.offset += lep_offset;
>  	}
>  }
> +
> +void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
> +					   int ntevs)
> +{
> +	struct probe_trace_event *tev;
> +	struct map *map;
> +	struct symbol *sym = NULL;
> +	struct rb_node *tmp;
> +	int i = 0;
> +
> +	map = get_target_map(pev->target, pev->uprobes);
> +	if (!map || map__load(map, NULL) < 0)
> +		return;
> +
> +	for (i = 0; i < ntevs; i++) {
> +		tev = &pev->tevs[i];
> +		map__for_each_symbol(map, sym, tmp) {
> +			if (map->unmap_ip(map, sym->start) == tev->point.address)
> +				arch__fix_tev_from_maps(pev, tev, map, sym);
> +		}
> +	}
> +}
> +
>  #endif
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4e215e7..5efa535 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -178,7 +178,7 @@ static struct map *kernel_get_module_map(const char *module)
>  	return NULL;
>  }
>  
> -static struct map *get_target_map(const char *target, bool user)
> +struct map *get_target_map(const char *target, bool user)
>  {
>  	/* Init maps of given executable or kernel */
>  	if (user)
> @@ -703,19 +703,32 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
>  	return skipped;
>  }
>  
> +void __weak
> +arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unused,
> +				      int ntevs __maybe_unused)
> +{
> +}
> +
>  /* Post processing the probe events */
> -static int post_process_probe_trace_events(struct probe_trace_event *tevs,
> +static int post_process_probe_trace_events(struct perf_probe_event *pev,
> +					   struct probe_trace_event *tevs,
>  					   int ntevs, const char *module,
>  					   bool uprobe)
>  {
> -	if (uprobe)
> -		return add_exec_to_probe_trace_events(tevs, ntevs, module);
> +	int ret;
>  
> -	if (module)
> +	if (uprobe)
> +		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
> +	else if (module)
>  		/* Currently ref_reloc_sym based probe is not for drivers */
> -		return add_module_to_probe_trace_events(tevs, ntevs, module);
> +		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
> +	else
> +		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
>  
> -	return post_process_kernel_probe_trace_events(tevs, ntevs);
> +	if (ret >= 0)
> +		arch__post_process_probe_trace_events(pev, ntevs);
> +
> +	return ret;
>  }
>  
>  /* Try to find perf_probe_event with debuginfo */
> @@ -756,7 +769,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>  
>  	if (ntevs > 0) {	/* Succeeded to find trace events */
>  		pr_debug("Found %d probe_trace_events.\n", ntevs);
> -		ret = post_process_probe_trace_events(*tevs, ntevs,
> +		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
>  						pev->target, pev->uprobes);
>  		if (ret < 0 || ret == ntevs) {
>  			clear_probe_trace_events(*tevs, ntevs);
> @@ -2943,8 +2956,6 @@ errout:
>  	return err;
>  }
>  
> -bool __weak arch__prefers_symtab(void) { return false; }
> -
>  /* Concatinate two arrays */
>  static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b)
>  {
> @@ -3165,12 +3176,6 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>  	if (ret > 0 || pev->sdt)	/* SDT can be found only in the cache */
>  		return ret == 0 ? -ENOENT : ret; /* Found in probe cache */
>  
> -	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
> -		ret = find_probe_trace_events_from_map(pev, tevs);
> -		if (ret > 0)
> -			return ret; /* Found in symbol table */
> -	}
> -
>  	/* Convert perf_probe_event with debuginfo */
>  	ret = try_to_find_probe_trace_events(pev, tevs);
>  	if (ret != 0)
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index e18ea9f..f4f45db 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -158,7 +158,6 @@ int show_line_range(struct line_range *lr, const char *module, bool user);
>  int show_available_vars(struct perf_probe_event *pevs, int npevs,
>  			struct strfilter *filter);
>  int show_available_funcs(const char *module, struct strfilter *filter, bool user);
> -bool arch__prefers_symtab(void);
>  void arch__fix_tev_from_maps(struct perf_probe_event *pev,
>  			     struct probe_trace_event *tev, struct map *map,
>  			     struct symbol *sym);
> @@ -173,4 +172,9 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
>  int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
>  			    struct perf_probe_arg *pvar);
>  
> +struct map *get_target_map(const char *target, bool user);
> +
> +void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
> +					   int ntevs);
> +
>  #endif /*_PROBE_EVENT_H */
> -- 
> 2.7.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-09 14:02   ` Masami Hiramatsu
@ 2016-08-09 15:09     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-09 15:09 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ravi Bangoria, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, wangnan0, namhyung

Em Tue, Aug 09, 2016 at 11:02:40PM +0900, Masami Hiramatsu escreveu:
> On Tue,  9 Aug 2016 01:23:25 -0500
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
> 
> > Powerpc has Global Entry Point and Local Entry Point for functions.
> > LEP catches call from both the GEP and the LEP. Symbol table of ELF
> > contains GEP and Offset from which we can calculate LEP, but debuginfo
> > does not have LEP info.
> > 
> > Currently, perf prioritize symbol table over dwarf to probe on LEP
> > for ppc64le. But when user tries to probe with function parameter,
> > we fall back to using dwarf(i.e. GEP) and when function called via
> > LEP, probe will never hit.
> > 
> > For example:
> >   $ objdump -d vmlinux
> >     ...
> >     do_sys_open():
> >     c0000000002eb4a0:       e8 00 4c 3c     addis   r2,r12,232
> >     c0000000002eb4a4:       60 00 42 38     addi    r2,r2,96
> >     c0000000002eb4a8:       a6 02 08 7c     mflr    r0
> >     c0000000002eb4ac:       d0 ff 41 fb     std     r26,-48(r1)
> > 
> >   $ sudo ./perf probe do_sys_open
> >   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
> >     p:probe/do_sys_open _text+3060904
> > 
> >   $ sudo ./perf probe 'do_sys_open filename:string'
> >   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
> >     p:probe/do_sys_open _text+3060896 filename_string=+0(%gpr4):string
> > 
> > For second case, perf probed on GEP. So when function will be called
> > via LEP, probe won't hit.
> > 
> >   $ sudo ./perf record -a -e probe:do_sys_open ls
> >     [ perf record: Woken up 1 times to write data ]
> >     [ perf record: Captured and wrote 0.195 MB perf.data ]
> > 
> > To resolve this issue, let's not prioritize symbol table, let perf
> > decide what it wants to use. Perf is already converting GEP to LEP
> > when it uses symbol table. When perf uses debuginfo, let it find
> > LEP offset form symbol table. This way we fall back to probe on LEP
> > for all cases.
> > 
> > After patch:
> >   $ sudo ./perf probe 'do_sys_open filename:string'
> >   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
> >     p:probe/do_sys_open _text+3060904 filename_string=+0(%gpr4):string
> > 
> >   $ sudo ./perf record -a -e probe:do_sys_open ls
> >     [ perf record: Woken up 1 times to write data ]
> >     [ perf record: Captured and wrote 0.197 MB perf.data (11 samples) ]
> 
> OK, I see.
> And I'd like to add note below.
> 
>  This reverts commit d5c2e2c17ae1 ("perf probe ppc64le: Prefer symbol
>  table lookup over DWARF").
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Ok, I'll do that to expedite this a little bit, trying to get this in my
next perf/urgent pull req to Ingo, which I expect to send soon.

- Arnaldo
 
> for this series. 
> 
> Thanks,
> 
> > 
> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> > ---
> >  tools/perf/arch/powerpc/util/sym-handling.c | 27 +++++++++++++++++----
> >  tools/perf/util/probe-event.c               | 37 ++++++++++++++++-------------
> >  tools/perf/util/probe-event.h               |  6 ++++-
> >  3 files changed, 49 insertions(+), 21 deletions(-)
> > 
> > diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
> > index c6d0f91..8d4dc97 100644
> > --- a/tools/perf/arch/powerpc/util/sym-handling.c
> > +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> > @@ -54,10 +54,6 @@ int arch__compare_symbol_names(const char *namea, const char *nameb)
> >  #endif
> >  
> >  #if defined(_CALL_ELF) && _CALL_ELF == 2
> > -bool arch__prefers_symtab(void)
> > -{
> > -	return true;
> > -}
> >  
> >  #ifdef HAVE_LIBELF_SUPPORT
> >  void arch__sym_update(struct symbol *s, GElf_Sym *sym)
> > @@ -100,4 +96,27 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
> >  			tev->point.offset += lep_offset;
> >  	}
> >  }
> > +
> > +void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
> > +					   int ntevs)
> > +{
> > +	struct probe_trace_event *tev;
> > +	struct map *map;
> > +	struct symbol *sym = NULL;
> > +	struct rb_node *tmp;
> > +	int i = 0;
> > +
> > +	map = get_target_map(pev->target, pev->uprobes);
> > +	if (!map || map__load(map, NULL) < 0)
> > +		return;
> > +
> > +	for (i = 0; i < ntevs; i++) {
> > +		tev = &pev->tevs[i];
> > +		map__for_each_symbol(map, sym, tmp) {
> > +			if (map->unmap_ip(map, sym->start) == tev->point.address)
> > +				arch__fix_tev_from_maps(pev, tev, map, sym);
> > +		}
> > +	}
> > +}
> > +
> >  #endif
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 4e215e7..5efa535 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -178,7 +178,7 @@ static struct map *kernel_get_module_map(const char *module)
> >  	return NULL;
> >  }
> >  
> > -static struct map *get_target_map(const char *target, bool user)
> > +struct map *get_target_map(const char *target, bool user)
> >  {
> >  	/* Init maps of given executable or kernel */
> >  	if (user)
> > @@ -703,19 +703,32 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
> >  	return skipped;
> >  }
> >  
> > +void __weak
> > +arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unused,
> > +				      int ntevs __maybe_unused)
> > +{
> > +}
> > +
> >  /* Post processing the probe events */
> > -static int post_process_probe_trace_events(struct probe_trace_event *tevs,
> > +static int post_process_probe_trace_events(struct perf_probe_event *pev,
> > +					   struct probe_trace_event *tevs,
> >  					   int ntevs, const char *module,
> >  					   bool uprobe)
> >  {
> > -	if (uprobe)
> > -		return add_exec_to_probe_trace_events(tevs, ntevs, module);
> > +	int ret;
> >  
> > -	if (module)
> > +	if (uprobe)
> > +		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
> > +	else if (module)
> >  		/* Currently ref_reloc_sym based probe is not for drivers */
> > -		return add_module_to_probe_trace_events(tevs, ntevs, module);
> > +		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
> > +	else
> > +		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
> >  
> > -	return post_process_kernel_probe_trace_events(tevs, ntevs);
> > +	if (ret >= 0)
> > +		arch__post_process_probe_trace_events(pev, ntevs);
> > +
> > +	return ret;
> >  }
> >  
> >  /* Try to find perf_probe_event with debuginfo */
> > @@ -756,7 +769,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> >  
> >  	if (ntevs > 0) {	/* Succeeded to find trace events */
> >  		pr_debug("Found %d probe_trace_events.\n", ntevs);
> > -		ret = post_process_probe_trace_events(*tevs, ntevs,
> > +		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
> >  						pev->target, pev->uprobes);
> >  		if (ret < 0 || ret == ntevs) {
> >  			clear_probe_trace_events(*tevs, ntevs);
> > @@ -2943,8 +2956,6 @@ errout:
> >  	return err;
> >  }
> >  
> > -bool __weak arch__prefers_symtab(void) { return false; }
> > -
> >  /* Concatinate two arrays */
> >  static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b)
> >  {
> > @@ -3165,12 +3176,6 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >  	if (ret > 0 || pev->sdt)	/* SDT can be found only in the cache */
> >  		return ret == 0 ? -ENOENT : ret; /* Found in probe cache */
> >  
> > -	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
> > -		ret = find_probe_trace_events_from_map(pev, tevs);
> > -		if (ret > 0)
> > -			return ret; /* Found in symbol table */
> > -	}
> > -
> >  	/* Convert perf_probe_event with debuginfo */
> >  	ret = try_to_find_probe_trace_events(pev, tevs);
> >  	if (ret != 0)
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index e18ea9f..f4f45db 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -158,7 +158,6 @@ int show_line_range(struct line_range *lr, const char *module, bool user);
> >  int show_available_vars(struct perf_probe_event *pevs, int npevs,
> >  			struct strfilter *filter);
> >  int show_available_funcs(const char *module, struct strfilter *filter, bool user);
> > -bool arch__prefers_symtab(void);
> >  void arch__fix_tev_from_maps(struct perf_probe_event *pev,
> >  			     struct probe_trace_event *tev, struct map *map,
> >  			     struct symbol *sym);
> > @@ -173,4 +172,9 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
> >  int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> >  			    struct perf_probe_arg *pvar);
> >  
> > +struct map *get_target_map(const char *target, bool user);
> > +
> > +void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
> > +					   int ntevs);
> > +
> >  #endif /*_PROBE_EVENT_H */
> > -- 
> > 2.7.4
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

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

* [tip:perf/urgent] perf probe: Add function to post process kernel trace events
  2016-08-09  6:23 [PATCH 1/2] perf: Add function to post process kernel trace events Ravi Bangoria
  2016-08-09  6:23 ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Ravi Bangoria
@ 2016-08-09 19:20 ` tip-bot for Ravi Bangoria
  1 sibling, 0 replies; 16+ messages in thread
From: tip-bot for Ravi Bangoria @ 2016-08-09 19:20 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: alexander.shishkin, linux-kernel, tglx, namhyung, mingo,
	naveen.n.rao, ananth, ravi.bangoria, bsingharora, mhiramat,
	peterz, acme, wangnan0, hpa

Commit-ID:  d820456dc70b231d62171ba46b43db0045e9bd57
Gitweb:     http://git.kernel.org/tip/d820456dc70b231d62171ba46b43db0045e9bd57
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Tue, 9 Aug 2016 01:23:24 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 9 Aug 2016 12:09:59 -0300

perf probe: Add function to post process kernel trace events

Instead of inline code, introduce function to post process kernel
probe trace events.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1470723805-5081-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1201f73..234fbfb 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -666,22 +666,14 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
 	return ret;
 }
 
-/* Post processing the probe events */
-static int post_process_probe_trace_events(struct probe_trace_event *tevs,
-					   int ntevs, const char *module,
-					   bool uprobe)
+static int
+post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
+				       int ntevs)
 {
 	struct ref_reloc_sym *reloc_sym;
 	char *tmp;
 	int i, skipped = 0;
 
-	if (uprobe)
-		return add_exec_to_probe_trace_events(tevs, ntevs, module);
-
-	/* Note that currently ref_reloc_sym based probe is not for drivers */
-	if (module)
-		return add_module_to_probe_trace_events(tevs, ntevs, module);
-
 	reloc_sym = kernel_get_ref_reloc_sym();
 	if (!reloc_sym) {
 		pr_warning("Relocated base symbol is not found!\n");
@@ -713,6 +705,21 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 	return skipped;
 }
 
+/* Post processing the probe events */
+static int post_process_probe_trace_events(struct probe_trace_event *tevs,
+					   int ntevs, const char *module,
+					   bool uprobe)
+{
+	if (uprobe)
+		return add_exec_to_probe_trace_events(tevs, ntevs, module);
+
+	if (module)
+		/* Currently ref_reloc_sym based probe is not for drivers */
+		return add_module_to_probe_trace_events(tevs, ntevs, module);
+
+	return post_process_kernel_probe_trace_events(tevs, ntevs);
+}
+
 /* Try to find perf_probe_event with debuginfo */
 static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 					  struct probe_trace_event **tevs)

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

* [tip:perf/urgent] perf probe ppc64le: Fix probe location when using DWARF
  2016-08-09  6:23 ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Ravi Bangoria
  2016-08-09 14:02   ` Masami Hiramatsu
@ 2016-08-09 19:21   ` tip-bot for Ravi Bangoria
  2016-08-10 23:54   ` [PATCH 2/2] perf " Anton Blanchard
  2 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Ravi Bangoria @ 2016-08-09 19:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: ananth, mingo, wangnan0, alexander.shishkin, mhiramat, namhyung,
	hpa, bsingharora, naveen.n.rao, tglx, peterz, acme, linux-kernel,
	ravi.bangoria

Commit-ID:  99e608b5954c9e1ebadbf9660b74697d9dfd9f20
Gitweb:     http://git.kernel.org/tip/99e608b5954c9e1ebadbf9660b74697d9dfd9f20
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Tue, 9 Aug 2016 01:23:25 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 9 Aug 2016 12:14:29 -0300

perf probe ppc64le: Fix probe location when using DWARF

Powerpc has Global Entry Point and Local Entry Point for functions.  LEP
catches call from both the GEP and the LEP. Symbol table of ELF contains
GEP and Offset from which we can calculate LEP, but debuginfo does not
have LEP info.

Currently, perf prioritize symbol table over dwarf to probe on LEP for
ppc64le. But when user tries to probe with function parameter, we fall
back to using dwarf(i.e. GEP) and when function called via LEP, probe
will never hit.

For example:

  $ objdump -d vmlinux
    ...
    do_sys_open():
    c0000000002eb4a0:       e8 00 4c 3c     addis   r2,r12,232
    c0000000002eb4a4:       60 00 42 38     addi    r2,r2,96
    c0000000002eb4a8:       a6 02 08 7c     mflr    r0
    c0000000002eb4ac:       d0 ff 41 fb     std     r26,-48(r1)

  $ sudo ./perf probe do_sys_open
  $ sudo cat /sys/kernel/debug/tracing/kprobe_events
    p:probe/do_sys_open _text+3060904

  $ sudo ./perf probe 'do_sys_open filename:string'
  $ sudo cat /sys/kernel/debug/tracing/kprobe_events
    p:probe/do_sys_open _text+3060896 filename_string=+0(%gpr4):string

For second case, perf probed on GEP. So when function will be called via
LEP, probe won't hit.

  $ sudo ./perf record -a -e probe:do_sys_open ls
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.195 MB perf.data ]

To resolve this issue, let's not prioritize symbol table, let perf
decide what it wants to use. Perf is already converting GEP to LEP when
it uses symbol table. When perf uses debuginfo, let it find LEP offset
form symbol table. This way we fall back to probe on LEP for all cases.

After patch:

  $ sudo ./perf probe 'do_sys_open filename:string'
  $ sudo cat /sys/kernel/debug/tracing/kprobe_events
    p:probe/do_sys_open _text+3060904 filename_string=+0(%gpr4):string

  $ sudo ./perf record -a -e probe:do_sys_open ls
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.197 MB perf.data (11 samples) ]

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1470723805-5081-2-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 27 +++++++++++++++++----
 tools/perf/util/probe-event.c               | 37 ++++++++++++++++-------------
 tools/perf/util/probe-event.h               |  6 ++++-
 3 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index c6d0f91..8d4dc97 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -54,10 +54,6 @@ int arch__compare_symbol_names(const char *namea, const char *nameb)
 #endif
 
 #if defined(_CALL_ELF) && _CALL_ELF == 2
-bool arch__prefers_symtab(void)
-{
-	return true;
-}
 
 #ifdef HAVE_LIBELF_SUPPORT
 void arch__sym_update(struct symbol *s, GElf_Sym *sym)
@@ -100,4 +96,27 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 			tev->point.offset += lep_offset;
 	}
 }
+
+void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
+					   int ntevs)
+{
+	struct probe_trace_event *tev;
+	struct map *map;
+	struct symbol *sym = NULL;
+	struct rb_node *tmp;
+	int i = 0;
+
+	map = get_target_map(pev->target, pev->uprobes);
+	if (!map || map__load(map, NULL) < 0)
+		return;
+
+	for (i = 0; i < ntevs; i++) {
+		tev = &pev->tevs[i];
+		map__for_each_symbol(map, sym, tmp) {
+			if (map->unmap_ip(map, sym->start) == tev->point.address)
+				arch__fix_tev_from_maps(pev, tev, map, sym);
+		}
+	}
+}
+
 #endif
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 234fbfb..2873396 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -180,7 +180,7 @@ static struct map *kernel_get_module_map(const char *module)
 	return NULL;
 }
 
-static struct map *get_target_map(const char *target, bool user)
+struct map *get_target_map(const char *target, bool user)
 {
 	/* Init maps of given executable or kernel */
 	if (user)
@@ -705,19 +705,32 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 	return skipped;
 }
 
+void __weak
+arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unused,
+				      int ntevs __maybe_unused)
+{
+}
+
 /* Post processing the probe events */
-static int post_process_probe_trace_events(struct probe_trace_event *tevs,
+static int post_process_probe_trace_events(struct perf_probe_event *pev,
+					   struct probe_trace_event *tevs,
 					   int ntevs, const char *module,
 					   bool uprobe)
 {
-	if (uprobe)
-		return add_exec_to_probe_trace_events(tevs, ntevs, module);
+	int ret;
 
-	if (module)
+	if (uprobe)
+		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
+	else if (module)
 		/* Currently ref_reloc_sym based probe is not for drivers */
-		return add_module_to_probe_trace_events(tevs, ntevs, module);
+		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
+	else
+		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
 
-	return post_process_kernel_probe_trace_events(tevs, ntevs);
+	if (ret >= 0)
+		arch__post_process_probe_trace_events(pev, ntevs);
+
+	return ret;
 }
 
 /* Try to find perf_probe_event with debuginfo */
@@ -758,7 +771,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
 		pr_debug("Found %d probe_trace_events.\n", ntevs);
-		ret = post_process_probe_trace_events(*tevs, ntevs,
+		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
 						pev->target, pev->uprobes);
 		if (ret < 0 || ret == ntevs) {
 			clear_probe_trace_events(*tevs, ntevs);
@@ -2945,8 +2958,6 @@ errout:
 	return err;
 }
 
-bool __weak arch__prefers_symtab(void) { return false; }
-
 /* Concatinate two arrays */
 static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b)
 {
@@ -3167,12 +3178,6 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 	if (ret > 0 || pev->sdt)	/* SDT can be found only in the cache */
 		return ret == 0 ? -ENOENT : ret; /* Found in probe cache */
 
-	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
-		ret = find_probe_trace_events_from_map(pev, tevs);
-		if (ret > 0)
-			return ret; /* Found in symbol table */
-	}
-
 	/* Convert perf_probe_event with debuginfo */
 	ret = try_to_find_probe_trace_events(pev, tevs);
 	if (ret != 0)
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e18ea9f..f4f45db 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -158,7 +158,6 @@ int show_line_range(struct line_range *lr, const char *module, bool user);
 int show_available_vars(struct perf_probe_event *pevs, int npevs,
 			struct strfilter *filter);
 int show_available_funcs(const char *module, struct strfilter *filter, bool user);
-bool arch__prefers_symtab(void);
 void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 			     struct probe_trace_event *tev, struct map *map,
 			     struct symbol *sym);
@@ -173,4 +172,9 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
 int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
 			    struct perf_probe_arg *pvar);
 
+struct map *get_target_map(const char *target, bool user);
+
+void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
+					   int ntevs);
+
 #endif /*_PROBE_EVENT_H */

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-09  6:23 ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Ravi Bangoria
  2016-08-09 14:02   ` Masami Hiramatsu
  2016-08-09 19:21   ` [tip:perf/urgent] perf probe " tip-bot for Ravi Bangoria
@ 2016-08-10 23:54   ` Anton Blanchard
  2016-08-11  4:31     ` Ravi Bangoria
  2016-08-11 15:49     ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Arnaldo Carvalho de Melo
  2 siblings, 2 replies; 16+ messages in thread
From: Anton Blanchard @ 2016-08-10 23:54 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: linux-kernel, peterz, mingo, acme, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung

Hi,

> Powerpc has Global Entry Point and Local Entry Point for functions.
> LEP catches call from both the GEP and the LEP. Symbol table of ELF
> contains GEP and Offset from which we can calculate LEP, but debuginfo
> does not have LEP info.
> 
> Currently, perf prioritize symbol table over dwarf to probe on LEP
> for ppc64le. But when user tries to probe with function parameter,
> we fall back to using dwarf(i.e. GEP) and when function called via
> LEP, probe will never hit.

This patch causes a build failure for me on ppc64le:

libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':

tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'

Anton

> For example:
>   $ objdump -d vmlinux
>     ...
>     do_sys_open():
>     c0000000002eb4a0:       e8 00 4c 3c     addis   r2,r12,232
>     c0000000002eb4a4:       60 00 42 38     addi    r2,r2,96
>     c0000000002eb4a8:       a6 02 08 7c     mflr    r0
>     c0000000002eb4ac:       d0 ff 41 fb     std     r26,-48(r1)
> 
>   $ sudo ./perf probe do_sys_open
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/do_sys_open _text+3060904
> 
>   $ sudo ./perf probe 'do_sys_open filename:string'
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/do_sys_open _text+3060896 filename_string=+0(%gpr4):string
> 
> For second case, perf probed on GEP. So when function will be called
> via LEP, probe won't hit.
> 
>   $ sudo ./perf record -a -e probe:do_sys_open ls
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.195 MB perf.data ]
> 
> To resolve this issue, let's not prioritize symbol table, let perf
> decide what it wants to use. Perf is already converting GEP to LEP
> when it uses symbol table. When perf uses debuginfo, let it find
> LEP offset form symbol table. This way we fall back to probe on LEP
> for all cases.
> 
> After patch:
>   $ sudo ./perf probe 'do_sys_open filename:string'
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/do_sys_open _text+3060904 filename_string=+0(%gpr4):string
> 
>   $ sudo ./perf record -a -e probe:do_sys_open ls
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.197 MB perf.data (11 samples)
> ]
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/arch/powerpc/util/sym-handling.c | 27
> +++++++++++++++++---- tools/perf/util/probe-event.c               |
> 37 ++++++++++++++++-------------
> tools/perf/util/probe-event.h               |  6 ++++- 3 files
> changed, 49 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/perf/arch/powerpc/util/sym-handling.c
> b/tools/perf/arch/powerpc/util/sym-handling.c index c6d0f91..8d4dc97
> 100644 --- a/tools/perf/arch/powerpc/util/sym-handling.c
> +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> @@ -54,10 +54,6 @@ int arch__compare_symbol_names(const char *namea,
> const char *nameb) #endif
>  
>  #if defined(_CALL_ELF) && _CALL_ELF == 2
> -bool arch__prefers_symtab(void)
> -{
> -	return true;
> -}
>  
>  #ifdef HAVE_LIBELF_SUPPORT
>  void arch__sym_update(struct symbol *s, GElf_Sym *sym)
> @@ -100,4 +96,27 @@ void arch__fix_tev_from_maps(struct
> perf_probe_event *pev, tev->point.offset += lep_offset;
>  	}
>  }
> +
> +void arch__post_process_probe_trace_events(struct perf_probe_event
> *pev,
> +					   int ntevs)
> +{
> +	struct probe_trace_event *tev;
> +	struct map *map;
> +	struct symbol *sym = NULL;
> +	struct rb_node *tmp;
> +	int i = 0;
> +
> +	map = get_target_map(pev->target, pev->uprobes);
> +	if (!map || map__load(map, NULL) < 0)
> +		return;
> +
> +	for (i = 0; i < ntevs; i++) {
> +		tev = &pev->tevs[i];
> +		map__for_each_symbol(map, sym, tmp) {
> +			if (map->unmap_ip(map, sym->start) ==
> tev->point.address)
> +				arch__fix_tev_from_maps(pev, tev,
> map, sym);
> +		}
> +	}
> +}
> +
>  #endif
> diff --git a/tools/perf/util/probe-event.c
> b/tools/perf/util/probe-event.c index 4e215e7..5efa535 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -178,7 +178,7 @@ static struct map *kernel_get_module_map(const
> char *module) return NULL;
>  }
>  
> -static struct map *get_target_map(const char *target, bool user)
> +struct map *get_target_map(const char *target, bool user)
>  {
>  	/* Init maps of given executable or kernel */
>  	if (user)
> @@ -703,19 +703,32 @@ post_process_kernel_probe_trace_events(struct
> probe_trace_event *tevs, return skipped;
>  }
>  
> +void __weak
> +arch__post_process_probe_trace_events(struct perf_probe_event *pev
> __maybe_unused,
> +				      int ntevs __maybe_unused)
> +{
> +}
> +
>  /* Post processing the probe events */
> -static int post_process_probe_trace_events(struct probe_trace_event
> *tevs, +static int post_process_probe_trace_events(struct
> perf_probe_event *pev,
> +					   struct probe_trace_event
> *tevs, int ntevs, const char *module,
>  					   bool uprobe)
>  {
> -	if (uprobe)
> -		return add_exec_to_probe_trace_events(tevs, ntevs,
> module);
> +	int ret;
>  
> -	if (module)
> +	if (uprobe)
> +		ret = add_exec_to_probe_trace_events(tevs, ntevs,
> module);
> +	else if (module)
>  		/* Currently ref_reloc_sym based probe is not for
> drivers */
> -		return add_module_to_probe_trace_events(tevs, ntevs,
> module);
> +		ret = add_module_to_probe_trace_events(tevs, ntevs,
> module);
> +	else
> +		ret = post_process_kernel_probe_trace_events(tevs,
> ntevs); 
> -	return post_process_kernel_probe_trace_events(tevs, ntevs);
> +	if (ret >= 0)
> +		arch__post_process_probe_trace_events(pev, ntevs);
> +
> +	return ret;
>  }
>  
>  /* Try to find perf_probe_event with debuginfo */
> @@ -756,7 +769,7 @@ static int try_to_find_probe_trace_events(struct
> perf_probe_event *pev, 
>  	if (ntevs > 0) {	/* Succeeded to find trace events */
>  		pr_debug("Found %d probe_trace_events.\n", ntevs);
> -		ret = post_process_probe_trace_events(*tevs, ntevs,
> +		ret = post_process_probe_trace_events(pev, *tevs,
> ntevs, pev->target, pev->uprobes);
>  		if (ret < 0 || ret == ntevs) {
>  			clear_probe_trace_events(*tevs, ntevs);
> @@ -2943,8 +2956,6 @@ errout:
>  	return err;
>  }
>  
> -bool __weak arch__prefers_symtab(void) { return false; }
> -
>  /* Concatinate two arrays */
>  static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b)
>  {
> @@ -3165,12 +3176,6 @@ static int
> convert_to_probe_trace_events(struct perf_probe_event *pev, if (ret >
> 0 || pev->sdt)	/* SDT can be found only in the cache */ return
> ret == 0 ? -ENOENT : ret; /* Found in probe cache */ 
> -	if (arch__prefers_symtab()
> && !perf_probe_event_need_dwarf(pev)) {
> -		ret = find_probe_trace_events_from_map(pev, tevs);
> -		if (ret > 0)
> -			return ret; /* Found in symbol table */
> -	}
> -
>  	/* Convert perf_probe_event with debuginfo */
>  	ret = try_to_find_probe_trace_events(pev, tevs);
>  	if (ret != 0)
> diff --git a/tools/perf/util/probe-event.h
> b/tools/perf/util/probe-event.h index e18ea9f..f4f45db 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -158,7 +158,6 @@ int show_line_range(struct line_range *lr, const
> char *module, bool user); int show_available_vars(struct
> perf_probe_event *pevs, int npevs, struct strfilter *filter);
>  int show_available_funcs(const char *module, struct strfilter
> *filter, bool user); -bool arch__prefers_symtab(void);
>  void arch__fix_tev_from_maps(struct perf_probe_event *pev,
>  			     struct probe_trace_event *tev, struct
> map *map, struct symbol *sym);
> @@ -173,4 +172,9 @@ int e_snprintf(char *str, size_t size, const char
> *format, ...) int copy_to_probe_trace_arg(struct probe_trace_arg
> *tvar, struct perf_probe_arg *pvar);
>  
> +struct map *get_target_map(const char *target, bool user);
> +
> +void arch__post_process_probe_trace_events(struct perf_probe_event
> *pev,
> +					   int ntevs);
> +
>  #endif /*_PROBE_EVENT_H */

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-10 23:54   ` [PATCH 2/2] perf " Anton Blanchard
@ 2016-08-11  4:31     ` Ravi Bangoria
  2016-08-11 11:50       ` Arnaldo Carvalho de Melo
  2016-08-16 18:14       ` [tip:perf/urgent] perf ppc64le: Fix build failure when libelf is not present tip-bot for Ravi Bangoria
  2016-08-11 15:49     ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Arnaldo Carvalho de Melo
  1 sibling, 2 replies; 16+ messages in thread
From: Ravi Bangoria @ 2016-08-11  4:31 UTC (permalink / raw)
  To: Anton Blanchard, acme
  Cc: linux-kernel, peterz, mingo, alexander.shishkin, bsingharora,
	naveen.n.rao, ananth, mhiramat, wangnan0, namhyung,
	Ravi Bangoria



On Thursday 11 August 2016 05:24 AM, Anton Blanchard wrote:
> Hi,
>
>> Powerpc has Global Entry Point and Local Entry Point for functions.
>> LEP catches call from both the GEP and the LEP. Symbol table of ELF
>> contains GEP and Offset from which we can calculate LEP, but debuginfo
>> does not have LEP info.
>>
>> Currently, perf prioritize symbol table over dwarf to probe on LEP
>> for ppc64le. But when user tries to probe with function parameter,
>> we fall back to using dwarf(i.e. GEP) and when function called via
>> LEP, probe will never hit.
> This patch causes a build failure for me on ppc64le:
>
> libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':
>
> tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'

Thanks Anton. Sorry, I should have caught that.

@Arnaldo, Can you please pick this up. I've prepared this on top of 
acme/perf/core.


 From 89c977ae9c3ae35c78b16cddabcf2b01d3cf5cc8 Mon Sep 17 00:00:00 2001
From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Date: Wed, 10 Aug 2016 23:13:45 -0500
Subject: [PATCH] perf ppc64le: Fix build failure when no dwarf support

Fix perf build failure on ppc64le because of Commit 99e608b5954c ("perf
probe ppc64le: Fix probe location when using DWARF")

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
  tools/perf/arch/powerpc/util/sym-handling.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 8d4dc97..c27a51a 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -97,6 +97,7 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
         }
  }

+#ifdef HAVE_LIBELF_SUPPORT
  void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
                                            int ntevs)
  {
@@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct 
perf_probe_event *pev,
                 }
         }
  }
+#endif

  #endif
-- 
2.7.4

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-11  4:31     ` Ravi Bangoria
@ 2016-08-11 11:50       ` Arnaldo Carvalho de Melo
  2016-08-11 13:21         ` Ravi Bangoria
  2016-08-12  6:06         ` Anton Blanchard
  2016-08-16 18:14       ` [tip:perf/urgent] perf ppc64le: Fix build failure when libelf is not present tip-bot for Ravi Bangoria
  1 sibling, 2 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-11 11:50 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: Anton Blanchard, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung

Em Thu, Aug 11, 2016 at 10:01:04AM +0530, Ravi Bangoria escreveu:
> 
> 
> On Thursday 11 August 2016 05:24 AM, Anton Blanchard wrote:
> > Hi,
> > 
> > > Powerpc has Global Entry Point and Local Entry Point for functions.
> > > LEP catches call from both the GEP and the LEP. Symbol table of ELF
> > > contains GEP and Offset from which we can calculate LEP, but debuginfo
> > > does not have LEP info.
> > > 
> > > Currently, perf prioritize symbol table over dwarf to probe on LEP
> > > for ppc64le. But when user tries to probe with function parameter,
> > > we fall back to using dwarf(i.e. GEP) and when function called via
> > > LEP, probe will never hit.
> > This patch causes a build failure for me on ppc64le:
> > 
> > libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':
> > 
> > tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'
> 
> Thanks Anton. Sorry, I should have caught that.
> 
> @Arnaldo, Can you please pick this up. I've prepared this on top of
> acme/perf/core.
> 
> 
> From 89c977ae9c3ae35c78b16cddabcf2b01d3cf5cc8 Mon Sep 17 00:00:00 2001
> From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> Date: Wed, 10 Aug 2016 23:13:45 -0500
> Subject: [PATCH] perf ppc64le: Fix build failure when no dwarf support
> 
> Fix perf build failure on ppc64le because of Commit 99e608b5954c ("perf
> probe ppc64le: Fix probe location when using DWARF")

Can you please provide a better explanation? I had to look at the patch
to understand what it was fixing, and then the patch adds LIBELF_SUPPORT
ifdefs while the patch description, talks about DWARF.

Anyway, Anton, does this fix the problem for you?

- Arnaldo
 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/arch/powerpc/util/sym-handling.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/arch/powerpc/util/sym-handling.c
> b/tools/perf/arch/powerpc/util/sym-handling.c
> index 8d4dc97..c27a51a 100644
> --- a/tools/perf/arch/powerpc/util/sym-handling.c
> +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> @@ -97,6 +97,7 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
>         }
>  }
> 
> +#ifdef HAVE_LIBELF_SUPPORT
>  void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
>                                            int ntevs)
>  {
> @@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct
> perf_probe_event *pev,
>                 }
>         }
>  }
> +#endif
> 
>  #endif
> -- 
> 2.7.4

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-11 11:50       ` Arnaldo Carvalho de Melo
@ 2016-08-11 13:21         ` Ravi Bangoria
  2016-08-11 14:49           ` Arnaldo Carvalho de Melo
  2016-08-12  6:06         ` Anton Blanchard
  1 sibling, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2016-08-11 13:21 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Anton Blanchard, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung,
	Ravi Bangoria, Hemant Kumar



On Thursday 11 August 2016 05:20 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 11, 2016 at 10:01:04AM +0530, Ravi Bangoria escreveu:
>>
>> On Thursday 11 August 2016 05:24 AM, Anton Blanchard wrote:
>>> Hi,
>>>
>>>> Powerpc has Global Entry Point and Local Entry Point for functions.
>>>> LEP catches call from both the GEP and the LEP. Symbol table of ELF
>>>> contains GEP and Offset from which we can calculate LEP, but debuginfo
>>>> does not have LEP info.
>>>>
>>>> Currently, perf prioritize symbol table over dwarf to probe on LEP
>>>> for ppc64le. But when user tries to probe with function parameter,
>>>> we fall back to using dwarf(i.e. GEP) and when function called via
>>>> LEP, probe will never hit.
>>> This patch causes a build failure for me on ppc64le:
>>>
>>> libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':
>>>
>>> tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'
>> Thanks Anton. Sorry, I should have caught that.
>>
>> @Arnaldo, Can you please pick this up. I've prepared this on top of
>> acme/perf/core.
>>
>>
>>  From 89c977ae9c3ae35c78b16cddabcf2b01d3cf5cc8 Mon Sep 17 00:00:00 2001
>> From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> Date: Wed, 10 Aug 2016 23:13:45 -0500
>> Subject: [PATCH] perf ppc64le: Fix build failure when no dwarf support
>>
>> Fix perf build failure on ppc64le because of Commit 99e608b5954c ("perf
>> probe ppc64le: Fix probe location when using DWARF")
> Can you please provide a better explanation? I had to look at the patch
> to understand what it was fixing, and then the patch adds LIBELF_SUPPORT
> ifdefs while the patch description, talks about DWARF.

Yes. Explanation could have been better. Apologies for that.

arch__post_process_probe_trace_events() calls get_target_map() to prepare
symbol table. get_target_map() is defined inside util/probe-event.c.

probe-event.c will only get included in perf binary if CONFIG_LIBELF is set.
Hence arch__post_process_probe_trace_events() needs to be defined inside
#ifdef HAVE_LIBELF_SUPPORT to solve compilation error.

Please let me know if any doubts.

Thanks,
Ravi

> Anyway, Anton, does this fix the problem for you?
>
> - Arnaldo
>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> ---
>>   tools/perf/arch/powerpc/util/sym-handling.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/perf/arch/powerpc/util/sym-handling.c
>> b/tools/perf/arch/powerpc/util/sym-handling.c
>> index 8d4dc97..c27a51a 100644
>> --- a/tools/perf/arch/powerpc/util/sym-handling.c
>> +++ b/tools/perf/arch/powerpc/util/sym-handling.c
>> @@ -97,6 +97,7 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
>>          }
>>   }
>>
>> +#ifdef HAVE_LIBELF_SUPPORT
>>   void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
>>                                             int ntevs)
>>   {
>> @@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct
>> perf_probe_event *pev,
>>                  }
>>          }
>>   }
>> +#endif
>>
>>   #endif
>> -- 
>> 2.7.4

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-11 13:21         ` Ravi Bangoria
@ 2016-08-11 14:49           ` Arnaldo Carvalho de Melo
  2016-08-11 17:40             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-11 14:49 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: Anton Blanchard, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung,
	Hemant Kumar

Em Thu, Aug 11, 2016 at 06:51:39PM +0530, Ravi Bangoria escreveu:
> 
> 
> On Thursday 11 August 2016 05:20 PM, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Aug 11, 2016 at 10:01:04AM +0530, Ravi Bangoria escreveu:
> > > 
> > > On Thursday 11 August 2016 05:24 AM, Anton Blanchard wrote:
> > > > Hi,
> > > > 
> > > > > Powerpc has Global Entry Point and Local Entry Point for functions.
> > > > > LEP catches call from both the GEP and the LEP. Symbol table of ELF
> > > > > contains GEP and Offset from which we can calculate LEP, but debuginfo
> > > > > does not have LEP info.
> > > > > 
> > > > > Currently, perf prioritize symbol table over dwarf to probe on LEP
> > > > > for ppc64le. But when user tries to probe with function parameter,
> > > > > we fall back to using dwarf(i.e. GEP) and when function called via
> > > > > LEP, probe will never hit.
> > > > This patch causes a build failure for me on ppc64le:
> > > > 
> > > > libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':
> > > > 
> > > > tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'
> > > Thanks Anton. Sorry, I should have caught that.
> > > 
> > > @Arnaldo, Can you please pick this up. I've prepared this on top of
> > > acme/perf/core.
> > > 
> > > 
> > >  From 89c977ae9c3ae35c78b16cddabcf2b01d3cf5cc8 Mon Sep 17 00:00:00 2001
> > > From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> > > Date: Wed, 10 Aug 2016 23:13:45 -0500
> > > Subject: [PATCH] perf ppc64le: Fix build failure when no dwarf support
> > > 
> > > Fix perf build failure on ppc64le because of Commit 99e608b5954c ("perf
> > > probe ppc64le: Fix probe location when using DWARF")
> > Can you please provide a better explanation? I had to look at the patch
> > to understand what it was fixing, and then the patch adds LIBELF_SUPPORT
> > ifdefs while the patch description, talks about DWARF.
> 
> Yes. Explanation could have been better. Apologies for that.
> 
> arch__post_process_probe_trace_events() calls get_target_map() to prepare
> symbol table. get_target_map() is defined inside util/probe-event.c.
> 
> probe-event.c will only get included in perf binary if CONFIG_LIBELF is set.
> Hence arch__post_process_probe_trace_events() needs to be defined inside
> #ifdef HAVE_LIBELF_SUPPORT to solve compilation error.
> 
> Please let me know if any doubts.

Thanks, that is better, will add this there.

- Arnaldo

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-10 23:54   ` [PATCH 2/2] perf " Anton Blanchard
  2016-08-11  4:31     ` Ravi Bangoria
@ 2016-08-11 15:49     ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-11 15:49 UTC (permalink / raw)
  To: Anton Blanchard
  Cc: Ravi Bangoria, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung

Em Thu, Aug 11, 2016 at 09:54:57AM +1000, Anton Blanchard escreveu:
> Hi,
> 
> > Powerpc has Global Entry Point and Local Entry Point for functions.
> > LEP catches call from both the GEP and the LEP. Symbol table of ELF
> > contains GEP and Offset from which we can calculate LEP, but debuginfo
> > does not have LEP info.
> > 
> > Currently, perf prioritize symbol table over dwarf to probe on LEP
> > for ppc64le. But when user tries to probe with function parameter,
> > we fall back to using dwarf(i.e. GEP) and when function called via
> > LEP, probe will never hit.
> 
> This patch causes a build failure for me on ppc64le:
> 
> libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':
> 
> tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'

Humm, he sent a patch, I'm applying it, but I'm trying to figure out why
this wasn't catched by my cross compiling container :-\

Complete log and:

  CC       /tmp/build/perf/util/probe-event.o

Because:

...                        libelf: [ on  ]

So I would have to build with NO_LIBELF=1? /me tries... further proof I need to
build with 'make -C tools/perf build-test' for all the arches, that will test, among
other combos, with NO_LIBELF=1 and without :-\

# docker run --entrypoint=/bin/bash -v /home/acme/git:/git:Z --rm -ti docker.io/acmel/linux-perf-tools-build-ubuntu:16.04-x-powerpc64el
perfbuilder@74bca30d5b61:/$ make NO_LIBELF=1 ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-gnu- -C /git/linux/tools/perf O=/tmp/build/perf
<SNIP>
  LINK     /tmp/build/perf/perf
/tmp/build/perf/libperf.a(libperf-in.o): In function `arch__post_process_probe_trace_events':
/git/linux/tools/perf/arch/powerpc/util/sym-handling.c:109: undefined reference to `get_target_map'
collect2: error: ld returned 1 exit status
Makefile.perf:435: recipe for target '/tmp/build/perf/perf' failed
make[1]: *** [/tmp/build/perf/perf] Error 1
Makefile:68: recipe for target 'all' failed
make: *** [all] Error 2
make: Leaving directory '/git/linux/tools/perf'
perfbuilder@74bca30d5b61:/$

Ok, reproduced, will add the NO_LIBELF=1 to the regular set of tests for ppc64le.

- Arnaldo

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-11 14:49           ` Arnaldo Carvalho de Melo
@ 2016-08-11 17:40             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-11 17:40 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: Anton Blanchard, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung,
	Hemant Kumar

Em Thu, Aug 11, 2016 at 11:49:00AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Aug 11, 2016 at 06:51:39PM +0530, Ravi Bangoria escreveu:
> > On Thursday 11 August 2016 05:20 PM, Arnaldo Carvalho de Melo wrote:
> > > Can you please provide a better explanation? I had to look at the patch
> > > to understand what it was fixing, and then the patch adds LIBELF_SUPPORT
> > > ifdefs while the patch description, talks about DWARF.

> > Yes. Explanation could have been better. Apologies for that.

> > arch__post_process_probe_trace_events() calls get_target_map() to prepare
> > symbol table. get_target_map() is defined inside util/probe-event.c.

> > probe-event.c will only get included in perf binary if CONFIG_LIBELF is set.
> > Hence arch__post_process_probe_trace_events() needs to be defined inside
> > #ifdef HAVE_LIBELF_SUPPORT to solve compilation error.
 
> > Please let me know if any doubts.
 
> Thanks, that is better, will add this there.

But the patch his mangled, you use:

User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
        Thunderbird/38.5.1

Please read Documentation/email-clients.txt, there are ways to fix the
sending of patches.

I fixed it up this time, there were two instances like:

[acme@jouet linux]$ patch -p1 < /wb/1.patch 
patching file tools/perf/arch/powerpc/util/sym-handling.c
patch: **** malformed patch at line 33: @@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct 

it breaks down long lines ;-\

[acme@jouet linux]$

- Arnaldo

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-11 11:50       ` Arnaldo Carvalho de Melo
  2016-08-11 13:21         ` Ravi Bangoria
@ 2016-08-12  6:06         ` Anton Blanchard
  2016-08-12 13:01           ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 16+ messages in thread
From: Anton Blanchard @ 2016-08-12  6:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ravi Bangoria, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung

Hi Arnaldo,

> Anyway, Anton, does this fix the problem for you?

Yes it does, thanks!

Anton

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

* Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
  2016-08-12  6:06         ` Anton Blanchard
@ 2016-08-12 13:01           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-12 13:01 UTC (permalink / raw)
  To: Anton Blanchard
  Cc: Ravi Bangoria, linux-kernel, peterz, mingo, alexander.shishkin,
	bsingharora, naveen.n.rao, ananth, mhiramat, wangnan0, namhyung

Em Fri, Aug 12, 2016 at 04:06:55PM +1000, Anton Blanchard escreveu:
> Hi Arnaldo,
> 
> > Anyway, Anton, does this fix the problem for you?
> 
> Yes it does, thanks!

Thanks, adding a Tested-by: you.

- Arnaldo

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

* [tip:perf/urgent] perf ppc64le: Fix build failure when libelf is not present
  2016-08-11  4:31     ` Ravi Bangoria
  2016-08-11 11:50       ` Arnaldo Carvalho de Melo
@ 2016-08-16 18:14       ` tip-bot for Ravi Bangoria
  1 sibling, 0 replies; 16+ messages in thread
From: tip-bot for Ravi Bangoria @ 2016-08-16 18:14 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bsingharora, peterz, hpa, ravi.bangoria, alexander.shishkin,
	acme, wangnan0, anton, namhyung, mhiramat, naveen.n.rao, ananth,
	tglx, linux-kernel, mingo

Commit-ID:  f046f3df665361d2f89e660f8c79ba164069f02a
Gitweb:     http://git.kernel.org/tip/f046f3df665361d2f89e660f8c79ba164069f02a
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Thu, 11 Aug 2016 14:43:59 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Aug 2016 14:39:48 -0300

perf ppc64le: Fix build failure when libelf is not present

arch__post_process_probe_trace_events() calls get_target_map() to
prepare symbol table. get_target_map() is defined inside
util/probe-event.c.

probe-event.c will only get included in perf binary if CONFIG_LIBELF is
set.  Hence arch__post_process_probe_trace_events() needs to be defined
inside #ifdef HAVE_LIBELF_SUPPORT to solve compilation error.

Reported-and-Tested-by: Anton Blanchard <anton@samba.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/57ABFF88.8030905@linux.vnet.ibm.com
[ Thunderbird MUA mangled it, fix that ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 8d4dc97..35745a7 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -97,6 +97,7 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 	}
 }
 
+#ifdef HAVE_LIBELF_SUPPORT
 void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
 					   int ntevs)
 {
@@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
 		}
 	}
 }
+#endif /* HAVE_LIBELF_SUPPORT */
 
 #endif

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

end of thread, other threads:[~2016-08-16 18:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09  6:23 [PATCH 1/2] perf: Add function to post process kernel trace events Ravi Bangoria
2016-08-09  6:23 ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Ravi Bangoria
2016-08-09 14:02   ` Masami Hiramatsu
2016-08-09 15:09     ` Arnaldo Carvalho de Melo
2016-08-09 19:21   ` [tip:perf/urgent] perf probe " tip-bot for Ravi Bangoria
2016-08-10 23:54   ` [PATCH 2/2] perf " Anton Blanchard
2016-08-11  4:31     ` Ravi Bangoria
2016-08-11 11:50       ` Arnaldo Carvalho de Melo
2016-08-11 13:21         ` Ravi Bangoria
2016-08-11 14:49           ` Arnaldo Carvalho de Melo
2016-08-11 17:40             ` Arnaldo Carvalho de Melo
2016-08-12  6:06         ` Anton Blanchard
2016-08-12 13:01           ` Arnaldo Carvalho de Melo
2016-08-16 18:14       ` [tip:perf/urgent] perf ppc64le: Fix build failure when libelf is not present tip-bot for Ravi Bangoria
2016-08-11 15:49     ` [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Arnaldo Carvalho de Melo
2016-08-09 19:20 ` [tip:perf/urgent] perf probe: Add function to post process kernel trace events tip-bot for Ravi Bangoria

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).