[PATCH v3 0/7] Fixes for perf probe issues on ppc

[PATCH v3 0/7] Fixes for perf probe issues on ppc

[Naveen N. Rao]

This patchset fixes various issues with perf probe on powerpc across ABIv1 and
ABIv2:
- in the presence of DWARF debug-info,
- in the absence of DWARF, but with the symbol table, and
- in the absence of debug-info, but with kallsyms.

Arnaldo,
I have moved all patches to use __weak functions. Kindly take a look and let me
know if this is what you had in mind.

Thanks!
- Naveen

----
Changes since v2:
The primary change is with the use of __weak functions instead of compiler
macros. Along with that, I have also addressed all previous review comments
(*). Individual patches have the detailed changelog. Please note that I have
dropped the first (kernel) patch from the last series since it has been pushed
upstream. This series now only includes the rest of the patches.

(*) http://thread.gmane.org/gmane.linux.kernel/1851128



Naveen N. Rao (7):
  perf probe: Improve detection of file/function name in the probe
    pattern
  perf probe/ppc: Fix symbol fixup issues due to ELF type
  perf probe/ppc: Use the right prefix when ignoring SyS symbols on ppc
  perf probe/ppc: Enable matching against dot symbols automatically
  perf probe/ppc64le: Fix ppc64 ABIv2 symbol decoding
  perf probe/ppc64le: Prefer symbol table lookup over DWARF
  perf probe/ppc64le: Fixup function entry if using kallsyms lookup

 tools/perf/arch/powerpc/util/Build          |  1 +
 tools/perf/arch/powerpc/util/sym-handling.c | 80 +++++++++++++++++++++++++++++
 tools/perf/util/map.c                       |  5 ++
 tools/perf/util/map.h                       |  3 +-
 tools/perf/util/probe-event.c               | 42 +++++++++++++--
 tools/perf/util/probe-event.h               |  3 ++
 tools/perf/util/symbol-elf.c                | 12 ++++-
 tools/perf/util/symbol.c                    | 25 +++++----
 tools/perf/util/symbol.h                    | 10 ++++
 9 files changed, 164 insertions(+), 17 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c

-- 
2.3.5

[PATCH v3 1/7] perf probe: Improve detection of file/function name in the probe pattern

[Naveen N. Rao]

Currently, perf probe considers patterns including a '.' to be a file.
However, this causes problems on powerpc ABIv1 where all functions have
a leading '.':

  $ perf probe -F | grep schedule_timeout_interruptible
  .schedule_timeout_interruptible
  $ perf probe .schedule_timeout_interruptible
  Semantic error :File always requires line number or lazy pattern.
    Error: Command Parse Error.

Fix this:
- by checking the probe pattern in more detail, and
- skipping leading dot if one exists when creating/deleting events.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
- Folded initial part of patch 04 [now 03] into this per Masami.
- Moved to using a bool file_spec and some whitespace changes.


 tools/perf/util/probe-event.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index d8bb616..a8c19d5 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1077,6 +1077,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	struct perf_probe_point *pp = &pev->point;
 	char *ptr, *tmp;
 	char c, nc = 0;
+	bool file_spec = false;
 	/*
 	 * <Syntax>
 	 * perf probe [EVENT=]SRC[:LN|;PTN]
@@ -1105,6 +1106,23 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 		arg = tmp;
 	}
 
+	/*
+	 * Check arg is function or file name and copy it.
+	 *
+	 * We consider arg to be a file spec if and only if it satisfies
+	 * all of the below criteria::
+	 * - it does not include any of "+@%",
+	 * - it includes one of ":;", and
+	 * - it has a period '.' in the name.
+	 *
+	 * Otherwise, we consider arg to be a function specification.
+	 */
+	if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
+		/* This is a file spec if it includes a '.' before ; or : */
+		if (memchr(arg, '.', ptr - arg))
+			file_spec = true;
+	}
+
 	ptr = strpbrk(arg, ";:+@%");
 	if (ptr) {
 		nc = *ptr;
@@ -1115,10 +1133,9 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	if (tmp == NULL)
 		return -ENOMEM;
 
-	/* Check arg is function or file and copy it */
-	if (strchr(tmp, '.'))	/* File */
+	if (file_spec)
 		pp->file = tmp;
-	else			/* Function */
+	else
 		pp->function = tmp;
 
 	/* Parse other options */
@@ -2265,6 +2282,9 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 {
 	int i, ret;
 
+	if (*base == '.')
+		base++;
+
 	/* Try no suffix */
 	ret = e_snprintf(buf, len, "%s", base);
 	if (ret < 0) {
@@ -2751,6 +2771,9 @@ int del_perf_probe_events(struct strlist *dellist)
 			event = str;
 		}
 
+		if (event && *event == '.')
+			event++;
+
 		ret = e_snprintf(buf, 128, "%s:%s", group, event);
 		if (ret < 0) {
 			pr_err("Failed to copy event.");
-- 
2.3.5

Re: [PATCH v3 1/7] perf probe: Improve detection of file/function name in the probe pattern

[Arnaldo Carvalho de Melo]

Em Tue, Apr 28, 2015 at 05:35:34PM +0530, Naveen N. Rao escreveu:
> Currently, perf probe considers patterns including a '.' to be a file.
> However, this causes problems on powerpc ABIv1 where all functions have
> a leading '.':
> 
>   $ perf probe -F | grep schedule_timeout_interruptible
>   .schedule_timeout_interruptible
>   $ perf probe .schedule_timeout_interruptible
>   Semantic error :File always requires line number or lazy pattern.
>     Error: Command Parse Error.

Just gave this a real quick look, saw no problems, but I'll defer this
to Masami, a reviewed-by tag from him, ok Masami?

- Arnaldo
 
> Fix this:
> - by checking the probe pattern in more detail, and
> - skipping leading dot if one exists when creating/deleting events.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> Changes:
> - Folded initial part of patch 04 [now 03] into this per Masami.
> - Moved to using a bool file_spec and some whitespace changes.
> 
> 
>  tools/perf/util/probe-event.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index d8bb616..a8c19d5 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1077,6 +1077,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	struct perf_probe_point *pp = &pev->point;
>  	char *ptr, *tmp;
>  	char c, nc = 0;
> +	bool file_spec = false;
>  	/*
>  	 * <Syntax>
>  	 * perf probe [EVENT=]SRC[:LN|;PTN]
> @@ -1105,6 +1106,23 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		arg = tmp;
>  	}
>  
> +	/*
> +	 * Check arg is function or file name and copy it.
> +	 *
> +	 * We consider arg to be a file spec if and only if it satisfies
> +	 * all of the below criteria::
> +	 * - it does not include any of "+@%",
> +	 * - it includes one of ":;", and
> +	 * - it has a period '.' in the name.
> +	 *
> +	 * Otherwise, we consider arg to be a function specification.
> +	 */
> +	if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
> +		/* This is a file spec if it includes a '.' before ; or : */
> +		if (memchr(arg, '.', ptr - arg))
> +			file_spec = true;
> +	}
> +
>  	ptr = strpbrk(arg, ";:+@%");
>  	if (ptr) {
>  		nc = *ptr;
> @@ -1115,10 +1133,9 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	if (tmp == NULL)
>  		return -ENOMEM;
>  
> -	/* Check arg is function or file and copy it */
> -	if (strchr(tmp, '.'))	/* File */
> +	if (file_spec)
>  		pp->file = tmp;
> -	else			/* Function */
> +	else
>  		pp->function = tmp;
>  
>  	/* Parse other options */
> @@ -2265,6 +2282,9 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
>  {
>  	int i, ret;
>  
> +	if (*base == '.')
> +		base++;
> +
>  	/* Try no suffix */
>  	ret = e_snprintf(buf, len, "%s", base);
>  	if (ret < 0) {
> @@ -2751,6 +2771,9 @@ int del_perf_probe_events(struct strlist *dellist)
>  			event = str;
>  		}
>  
> +		if (event && *event == '.')
> +			event++;
> +
>  		ret = e_snprintf(buf, 128, "%s:%s", group, event);
>  		if (ret < 0) {
>  			pr_err("Failed to copy event.");
> -- 
> 2.3.5

[PATCH v3 2/7] perf probe/ppc: Fix symbol fixup issues due to ELF type

[Naveen N. Rao]

If using the symbol table, symbol addresses are not being fixed up
properly, resulting in probes being placed at wrong addresses:

  # perf probe do_fork
  Added new event:
    probe:do_fork        (on do_fork)

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

	  perf record -e probe:do_fork -aR sleep 1

  # cat /sys/kernel/debug/tracing/kprobe_events
  p:probe/do_fork _text+635952
  # printf "%x" 635952
  9b430
  # grep do_fork /boot/System.map
  c0000000000ab430 T .do_fork

Fix by checking for ELF type ET_DYN used by ppc64 kernels.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
- Introduce arch helper to limit change to ppc.

 tools/perf/arch/powerpc/util/Build          |  1 +
 tools/perf/arch/powerpc/util/sym-handling.c | 19 +++++++++++++++++++
 tools/perf/util/symbol-elf.c                |  8 ++++++--
 tools/perf/util/symbol.h                    |  4 ++++
 4 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c

diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
index 0af6e9b..7b8b0d1 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -1,4 +1,5 @@
 libperf-y += header.o
+libperf-y += sym-handling.o
 
 libperf-$(CONFIG_DWARF) += dwarf-regs.o
 libperf-$(CONFIG_DWARF) += skip-callchain-idx.o
diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
new file mode 100644
index 0000000..c9de001
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -0,0 +1,19 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * Copyright (C) 2015 Naveen N. Rao, IBM Corporation
+ */
+
+#include "debug.h"
+#include "symbol.h"
+
+#ifdef HAVE_LIBELF_SUPPORT
+bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
+{
+	return ehdr.e_type == ET_EXEC ||
+	       ehdr.e_type == ET_REL ||
+	       ehdr.e_type == ET_DYN;
+}
+#endif
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index ac09c60..7f5d237 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -630,6 +630,11 @@ void symsrc__destroy(struct symsrc *ss)
 	close(ss->fd);
 }
 
+bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
+{
+	return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
+}
+
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 		 enum dso_binary_type type)
 {
@@ -712,8 +717,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 						     ".gnu.prelink_undo",
 						     NULL) != NULL);
 	} else {
-		ss->adjust_symbols = ehdr.e_type == ET_EXEC ||
-				     ehdr.e_type == ET_REL;
+		ss->adjust_symbols = elf__needs_adjust_symbols(ehdr);
 	}
 
 	ss->name   = strdup(name);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 0956150..8cb0af4 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -303,4 +303,8 @@ int setup_list(struct strlist **list, const char *list_str,
 int setup_intlist(struct intlist **list, const char *list_str,
 		  const char *list_name);
 
+#ifdef HAVE_LIBELF_SUPPORT
+bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
+#endif
+
 #endif /* __PERF_SYMBOL */
-- 
2.3.5

Re: [PATCH v3 2/7] perf probe/ppc: Fix symbol fixup issues due to ELF type

[Arnaldo Carvalho de Melo]

Em Tue, Apr 28, 2015 at 05:35:35PM +0530, Naveen N. Rao escreveu:
> If using the symbol table, symbol addresses are not being fixed up
> properly, resulting in probes being placed at wrong addresses:
> 
>   # perf probe do_fork
>   Added new event:
>     probe:do_fork        (on do_fork)
> 
>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe:do_fork -aR sleep 1
> 
>   # cat /sys/kernel/debug/tracing/kprobe_events
>   p:probe/do_fork _text+635952
>   # printf "%x" 635952
>   9b430
>   # grep do_fork /boot/System.map
>   c0000000000ab430 T .do_fork
> 
> Fix by checking for ELF type ET_DYN used by ppc64 kernels.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

This one looks great, Are you keeping the Reviewed by from a previous
patch? Or has Srikar reissued it? Srikar?

- Arnaldo
> ---
> Changes:
> - Introduce arch helper to limit change to ppc.
> 
>  tools/perf/arch/powerpc/util/Build          |  1 +
>  tools/perf/arch/powerpc/util/sym-handling.c | 19 +++++++++++++++++++
>  tools/perf/util/symbol-elf.c                |  8 ++++++--
>  tools/perf/util/symbol.h                    |  4 ++++
>  4 files changed, 30 insertions(+), 2 deletions(-)
>  create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c
> 
> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
> index 0af6e9b..7b8b0d1 100644
> --- a/tools/perf/arch/powerpc/util/Build
> +++ b/tools/perf/arch/powerpc/util/Build
> @@ -1,4 +1,5 @@
>  libperf-y += header.o
> +libperf-y += sym-handling.o
>  
>  libperf-$(CONFIG_DWARF) += dwarf-regs.o
>  libperf-$(CONFIG_DWARF) += skip-callchain-idx.o
> diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
> new file mode 100644
> index 0000000..c9de001
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> @@ -0,0 +1,19 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * Copyright (C) 2015 Naveen N. Rao, IBM Corporation
> + */
> +
> +#include "debug.h"
> +#include "symbol.h"
> +
> +#ifdef HAVE_LIBELF_SUPPORT
> +bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
> +{
> +	return ehdr.e_type == ET_EXEC ||
> +	       ehdr.e_type == ET_REL ||
> +	       ehdr.e_type == ET_DYN;
> +}
> +#endif
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index ac09c60..7f5d237 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -630,6 +630,11 @@ void symsrc__destroy(struct symsrc *ss)
>  	close(ss->fd);
>  }
>  
> +bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
> +{
> +	return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
> +}
> +
>  int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>  		 enum dso_binary_type type)
>  {
> @@ -712,8 +717,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>  						     ".gnu.prelink_undo",
>  						     NULL) != NULL);
>  	} else {
> -		ss->adjust_symbols = ehdr.e_type == ET_EXEC ||
> -				     ehdr.e_type == ET_REL;
> +		ss->adjust_symbols = elf__needs_adjust_symbols(ehdr);
>  	}
>  
>  	ss->name   = strdup(name);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 0956150..8cb0af4 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -303,4 +303,8 @@ int setup_list(struct strlist **list, const char *list_str,
>  int setup_intlist(struct intlist **list, const char *list_str,
>  		  const char *list_name);
>  
> +#ifdef HAVE_LIBELF_SUPPORT
> +bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
> +#endif
> +
>  #endif /* __PERF_SYMBOL */
> -- 
> 2.3.5

[PATCH v3 3/7] perf probe/ppc: Use the right prefix when ignoring SyS symbols on ppc

[Naveen N. Rao]

Use the proper prefix when ignoring SyS symbols on ppc ABIv1. While at
it, generalize symbol selection so architectures can implement their
own logic.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
- Move logic to arch helper.

 tools/perf/arch/powerpc/util/sym-handling.c | 20 ++++++++++++++++++++
 tools/perf/util/symbol.c                    | 21 ++++++++++++---------
 tools/perf/util/symbol.h                    |  5 +++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index c9de001..5522a40 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -17,3 +17,23 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
 	       ehdr.e_type == ET_DYN;
 }
 #endif
+
+#if !defined(_CALL_ELF) || _CALL_ELF != 2
+int arch__choose_best_symbol(struct symbol *syma,
+			     struct symbol *symb __maybe_unused)
+{
+	char *sym = syma->name;
+
+	/* Skip over any initial dot */
+	if (*sym == '.')
+		sym++;
+
+	/* Avoid "SyS" kernel syscall aliases */
+	if (strlen(sym) >= 3 && !strncmp(sym, "SyS", 3))
+		return SYMBOL_B;
+	if (strlen(sym) >= 10 && !strncmp(sym, "compat_SyS", 10))
+		return SYMBOL_B;
+
+	return SYMBOL_A;
+}
+#endif
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 201f6c4c..f805757 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -85,8 +85,17 @@ static int prefix_underscores_count(const char *str)
 	return tail - str;
 }
 
-#define SYMBOL_A 0
-#define SYMBOL_B 1
+int __weak arch__choose_best_symbol(struct symbol *syma,
+				    struct symbol *symb __maybe_unused)
+{
+	/* Avoid "SyS" kernel syscall aliases */
+	if (strlen(syma->name) >= 3 && !strncmp(syma->name, "SyS", 3))
+		return SYMBOL_B;
+	if (strlen(syma->name) >= 10 && !strncmp(syma->name, "compat_SyS", 10))
+		return SYMBOL_B;
+
+	return SYMBOL_A;
+}
 
 static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
 {
@@ -134,13 +143,7 @@ static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
 	else if (na < nb)
 		return SYMBOL_B;
 
-	/* Avoid "SyS" kernel syscall aliases */
-	if (na >= 3 && !strncmp(syma->name, "SyS", 3))
-		return SYMBOL_B;
-	if (na >= 10 && !strncmp(syma->name, "compat_SyS", 10))
-		return SYMBOL_B;
-
-	return SYMBOL_A;
+	return arch__choose_best_symbol(syma, symb);
 }
 
 void symbols__fixup_duplicate(struct rb_root *symbols)
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 8cb0af4..bd50ba0 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -307,4 +307,9 @@ int setup_intlist(struct intlist **list, const char *list_str,
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
 #endif
 
+#define SYMBOL_A 0
+#define SYMBOL_B 1
+
+int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
+
 #endif /* __PERF_SYMBOL */
-- 
2.3.5

[PATCH v3 4/7] perf probe/ppc: Enable matching against dot symbols automatically

[Naveen N. Rao]

Allow perf probe to work on ppc ABIv1 without the need to specify the
leading dot '.' for functions. 'perf probe do_fork' works with this
patch.

We do this by changing how symbol name comparison works on ppc ABIv1 -
we simply ignore and skip over the initial dot, if one exists, during
symbol name comparison.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
This patch has been redone since upstream changed quite a bit. We now implement
arch-specific helper during symbol name comparison to ignore leading dot on ppc
symbols.

 tools/perf/arch/powerpc/util/sym-handling.c | 13 +++++++++++++
 tools/perf/util/map.c                       |  5 +++++
 tools/perf/util/map.h                       |  3 ++-
 tools/perf/util/symbol.c                    |  4 ++--
 4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 5522a40..2de2cc4 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -8,6 +8,7 @@
 
 #include "debug.h"
 #include "symbol.h"
+#include "map.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -36,4 +37,16 @@ int arch__choose_best_symbol(struct symbol *syma,
 
 	return SYMBOL_A;
 }
+
+/* Allow matching against dot variants */
+int arch__compare_symbol_names(const char *namea, const char *nameb)
+{
+	/* Skip over initial dot */
+	if (*namea == '.')
+		namea++;
+	if (*nameb == '.')
+		nameb++;
+
+	return strcmp(namea, nameb);
+}
 #endif
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index a14f08f..cd0e335 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -292,6 +292,11 @@ int map__load(struct map *map, symbol_filter_t filter)
 	return 0;
 }
 
+int __weak arch__compare_symbol_names(const char *namea, const char *nameb)
+{
+	return strcmp(namea, nameb);
+}
+
 struct symbol *map__find_symbol(struct map *map, u64 addr,
 				symbol_filter_t filter)
 {
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index ec19c59..4e0c729 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -124,7 +124,7 @@ struct thread;
  */
 #define __map__for_each_symbol_by_name(map, sym_name, pos, filter)	\
 	for (pos = map__find_symbol_by_name(map, sym_name, filter);	\
-	     pos && strcmp(pos->name, sym_name) == 0;		\
+	     pos && arch__compare_symbol_names(pos->name, sym_name) == 0;	\
 	     pos = symbol__next_by_name(pos))
 
 #define map__for_each_symbol_by_name(map, sym_name, pos)		\
@@ -132,6 +132,7 @@ struct thread;
 
 typedef int (*symbol_filter_t)(struct map *map, struct symbol *sym);
 
+int arch__compare_symbol_names(const char *namea, const char *nameb);
 void map__init(struct map *map, enum map_type type,
 	       u64 start, u64 end, u64 pgoff, struct dso *dso);
 struct map *map__new(struct machine *machine, u64 start, u64 len,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f805757..45ba48a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -411,7 +411,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 		int cmp;
 
 		s = rb_entry(n, struct symbol_name_rb_node, rb_node);
-		cmp = strcmp(name, s->sym.name);
+		cmp = arch__compare_symbol_names(name, s->sym.name);
 
 		if (cmp < 0)
 			n = n->rb_left;
@@ -429,7 +429,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 		struct symbol_name_rb_node *tmp;
 
 		tmp = rb_entry(n, struct symbol_name_rb_node, rb_node);
-		if (strcmp(tmp->sym.name, s->sym.name))
+		if (arch__compare_symbol_names(tmp->sym.name, s->sym.name))
 			break;
 
 		s = tmp;
-- 
2.3.5

[PATCH v3 5/7] perf probe/ppc64le: Fix ppc64 ABIv2 symbol decoding

[Naveen N. Rao]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

ppc64 ELF ABIv2 has a Global Entry Point (GEP) and a Local Entry Point
(LEP). For purposes of probing, we need the LEP - the offset to which is
encoded in st_other.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
- No changes other than move to use __weak functions

 tools/perf/arch/powerpc/util/sym-handling.c | 7 +++++++
 tools/perf/util/symbol-elf.c                | 4 ++++
 tools/perf/util/symbol.h                    | 1 +
 3 files changed, 12 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 2de2cc4..012a0f8 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -17,6 +17,13 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
 	       ehdr.e_type == ET_REL ||
 	       ehdr.e_type == ET_DYN;
 }
+
+#if defined(_CALL_ELF) && _CALL_ELF == 2
+void arch__elf_sym_adjust(GElf_Sym *sym)
+{
+	sym->st_value += PPC64_LOCAL_ENTRY_OFFSET(sym->st_other);
+}
+#endif
 #endif
 
 #if !defined(_CALL_ELF) || _CALL_ELF != 2
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 7f5d237..9d526a5 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -776,6 +776,8 @@ static bool want_demangle(bool is_kernel_sym)
 	return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
 }
 
+void __weak arch__elf_sym_adjust(GElf_Sym *sym __maybe_unused) { }
+
 int dso__load_sym(struct dso *dso, struct map *map,
 		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
 		  symbol_filter_t filter, int kmodule)
@@ -940,6 +942,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
 		    (sym.st_value & 1))
 			--sym.st_value;
 
+		arch__elf_sym_adjust(&sym);
+
 		if (dso->kernel || kmodule) {
 			char dso_name[PATH_MAX];
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bd50ba0..9096529 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -305,6 +305,7 @@ int setup_intlist(struct intlist **list, const char *list_str,
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
+void arch__elf_sym_adjust(GElf_Sym *sym);
 #endif
 
 #define SYMBOL_A 0
-- 
2.3.5

[PATCH v3 6/7] perf probe/ppc64le: Prefer symbol table lookup over DWARF

[Naveen N. Rao]

Use symbol table lookups by default if DWARF is not necessary, since
powerpc ABIv2 encodes local entry points in the symbol table and the
function entry address in DWARF may not be appropriate for kprobes, as
described here:
https://sourceware.org/bugzilla/show_bug.cgi?id=17638

"The DWARF address ranges deliberately include the *whole* function,
both global and local entry points."
...
"If you want to set probes on a local entry point, you should look up
the symbol in the main symbol table (not DWARF), and check the st_other
bits; they will indicate whether the function has a local entry point,
and what its offset from the global entry point is.  Note that GDB does
the same when setting a breakpoint on a function entry."

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
- No changes other than move to use __weak functions

 tools/perf/arch/powerpc/util/sym-handling.c | 8 ++++++++
 tools/perf/util/probe-event.c               | 8 ++++++++
 tools/perf/util/probe-event.h               | 1 +
 3 files changed, 17 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 012a0f8..a170060 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -9,6 +9,7 @@
 #include "debug.h"
 #include "symbol.h"
 #include "map.h"
+#include "probe-event.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -57,3 +58,10 @@ int arch__compare_symbol_names(const char *namea, const char *nameb)
 	return strcmp(namea, nameb);
 }
 #endif
+
+#if defined(_CALL_ELF) && _CALL_ELF == 2
+bool arch__prefers_symtab(void)
+{
+	return true;
+}
+#endif
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a8c19d5..5e62a07 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2587,6 +2587,8 @@ err_out:
 	goto out;
 }
 
+bool __weak arch__prefers_symtab(void) { return false; }
+
 static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 					  struct probe_trace_event **tevs,
 					  int max_tevs, const char *target)
@@ -2602,6 +2604,12 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 		}
 	}
 
+	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
+		ret = find_probe_trace_events_from_map(pev, tevs, max_tevs, target);
+		if (ret > 0)
+			return ret; /* Found in symbol table */
+	}
+
 	/* Convert perf_probe_event with debuginfo */
 	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
 	if (ret != 0)
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index d6b7834..52bca4b 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -135,6 +135,7 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
 			       struct strfilter *filter, bool externs);
 extern int show_available_funcs(const char *module, struct strfilter *filter,
 				bool user);
+bool arch__prefers_symtab(void);
 
 /* Maximum index number of event-name postfix */
 #define MAX_EVENT_INDEX	1024
-- 
2.3.5

[PATCH v3 7/7] perf probe/ppc64le: Fixup function entry if using kallsyms lookup

[Naveen N. Rao]

On powerpc ABIv2, if no debug-info is found and we use kallsyms, we need
to fixup the function entry to point to the local entry point. Use
offset of 8 since current toolchains always generate 2 instructions (8
bytes).

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changes:
- No changes other than move to use __weak functions

 tools/perf/arch/powerpc/util/sym-handling.c | 15 +++++++++++++++
 tools/perf/util/probe-event.c               |  5 +++++
 tools/perf/util/probe-event.h               |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index a170060..bbc1a50 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -64,4 +64,19 @@ bool arch__prefers_symtab(void)
 {
 	return true;
 }
+
+#define PPC64LE_LEP_OFFSET	8
+
+void arch__fix_tev_from_maps(struct perf_probe_event *pev,
+			     struct probe_trace_event *tev, struct map *map)
+{
+	/*
+	 * ppc64 ABIv2 local entry point is currently always 2 instructions
+	 * (8 bytes) after the global entry point.
+	 */
+	if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
+		tev->point.address += PPC64LE_LEP_OFFSET;
+		tev->point.offset += PPC64LE_LEP_OFFSET;
+	}
+}
 #endif
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 5e62a07..afbb346 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2467,6 +2467,10 @@ static int find_probe_functions(struct map *map, char *name)
 #define strdup_or_goto(str, label)	\
 	({ char *__p = strdup(str); if (!__p) goto label; __p; })
 
+void __weak arch__fix_tev_from_maps(struct perf_probe_event *pev __maybe_unused,
+				struct probe_trace_event *tev __maybe_unused,
+				struct map *map __maybe_unused) { }
+
 /*
  * Find probe function addresses from map.
  * Return an error or the number of found probe_trace_event
@@ -2573,6 +2577,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 					strdup_or_goto(pev->args[i].type,
 							nomem_out);
 		}
+		arch__fix_tev_from_maps(pev, tev, map);
 	}
 
 out:
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 52bca4b..180f142 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -136,6 +136,8 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
 extern 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);
 
 /* Maximum index number of event-name postfix */
 #define MAX_EVENT_INDEX	1024
-- 
2.3.5

Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Arnaldo Carvalho de Melo]

Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
> This patchset fixes various issues with perf probe on powerpc across ABIv1 and
> ABIv2:
> - in the presence of DWARF debug-info,
> - in the absence of DWARF, but with the symbol table, and
> - in the absence of debug-info, but with kallsyms.
> 
> Arnaldo,
> I have moved all patches to use __weak functions. Kindly take a look and let me
> know if this is what you had in mind.

Ok, I applied all but the first, for which I am waiting for Masami's
reaction, I kept Srikar's reviewed-by for the other patches, but would
as well like to get his word that he keeps it after the __weak changes.

So, for now, I'll leave it for a while sitting in my local tree, to give
time to Masami and Srikar, ok?

- Arnaldo

Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Naveen N. Rao]

On 2015/04/28 10:54AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
> > This patchset fixes various issues with perf probe on powerpc across ABIv1 and
> > ABIv2:
> > - in the presence of DWARF debug-info,
> > - in the absence of DWARF, but with the symbol table, and
> > - in the absence of debug-info, but with kallsyms.
> > 
> > Arnaldo,
> > I have moved all patches to use __weak functions. Kindly take a look and let me
> > know if this is what you had in mind.
> 
> Ok, I applied all but the first, for which I am waiting for Masami's
> reaction, I kept Srikar's reviewed-by for the other patches, but would
> as well like to get his word that he keeps it after the __weak changes.
> 
> So, for now, I'll leave it for a while sitting in my local tree, to give
> time to Masami and Srikar, ok?

Sure. We will wait for Masami and Srikar to review/confirm, but Srikar's 
reviewed-by is for the current revision.

Thanks!
- Naveen

Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Arnaldo Carvalho de Melo]

Em Tue, Apr 28, 2015 at 09:42:17PM +0530, Naveen N. Rao escreveu:
> On 2015/04/28 10:54AM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
> > > This patchset fixes various issues with perf probe on powerpc across ABIv1 and
> > > ABIv2:
> > > - in the presence of DWARF debug-info,
> > > - in the absence of DWARF, but with the symbol table, and
> > > - in the absence of debug-info, but with kallsyms.
> > > 
> > > Arnaldo,
> > > I have moved all patches to use __weak functions. Kindly take a look and let me
> > > know if this is what you had in mind.
> > 
> > Ok, I applied all but the first, for which I am waiting for Masami's
> > reaction, I kept Srikar's reviewed-by for the other patches, but would
> > as well like to get his word that he keeps it after the __weak changes.
> > 
> > So, for now, I'll leave it for a while sitting in my local tree, to give
> > time to Masami and Srikar, ok?
> 
> Sure. We will wait for Masami and Srikar to review/confirm, but Srikar's 
> reviewed-by is for the current revision.

Well, then I can go ahead and push it, later today, as I have applied
all of the patches that Srikar has reviewed, the one I'm waiting for
Masami can go when he reacts.

- Arnaldo

Re: Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Masami Hiramatsu]

On 2015/04/28 22:54, Arnaldo Carvalho de Melo wrote:
> Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
>> This patchset fixes various issues with perf probe on powerpc across ABIv1 and
>> ABIv2:
>> - in the presence of DWARF debug-info,
>> - in the absence of DWARF, but with the symbol table, and
>> - in the absence of debug-info, but with kallsyms.
>>
>> Arnaldo,
>> I have moved all patches to use __weak functions. Kindly take a look and let me
>> know if this is what you had in mind.
> 
> Ok, I applied all but the first, for which I am waiting for Masami's
> reaction, I kept Srikar's reviewed-by for the other patches, but would
> as well like to get his word that he keeps it after the __weak changes.
> 
> So, for now, I'll leave it for a while sitting in my local tree, to give
> time to Masami and Srikar, ok?

OK, I reviewed all the patches in this series :)

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
for this series.

Thank you!

-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Naveen N. Rao]

On 2015/04/29 07:17AM, Masami Hiramatsu wrote:
> On 2015/04/28 22:54, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
> >> This patchset fixes various issues with perf probe on powerpc across ABIv1 and
> >> ABIv2:
> >> - in the presence of DWARF debug-info,
> >> - in the absence of DWARF, but with the symbol table, and
> >> - in the absence of debug-info, but with kallsyms.
> >>
> >> Arnaldo,
> >> I have moved all patches to use __weak functions. Kindly take a look and let me
> >> know if this is what you had in mind.
> > 
> > Ok, I applied all but the first, for which I am waiting for Masami's
> > reaction, I kept Srikar's reviewed-by for the other patches, but would
> > as well like to get his word that he keeps it after the __weak changes.
> > 
> > So, for now, I'll leave it for a while sitting in my local tree, to give
> > time to Masami and Srikar, ok?
> 
> OK, I reviewed all the patches in this series :)

Thanks for the review!
- Naveen

> 
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> for this series.
> 
> Thank you!
> 
> -- 
> Masami HIRAMATSU
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering
> Hitachi, Ltd., Research & Development Group
> E-mail: masami.hiramatsu.pt@hitachi.com
> 

Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Srikar Dronamraju]

* Arnaldo Carvalho de Melo <acme@kernel.org> [2015-04-28 10:54:53]:

> Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
> > This patchset fixes various issues with perf probe on powerpc across ABIv1 and
> > ABIv2:
> > - in the presence of DWARF debug-info,
> > - in the absence of DWARF, but with the symbol table, and
> > - in the absence of debug-info, but with kallsyms.
> > 
> > Arnaldo,
> > I have moved all patches to use __weak functions. Kindly take a look and let me
> > know if this is what you had in mind.
> 
> Ok, I applied all but the first, for which I am waiting for Masami's
> reaction, I kept Srikar's reviewed-by for the other patches, but would
> as well like to get his word that he keeps it after the __weak changes.
> 
> So, for now, I'll leave it for a while sitting in my local tree, to give
> time to Masami and Srikar, ok?
> 

Yes Arnaldo, I have looked at the patches after the __weak changes and
they look good to me.

> - Arnaldo
> 

-- 
Thanks and Regards
Srikar Dronamraju

Re: [PATCH v3 0/7] Fixes for perf probe issues on ppc

[Arnaldo Carvalho de Melo]

Em Wed, Apr 29, 2015 at 11:07:58AM +0530, Srikar Dronamraju escreveu:
> * Arnaldo Carvalho de Melo <acme@kernel.org> [2015-04-28 10:54:53]:
> 
> > Em Tue, Apr 28, 2015 at 05:35:33PM +0530, Naveen N. Rao escreveu:
> > > This patchset fixes various issues with perf probe on powerpc across ABIv1 and
> > > ABIv2:
> > > - in the presence of DWARF debug-info,
> > > - in the absence of DWARF, but with the symbol table, and
> > > - in the absence of debug-info, but with kallsyms.
> > > 
> > > Arnaldo,
> > > I have moved all patches to use __weak functions. Kindly take a look and let me
> > > know if this is what you had in mind.
> > 
> > Ok, I applied all but the first, for which I am waiting for Masami's
> > reaction, I kept Srikar's reviewed-by for the other patches, but would
> > as well like to get his word that he keeps it after the __weak changes.
> > 
> > So, for now, I'll leave it for a while sitting in my local tree, to give
> > time to Masami and Srikar, ok?
> > 
> 
> Yes Arnaldo, I have looked at the patches after the __weak changes and
> they look good to me.

Thanks everybody, I got it merged on my perf/core branch, that I'll
submit as soon as I see an Ingo :-)

- Arnaldo