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

Groups > linux.kernel > #1430609 > unrolled thread

[RFC 0/6] perf annotate: Enable cross arch annotate

Back to article view | Back to linux.kernel

Contents

  [RFC 0/6] perf annotate: Enable cross arch annotate Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-24 14:00 +0200
    [RFC 4/6] perf annotate: generalize handling of ret instructions Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-24 14:00 +0200
      Re: [RFC 4/6] perf annotate: generalize handling of ret instructions Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-27 19:40 +0200
      [tip:perf/core] perf annotate: Generalize handling of 'ret'  instructions "tip-bot for Naveen N. Rao" <tipbot@zytor.com> - 2016-06-29 11:50 +0200
    [RFC 5/6] perf annotate: add powerpc support Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-24 14:00 +0200
    [RFC 2/6] perf annotate: Define macro for arch names Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-24 14:00 +0200
    [RFC 1/6] perf: Remove unused hist_entry__annotate function Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-24 14:00 +0200
      Re: [RFC 1/6] perf: Remove unused hist_entry__annotate function Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-27 19:40 +0200
      [tip:perf/core] perf annotate: Remove unused hist_entry__annotate  function tip-bot for Ravi Bangoria <tipbot@zytor.com> - 2016-06-29 11:50 +0200
    [RFC 3/6] perf annotate: Enable cross arch annotate Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-24 14:00 +0200
      Re: [RFC 3/6] perf annotate: Enable cross arch annotate Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-27 19:20 +0200
        Re: [RFC 3/6] perf annotate: Enable cross arch annotate Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-06-28 13:50 +0200

#1430609 — [RFC 0/6] perf annotate: Enable cross arch annotate

Perf can currently only support code navigation (branches and calls) in
annotate when run on the same architecture where perf.data was recorded.
But cross arch annotate is not supported.

This patchset enables cross arch annotate. Currently I've used x86
and arm instructions which are already available and adding support
for powerpc as well. Adding support for other arch will be easy.

I've created this patch on top of acme/perf/core. And tested it with
x86 and powerpc only.

Example:

  Record on powerpc:
  $ ./perf record -a

  Report -> Annotate on x86:
  $ ./perf report -i perf.data.powerpc --vmlinux vmlinux.powerpc

Naveen N. Rao (2):
  perf annotate: generalize handling of ret instructions
  perf annotate: add powerpc support

Ravi Bangoria (4):
  perf: Remove unused hist_entry__annotate function
  perf annotate: Define macro for arch names
  perf annotate: Enable cross arch annotate
  perf: add more triplets

 tools/perf/arch/common.c           |  53 ++++++----
 tools/perf/arch/common.h           |  10 ++
 tools/perf/builtin-top.c           |   2 +-
 tools/perf/ui/browsers/annotate.c  |  25 ++---
 tools/perf/ui/gtk/annotate.c       |   6 +-
 tools/perf/util/annotate.c         | 195 ++++++++++++++++++++++++++++++-------
 tools/perf/util/annotate.h         |   6 +-
 tools/perf/util/evsel.c            |   7 ++
 tools/perf/util/evsel.h            |   2 +
 tools/perf/util/unwind-libunwind.c |   5 +-
 10 files changed, 240 insertions(+), 71 deletions(-)

--
2.5.5

[toc] | [next] | [standalone]

#1430610 — [RFC 4/6] perf annotate: generalize handling of ret instructions

From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>

Introduce helper to detect ret instructions and use the same in the tui.
A helper is needed since some architectures such as powerpc have more
than one return instruction.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 tools/perf/ui/browsers/annotate.c | 20 +++++++++-----------
 tools/perf/util/annotate.c        | 10 ++++++++++
 tools/perf/util/annotate.h        |  1 +
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index b65a979..288200f 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -223,16 +223,14 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 			} else if (ins__is_call(dl->ins)) {
 				ui_browser__write_graph(browser, SLSMG_RARROW_CHAR);
 				SLsmg_write_char(' ');
+			} else if (ins__is_ret(dl->ins)) {
+				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
+				SLsmg_write_char(' ');
 			} else {
 				ui_browser__write_nstring(browser, " ", 2);
 			}
 		} else {
-			if (strcmp(dl->name, "retq")) {
-				ui_browser__write_nstring(browser, " ", 2);
-			} else {
-				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
-				SLsmg_write_char(' ');
-			}
+			ui_browser__write_nstring(browser, " ", 2);
 		}
 
 		disasm_line__scnprintf(dl, bf, sizeof(bf), !annotate_browser__opts.use_offset);
@@ -843,14 +841,14 @@ show_help:
 				ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
 			else if (browser->selection->offset == -1)
 				ui_helpline__puts("Actions are only available for assembly lines.");
-			else if (!browser->selection->ins) {
-				if (strcmp(browser->selection->name, "retq"))
-					goto show_sup_ins;
+			else if (!browser->selection->ins)
+				goto show_sup_ins;
+			else if (ins__is_ret(browser->selection->ins))
 				goto out;
-			} else if (!(annotate_browser__jump(browser) ||
+			else if (!(annotate_browser__jump(browser) ||
 				     annotate_browser__callq(browser, evsel, hbt))) {
 show_sup_ins:
-				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
+				ui_helpline__puts("Actions are only available for function call/return & jump/branch instructions.");
 			}
 			continue;
 		case 't':
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index e0dc7b2..634daf5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -363,6 +363,15 @@ static struct ins_ops nop_ops = {
 	.scnprintf = nop__scnprintf,
 };
 
+static struct ins_ops ret_ops = {
+	.scnprintf = ins__raw_scnprintf,
+};
+
+bool ins__is_ret(const struct ins *ins)
+{
+	return ins->ops == &ret_ops;
+}
+
 static struct ins instructions_x86[] = {
 	{ .name = "add",   .ops  = &mov_ops, },
 	{ .name = "addl",  .ops  = &mov_ops, },
@@ -439,6 +448,7 @@ static struct ins instructions_x86[] = {
 	{ .name = "xadd",  .ops  = &mov_ops, },
 	{ .name = "xbeginl", .ops  = &jump_ops, },
 	{ .name = "xbeginq", .ops  = &jump_ops, },
+	{ .name = "retq",  .ops  = &ret_ops, },
 };
 
 static struct ins instructions_arm[] = {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index f7b669e..488c427 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -48,6 +48,7 @@ struct ins {
 
 bool ins__is_jump(const struct ins *ins);
 bool ins__is_call(const struct ins *ins);
+bool ins__is_ret(const struct ins *ins);
 int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
 
 struct annotation;
-- 
2.5.5

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

#1432191 — Re: [RFC 4/6] perf annotate: generalize handling of ret instructions

Em Fri, Jun 24, 2016 at 05:23:58PM +0530, Ravi Bangoria escreveu:
> From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
> 
> Introduce helper to detect ret instructions and use the same in the tui.
> A helper is needed since some architectures such as powerpc have more
> than one return instruction.

Thanks, applied.
 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  tools/perf/ui/browsers/annotate.c | 20 +++++++++-----------
>  tools/perf/util/annotate.c        | 10 ++++++++++
>  tools/perf/util/annotate.h        |  1 +
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index b65a979..288200f 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -223,16 +223,14 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>  			} else if (ins__is_call(dl->ins)) {
>  				ui_browser__write_graph(browser, SLSMG_RARROW_CHAR);
>  				SLsmg_write_char(' ');
> +			} else if (ins__is_ret(dl->ins)) {
> +				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
> +				SLsmg_write_char(' ');
>  			} else {
>  				ui_browser__write_nstring(browser, " ", 2);
>  			}
>  		} else {
> -			if (strcmp(dl->name, "retq")) {
> -				ui_browser__write_nstring(browser, " ", 2);
> -			} else {
> -				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
> -				SLsmg_write_char(' ');
> -			}
> +			ui_browser__write_nstring(browser, " ", 2);
>  		}
>  
>  		disasm_line__scnprintf(dl, bf, sizeof(bf), !annotate_browser__opts.use_offset);
> @@ -843,14 +841,14 @@ show_help:
>  				ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
>  			else if (browser->selection->offset == -1)
>  				ui_helpline__puts("Actions are only available for assembly lines.");
> -			else if (!browser->selection->ins) {
> -				if (strcmp(browser->selection->name, "retq"))
> -					goto show_sup_ins;
> +			else if (!browser->selection->ins)
> +				goto show_sup_ins;
> +			else if (ins__is_ret(browser->selection->ins))
>  				goto out;
> -			} else if (!(annotate_browser__jump(browser) ||
> +			else if (!(annotate_browser__jump(browser) ||
>  				     annotate_browser__callq(browser, evsel, hbt))) {
>  show_sup_ins:
> -				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
> +				ui_helpline__puts("Actions are only available for function call/return & jump/branch instructions.");
>  			}
>  			continue;
>  		case 't':
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index e0dc7b2..634daf5 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -363,6 +363,15 @@ static struct ins_ops nop_ops = {
>  	.scnprintf = nop__scnprintf,
>  };
>  
> +static struct ins_ops ret_ops = {
> +	.scnprintf = ins__raw_scnprintf,
> +};
> +
> +bool ins__is_ret(const struct ins *ins)
> +{
> +	return ins->ops == &ret_ops;
> +}
> +
>  static struct ins instructions_x86[] = {
>  	{ .name = "add",   .ops  = &mov_ops, },
>  	{ .name = "addl",  .ops  = &mov_ops, },
> @@ -439,6 +448,7 @@ static struct ins instructions_x86[] = {
>  	{ .name = "xadd",  .ops  = &mov_ops, },
>  	{ .name = "xbeginl", .ops  = &jump_ops, },
>  	{ .name = "xbeginq", .ops  = &jump_ops, },
> +	{ .name = "retq",  .ops  = &ret_ops, },
>  };
>  
>  static struct ins instructions_arm[] = {
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index f7b669e..488c427 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -48,6 +48,7 @@ struct ins {
>  
>  bool ins__is_jump(const struct ins *ins);
>  bool ins__is_call(const struct ins *ins);
> +bool ins__is_ret(const struct ins *ins);
>  int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
>  
>  struct annotation;
> -- 
> 2.5.5

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

#1433628 — [tip:perf/core] perf annotate: Generalize handling of 'ret' instructions

Commit-ID:  6ef9492915b09816c75bb41e7e37b2e507d2f70f
Gitweb:     http://git.kernel.org/tip/6ef9492915b09816c75bb41e7e37b2e507d2f70f
Author:     Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
AuthorDate: Fri, 24 Jun 2016 17:23:58 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Jun 2016 14:25:05 -0300

perf annotate: Generalize handling of 'ret' instructions

Introduce helper to detect 'ret' instructions and use the same in the TUI.
A helper is needed since some architectures such as powerpc have more
than one return instruction.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Link: http://lkml.kernel.org/r/1466769240-12376-5-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c | 20 +++++++++-----------
 tools/perf/util/annotate.c        | 10 ++++++++++
 tools/perf/util/annotate.h        |  1 +
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 0e106bb..29dc6d2 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -223,16 +223,14 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 			} else if (ins__is_call(dl->ins)) {
 				ui_browser__write_graph(browser, SLSMG_RARROW_CHAR);
 				SLsmg_write_char(' ');
+			} else if (ins__is_ret(dl->ins)) {
+				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
+				SLsmg_write_char(' ');
 			} else {
 				ui_browser__write_nstring(browser, " ", 2);
 			}
 		} else {
-			if (strcmp(dl->name, "retq")) {
-				ui_browser__write_nstring(browser, " ", 2);
-			} else {
-				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
-				SLsmg_write_char(' ');
-			}
+			ui_browser__write_nstring(browser, " ", 2);
 		}
 
 		disasm_line__scnprintf(dl, bf, sizeof(bf), !annotate_browser__opts.use_offset);
@@ -843,14 +841,14 @@ show_help:
 				ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
 			else if (browser->selection->offset == -1)
 				ui_helpline__puts("Actions are only available for assembly lines.");
-			else if (!browser->selection->ins) {
-				if (strcmp(browser->selection->name, "retq"))
-					goto show_sup_ins;
+			else if (!browser->selection->ins)
+				goto show_sup_ins;
+			else if (ins__is_ret(browser->selection->ins))
 				goto out;
-			} else if (!(annotate_browser__jump(browser) ||
+			else if (!(annotate_browser__jump(browser) ||
 				     annotate_browser__callq(browser, evsel, hbt))) {
 show_sup_ins:
-				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
+				ui_helpline__puts("Actions are only available for function call/return & jump/branch instructions.");
 			}
 			continue;
 		case 't':
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b2c7ae4..c385fec 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -354,6 +354,15 @@ static struct ins_ops nop_ops = {
 	.scnprintf = nop__scnprintf,
 };
 
+static struct ins_ops ret_ops = {
+	.scnprintf = ins__raw_scnprintf,
+};
+
+bool ins__is_ret(const struct ins *ins)
+{
+	return ins->ops == &ret_ops;
+}
+
 static struct ins instructions[] = {
 	{ .name = "add",   .ops  = &mov_ops, },
 	{ .name = "addl",  .ops  = &mov_ops, },
@@ -444,6 +453,7 @@ static struct ins instructions[] = {
 	{ .name = "xadd",  .ops  = &mov_ops, },
 	{ .name = "xbeginl", .ops  = &jump_ops, },
 	{ .name = "xbeginq", .ops  = &jump_ops, },
+	{ .name = "retq",  .ops  = &ret_ops, },
 };
 
 static int ins__key_cmp(const void *name, const void *insp)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 82f3781..a23084f 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -48,6 +48,7 @@ struct ins {
 
 bool ins__is_jump(const struct ins *ins);
 bool ins__is_call(const struct ins *ins);
+bool ins__is_ret(const struct ins *ins);
 int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
 
 struct annotation;

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

#1430612 — [RFC 5/6] perf annotate: add powerpc support

From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>

Powerpc has long list of branch instructions and hardcoding them in table
appears to be error-prone. So, add new function to find instruction
instead of creating table.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/annotate.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 634daf5..ad01825 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -492,6 +492,68 @@ static struct ins *ins__find(const char *name)
 		       sizeof(struct ins), ins__key_cmp);
 }
 
+static struct ins *ins__find_powerpc(const char *name)
+{
+	int i;
+	struct ins *ins;
+
+	ins = zalloc(sizeof(struct ins));
+	if (!ins)
+		return NULL;
+
+	ins->name = strdup(name);
+	if (!ins->name)
+		return NULL;
+
+	if (name[0] == 'b') {
+		/* branch instructions */
+		ins->ops = &jump_ops;
+
+		/*
+		 * - Few start with 'b', but aren't branch instructions.
+		 * - Let's also ignore instructions involving 'ctr' and
+		 *   'tar' since target branch addresses for those can't
+		 *   be determined statically.
+		 */
+		if (!strncmp(name, "bcd", 3)   ||
+		    !strncmp(name, "brinc", 5) ||
+		    !strncmp(name, "bper", 4)  ||
+		    strstr(name, "ctr")        ||
+		    strstr(name, "tar"))
+			return NULL;
+
+		i = strlen(name) - 1;
+		if (i < 0)
+			return NULL;
+
+		/* ignore optional hints at the end of the instructions */
+		if (name[i] == '+' || name[i] == '-')
+			i--;
+
+		if (name[i] == 'l' || (name[i] == 'a' && name[i-1] == 'l')) {
+			/*
+			 * if the instruction ends up with 'l' or 'la', then
+			 * those are considered 'calls' since they update LR.
+			 * ... except for 'bnl' which is branch if not less than
+			 * and the absolute form of the same.
+			 */
+			if (strcmp(name, "bnl") && strcmp(name, "bnl+") &&
+			    strcmp(name, "bnl-") && strcmp(name, "bnla") &&
+			    strcmp(name, "bnla+") && strcmp(name, "bnla-"))
+				ins->ops = &call_ops;
+		}
+		if (name[i] == 'r' && name[i-1] == 'l')
+			/*
+			 * instructions ending with 'lr' are considered to be
+			 * return instructions
+			 */
+			ins->ops = &ret_ops;
+
+		return ins;
+	}
+	return NULL;
+}
+
 static void __init_arch_ins(const char *arch, struct ins *instructions,
 			    int size, struct ins *(*func)(const char *))
 {
@@ -513,6 +575,8 @@ static int _init_arch_ins(const char *norm_arch)
 		__init_arch_ins(norm_arch, instructions_arm,
 				ARRAY_SIZE(instructions_arm),
 				ins__find);
+	else if (!strcmp(norm_arch, PERF_ARCH_POWERPC))
+		__init_arch_ins(norm_arch, NULL, 0, ins__find_powerpc);
 	else
 		return -1;
 
-- 
2.5.5

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

#1430614 — [RFC 2/6] perf annotate: Define macro for arch names

Define macro for each arch name and use them instead of using arch
name as string.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/arch/common.c           | 36 ++++++++++++++++++------------------
 tools/perf/arch/common.h           | 10 ++++++++++
 tools/perf/util/unwind-libunwind.c |  5 +++--
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index fa090a9..7da6ac7 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -105,25 +105,25 @@ static int lookup_triplets(const char *const *triplets, const char *name)
 const char *normalize_arch(char *arch)
 {
 	if (!strcmp(arch, "x86_64"))
-		return "x86";
+		return PERF_ARCH_X86;
 	if (arch[0] == 'i' && arch[2] == '8' && arch[3] == '6')
-		return "x86";
+		return PERF_ARCH_X86;
 	if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5))
-		return "sparc";
+		return PERF_ARCH_SPARC;
 	if (!strcmp(arch, "aarch64") || !strcmp(arch, "arm64"))
-		return "arm64";
+		return PERF_ARCH_ARM64;
 	if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110"))
-		return "arm";
+		return PERF_ARCH_ARM;
 	if (!strncmp(arch, "s390", 4))
-		return "s390";
+		return PERF_ARCH_S390;
 	if (!strncmp(arch, "parisc", 6))
-		return "parisc";
+		return PERF_ARCH_PARISC;
 	if (!strncmp(arch, "powerpc", 7) || !strncmp(arch, "ppc", 3))
-		return "powerpc";
+		return PERF_ARCH_POWERPC;
 	if (!strncmp(arch, "mips", 4))
-		return "mips";
+		return PERF_ARCH_MIPS;
 	if (!strncmp(arch, "sh", 2) && isdigit(arch[2]))
-		return "sh";
+		return PERF_ARCH_SH;
 
 	return arch;
 }
@@ -163,21 +163,21 @@ static int perf_env__lookup_binutils_path(struct perf_env *env,
 		zfree(&buf);
 	}
 
-	if (!strcmp(arch, "arm"))
+	if (!strcmp(arch, PERF_ARCH_ARM))
 		path_list = arm_triplets;
-	else if (!strcmp(arch, "arm64"))
+	else if (!strcmp(arch, PERF_ARCH_ARM64))
 		path_list = arm64_triplets;
-	else if (!strcmp(arch, "powerpc"))
+	else if (!strcmp(arch, PERF_ARCH_POWERPC))
 		path_list = powerpc_triplets;
-	else if (!strcmp(arch, "sh"))
+	else if (!strcmp(arch, PERF_ARCH_SH))
 		path_list = sh_triplets;
-	else if (!strcmp(arch, "s390"))
+	else if (!strcmp(arch, PERF_ARCH_S390))
 		path_list = s390_triplets;
-	else if (!strcmp(arch, "sparc"))
+	else if (!strcmp(arch, PERF_ARCH_SPARC))
 		path_list = sparc_triplets;
-	else if (!strcmp(arch, "x86"))
+	else if (!strcmp(arch, PERF_ARCH_X86))
 		path_list = x86_triplets;
-	else if (!strcmp(arch, "mips"))
+	else if (!strcmp(arch, PERF_ARCH_MIPS))
 		path_list = mips_triplets;
 	else {
 		ui__error("binutils for %s not supported.\n", arch);
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
index 6b01c73..bbb6960 100644
--- a/tools/perf/arch/common.h
+++ b/tools/perf/arch/common.h
@@ -5,6 +5,16 @@
 
 extern const char *objdump_path;
 
+#define PERF_ARCH_X86		"x86"
+#define PERF_ARCH_SPARC		"sparc"
+#define PERF_ARCH_ARM64		"arm64"
+#define PERF_ARCH_ARM		"arm"
+#define PERF_ARCH_S390		"s390"
+#define PERF_ARCH_PARISC	"parisc"
+#define PERF_ARCH_POWERPC	"powerpc"
+#define PERF_ARCH_MIPS		"mips"
+#define PERF_ARCH_SH		"sh"
+
 int perf_env__lookup_objdump(struct perf_env *env);
 const char *normalize_arch(char *arch);
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 8547119..ccebe5e 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -36,10 +36,11 @@ int unwind__prepare_access(struct thread *thread, struct map *map)
 
 	arch = normalize_arch(thread->mg->machine->env->arch);
 
-	if (!strcmp(arch, "x86")) {
+	if (!strcmp(arch, PERF_ARCH_X86)) {
 		if (dso_type != DSO__TYPE_64BIT)
 			ops = x86_32_unwind_libunwind_ops;
-	} else if (!strcmp(arch, "arm64") || !strcmp(arch, "arm")) {
+	} else if (!strcmp(arch, PERF_ARCH_ARM64) ||
+		   !strcmp(arch, PERF_ARCH_ARM)) {
 		if (dso_type == DSO__TYPE_64BIT)
 			ops = arm64_unwind_libunwind_ops;
 	}
-- 
2.5.5

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

#1430616 — [RFC 1/6] perf: Remove unused hist_entry__annotate function

hist_entry__annotate looks part of API but I don't find any caller
of this function. Removing it.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/annotate.c | 5 -----
 tools/perf/util/annotate.h | 2 --
 2 files changed, 7 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 7e5a1e8..b2c7ae4 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1676,11 +1676,6 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 	return 0;
 }
 
-int hist_entry__annotate(struct hist_entry *he, size_t privsize)
-{
-	return symbol__annotate(he->ms.sym, he->ms.map, privsize);
-}
-
 bool ui__has_annotation(void)
 {
 	return use_browser == 1 && perf_hpp_list.sym;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9241f8c..82f3781 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -156,8 +156,6 @@ void symbol__annotate_zero_histograms(struct symbol *sym);
 
 int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
 
-int hist_entry__annotate(struct hist_entry *he, size_t privsize);
-
 int symbol__annotate_init(struct map *map, struct symbol *sym);
 int symbol__annotate_printf(struct symbol *sym, struct map *map,
 			    struct perf_evsel *evsel, bool full_paths,
-- 
2.5.5

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

#1432190 — Re: [RFC 1/6] perf: Remove unused hist_entry__annotate function

Em Fri, Jun 24, 2016 at 05:23:55PM +0530, Ravi Bangoria escreveu:
> hist_entry__annotate looks part of API but I don't find any caller
> of this function. Removing it.

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/util/annotate.c | 5 -----
>  tools/perf/util/annotate.h | 2 --
>  2 files changed, 7 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 7e5a1e8..b2c7ae4 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1676,11 +1676,6 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
>  	return 0;
>  }
>  
> -int hist_entry__annotate(struct hist_entry *he, size_t privsize)
> -{
> -	return symbol__annotate(he->ms.sym, he->ms.map, privsize);
> -}
> -
>  bool ui__has_annotation(void)
>  {
>  	return use_browser == 1 && perf_hpp_list.sym;
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 9241f8c..82f3781 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -156,8 +156,6 @@ void symbol__annotate_zero_histograms(struct symbol *sym);
>  
>  int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
>  
> -int hist_entry__annotate(struct hist_entry *he, size_t privsize);
> -
>  int symbol__annotate_init(struct map *map, struct symbol *sym);
>  int symbol__annotate_printf(struct symbol *sym, struct map *map,
>  			    struct perf_evsel *evsel, bool full_paths,
> -- 
> 2.5.5

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

#1433631 — [tip:perf/core] perf annotate: Remove unused hist_entry__annotate function

Commit-ID:  f2f4fe4410ac6de96f8561aefeadbb680e5ddc99
Gitweb:     http://git.kernel.org/tip/f2f4fe4410ac6de96f8561aefeadbb680e5ddc99
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Fri, 24 Jun 2016 17:23:55 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Jun 2016 10:58:50 -0300

perf annotate: Remove unused hist_entry__annotate function

hist_entry__annotate looks part of API but I don't find any caller
of this function. Removing it.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Link: http://lkml.kernel.org/r/1466769240-12376-2-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 5 -----
 tools/perf/util/annotate.h | 2 --
 2 files changed, 7 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 7e5a1e8..b2c7ae4 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1676,11 +1676,6 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 	return 0;
 }
 
-int hist_entry__annotate(struct hist_entry *he, size_t privsize)
-{
-	return symbol__annotate(he->ms.sym, he->ms.map, privsize);
-}
-
 bool ui__has_annotation(void)
 {
 	return use_browser == 1 && perf_hpp_list.sym;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9241f8c..82f3781 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -156,8 +156,6 @@ void symbol__annotate_zero_histograms(struct symbol *sym);
 
 int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
 
-int hist_entry__annotate(struct hist_entry *he, size_t privsize);
-
 int symbol__annotate_init(struct map *map, struct symbol *sym);
 int symbol__annotate_printf(struct symbol *sym, struct map *map,
 			    struct perf_evsel *evsel, bool full_paths,

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

#1430618 — [RFC 3/6] perf annotate: Enable cross arch annotate

Change current data structures and function to enable cross arch annotate
and add support for x86 and arm instructions.

Current implementation does not contain logic of recording on one arch
and annotating on other. This remote annotate is partially possible with
current implementation for x86 (or may be arm as well) only. But, to make
remote annotation work properly, all architecture instruction tables need
to be included in the perf binary. And while annotating, look for
instruction table where perf.data was recorded.

For arm, few instructions were defined under #if __arm__ which I've used
as a table for arm. But I'm not sure whether instruction defined outside
of that also contains arm instructions.

Note:
Here arch_ins is global var. And init_arch_ins will be called every
time when we annotate symbol. So I still need to optimize this.
May be make arch_ins per session. Please suggest best way to do it.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/builtin-top.c          |   2 +-
 tools/perf/ui/browsers/annotate.c |   5 +-
 tools/perf/ui/gtk/annotate.c      |   6 +-
 tools/perf/util/annotate.c        | 116 +++++++++++++++++++++++++++++---------
 tools/perf/util/annotate.h        |   3 +-
 tools/perf/util/evsel.c           |   7 +++
 tools/perf/util/evsel.h           |   2 +
 7 files changed, 108 insertions(+), 33 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 07fc792..d4fd947 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -128,7 +128,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
 		return err;
 	}
 
-	err = symbol__annotate(sym, map, 0);
+	err = symbol__annotate(sym, map, 0, NULL);
 	if (err == 0) {
 out_assign:
 		top->sym_filter_entry = he;
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 0e106bb..b65a979 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -1031,6 +1031,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 	int ret = -1;
 	int nr_pcnt = 1;
 	size_t sizeof_bdl = sizeof(struct browser_disasm_line);
+	char *target_arch = NULL;
 
 	if (sym == NULL)
 		return -1;
@@ -1052,7 +1053,9 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 		  (nr_pcnt - 1);
 	}
 
-	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
+	target_arch = perf_evsel__env_arch(evsel);
+
+	if (symbol__annotate(sym, map, sizeof_bdl, target_arch) < 0) {
 		ui__error("%s", ui_helpline__last_msg);
 		goto out_free_offsets;
 	}
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 9c7ff8d..e468c1a 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -4,7 +4,6 @@
 #include "util/evsel.h"
 #include "ui/helpline.h"
 
-
 enum {
 	ANN_COL__PERCENT,
 	ANN_COL__OFFSET,
@@ -162,11 +161,14 @@ static int symbol__gtk_annotate(struct symbol *sym, struct map *map,
 	GtkWidget *notebook;
 	GtkWidget *scrolled_window;
 	GtkWidget *tab_label;
+	char *target_arch = NULL;
 
 	if (map->dso->annotate_warned)
 		return -1;
 
-	if (symbol__annotate(sym, map, 0) < 0) {
+	target_arch = perf_evsel__env_arch(evsel);
+
+	if (symbol__annotate(sym, map, 0, target_arch) < 0) {
 		ui__error("%s", ui_helpline__current);
 		return -1;
 	}
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b2c7ae4..e0dc7b2 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -20,6 +20,8 @@
 #include <regex.h>
 #include <pthread.h>
 #include <linux/bitops.h>
+#include <sys/utsname.h>
+#include "../arch/common.h"
 
 const char 	*disassembler_style;
 const char	*objdump_path;
@@ -28,6 +30,13 @@ static regex_t	 file_lineno;
 static struct ins *ins__find(const char *name);
 static int disasm_line__parse(char *line, char **namep, char **rawp);
 
+static struct arch_instructions {
+	const char *arch;
+	int	   nmemb;
+	struct ins *instructions;
+	struct ins *(*ins__find)(const char *);
+} arch_ins;
+
 static void ins__delete(struct ins_operands *ops)
 {
 	if (ops == NULL)
@@ -183,7 +192,7 @@ static int lock__parse(struct ins_operands *ops)
 	if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0)
 		goto out_free_ops;
 
-	ops->locked.ins = ins__find(name);
+	ops->locked.ins = arch_ins.ins__find(name);
 	free(name);
 
 	if (ops->locked.ins == NULL)
@@ -354,26 +363,12 @@ static struct ins_ops nop_ops = {
 	.scnprintf = nop__scnprintf,
 };
 
-static struct ins instructions[] = {
+static struct ins instructions_x86[] = {
 	{ .name = "add",   .ops  = &mov_ops, },
 	{ .name = "addl",  .ops  = &mov_ops, },
 	{ .name = "addq",  .ops  = &mov_ops, },
 	{ .name = "addw",  .ops  = &mov_ops, },
 	{ .name = "and",   .ops  = &mov_ops, },
-#ifdef __arm__
-	{ .name = "b",     .ops  = &jump_ops, }, // might also be a call
-	{ .name = "bcc",   .ops  = &jump_ops, },
-	{ .name = "bcs",   .ops  = &jump_ops, },
-	{ .name = "beq",   .ops  = &jump_ops, },
-	{ .name = "bge",   .ops  = &jump_ops, },
-	{ .name = "bgt",   .ops  = &jump_ops, },
-	{ .name = "bhi",   .ops  = &jump_ops, },
-	{ .name = "bl",    .ops  = &call_ops, },
-	{ .name = "bls",   .ops  = &jump_ops, },
-	{ .name = "blt",   .ops  = &jump_ops, },
-	{ .name = "blx",   .ops  = &call_ops, },
-	{ .name = "bne",   .ops  = &jump_ops, },
-#endif
 	{ .name = "bts",   .ops  = &mov_ops, },
 	{ .name = "call",  .ops  = &call_ops, },
 	{ .name = "callq", .ops  = &call_ops, },
@@ -446,6 +441,21 @@ static struct ins instructions[] = {
 	{ .name = "xbeginq", .ops  = &jump_ops, },
 };
 
+static struct ins instructions_arm[] = {
+	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */
+	{ .name = "bcc",   .ops  = &jump_ops, },
+	{ .name = "bcs",   .ops  = &jump_ops, },
+	{ .name = "beq",   .ops  = &jump_ops, },
+	{ .name = "bge",   .ops  = &jump_ops, },
+	{ .name = "bgt",   .ops  = &jump_ops, },
+	{ .name = "bhi",   .ops  = &jump_ops, },
+	{ .name = "bl",    .ops  = &call_ops, },
+	{ .name = "bls",   .ops  = &jump_ops, },
+	{ .name = "blt",   .ops  = &jump_ops, },
+	{ .name = "blx",   .ops  = &call_ops, },
+	{ .name = "bne",   .ops  = &jump_ops, },
+};
+
 static int ins__key_cmp(const void *name, const void *insp)
 {
 	const struct ins *ins = insp;
@@ -461,24 +471,69 @@ static int ins__cmp(const void *a, const void *b)
 	return strcmp(ia->name, ib->name);
 }
 
-static void ins__sort(void)
+static void ins__sort(struct ins *instructions, int nmemb)
 {
-	const int nmemb = ARRAY_SIZE(instructions);
-
 	qsort(instructions, nmemb, sizeof(struct ins), ins__cmp);
 }
 
 static struct ins *ins__find(const char *name)
 {
-	const int nmemb = ARRAY_SIZE(instructions);
-	static bool sorted;
+	return bsearch(name, arch_ins.instructions, arch_ins.nmemb,
+		       sizeof(struct ins), ins__key_cmp);
+}
+
+static void __init_arch_ins(const char *arch, struct ins *instructions,
+			    int size, struct ins *(*func)(const char *))
+{
+	ins__sort(instructions, size);
+
+	arch_ins.arch = arch;
+	arch_ins.nmemb = size;
+	arch_ins.instructions = instructions;
+	arch_ins.ins__find = func;
+}
+
+static int _init_arch_ins(const char *norm_arch)
+{
+	if (!strcmp(norm_arch, PERF_ARCH_X86))
+		__init_arch_ins(norm_arch, instructions_x86,
+				ARRAY_SIZE(instructions_x86),
+				ins__find);
+	else if (!strcmp(norm_arch, PERF_ARCH_ARM))
+		__init_arch_ins(norm_arch, instructions_arm,
+				ARRAY_SIZE(instructions_arm),
+				ins__find);
+	else
+		return -1;
+
+	return 0;
+}
+
+static int init_arch_ins(char *target_arch)
+{
+	const char *norm_arch = NULL;
+	struct utsname uts;
 
-	if (!sorted) {
-		ins__sort();
-		sorted = true;
+	if (!target_arch) { /* Assume we are annotating locally. */
+		if (uname(&uts) < 0) {
+			pr_err("Can not annotate. Could not determine architecture.");
+			return -1;
+		}
+		target_arch = uts.machine;
 	}
 
-	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__key_cmp);
+	norm_arch = normalize_arch(target_arch);
+
+	/* retuen if already initialized. */
+	if (arch_ins.arch && !strcmp(norm_arch, arch_ins.arch))
+		return 0;
+
+	if (_init_arch_ins(norm_arch)) {
+		pr_err("perf annotate not supported by %s arch\n", target_arch);
+		return -1;
+	}
+
+	return 0;
 }
 
 int symbol__annotate_init(struct map *map __maybe_unused, struct symbol *sym)
@@ -707,7 +762,7 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
 
 static void disasm_line__init_ins(struct disasm_line *dl)
 {
-	dl->ins = ins__find(dl->name);
+	dl->ins = arch_ins.ins__find(dl->name);
 
 	if (dl->ins == NULL)
 		return;
@@ -1113,7 +1168,8 @@ static void delete_last_nop(struct symbol *sym)
 	}
 }
 
-int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
+int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize,
+		     char *target_arch)
 {
 	struct dso *dso = map->dso;
 	char *filename = dso__build_id_filename(dso, NULL, 0);
@@ -1258,6 +1314,9 @@ fallback:
 		goto out_remove_tmp;
 	}
 
+	if (init_arch_ins(target_arch) < 0)
+		goto out_arch_err;
+
 	nline = 0;
 	while (!feof(file)) {
 		if (symbol__parse_objdump_line(sym, map, file, privsize,
@@ -1269,6 +1328,7 @@ fallback:
 	if (nline == 0)
 		pr_err("No output from %s\n", command);
 
+out_arch_err:
 	/*
 	 * kallsyms does not have symbol sizes so there may a nop at the end.
 	 * Remove it.
@@ -1655,7 +1715,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 	struct rb_root source_line = RB_ROOT;
 	u64 len;
 
-	if (symbol__annotate(sym, map, 0) < 0)
+	if (symbol__annotate(sym, map, 0, perf_evsel__env_arch(evsel)) < 0)
 		return -1;
 
 	len = symbol__size(sym);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 82f3781..f7b669e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -154,7 +154,8 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr);
 int symbol__alloc_hist(struct symbol *sym);
 void symbol__annotate_zero_histograms(struct symbol *sym);
 
-int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
+int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize,
+		     char *target_arch);
 
 int symbol__annotate_init(struct map *map, struct symbol *sym);
 int symbol__annotate_printf(struct symbol *sym, struct map *map,
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1d8f2bb..0fea724 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2422,3 +2422,10 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 			 err, strerror_r(err, sbuf, sizeof(sbuf)),
 			 perf_evsel__name(evsel));
 }
+
+char *perf_evsel__env_arch(struct perf_evsel *evsel)
+{
+	if (evsel && evsel->evlist && evsel->evlist->env)
+		return evsel->evlist->env->arch;
+	return NULL;
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 828ddd1..86fed7a 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -435,4 +435,6 @@ typedef int (*attr__fprintf_f)(FILE *, const char *, const char *, void *);
 int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 			     attr__fprintf_f attr__fprintf, void *priv);
 
+char *perf_evsel__env_arch(struct perf_evsel *evsel);
+
 #endif /* __PERF_EVSEL_H */
-- 
2.5.5

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

#1432178 — Re: [RFC 3/6] perf annotate: Enable cross arch annotate

Em Fri, Jun 24, 2016 at 05:23:57PM +0530, Ravi Bangoria escreveu:
> Change current data structures and function to enable cross arch annotate
> and add support for x86 and arm instructions.
> 
> Current implementation does not contain logic of recording on one arch
> and annotating on other. This remote annotate is partially possible with
> current implementation for x86 (or may be arm as well) only. But, to make
> remote annotation work properly, all architecture instruction tables need
> to be included in the perf binary. And while annotating, look for
> instruction table where perf.data was recorded.
> 
> For arm, few instructions were defined under #if __arm__ which I've used
> as a table for arm. But I'm not sure whether instruction defined outside
> of that also contains arm instructions.
> 
> Note:
> Here arch_ins is global var. And init_arch_ins will be called every
> time when we annotate symbol. So I still need to optimize this.
> May be make arch_ins per session. Please suggest best way to do it.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/builtin-top.c          |   2 +-
>  tools/perf/ui/browsers/annotate.c |   5 +-
>  tools/perf/ui/gtk/annotate.c      |   6 +-
>  tools/perf/util/annotate.c        | 116 +++++++++++++++++++++++++++++---------
>  tools/perf/util/annotate.h        |   3 +-
>  tools/perf/util/evsel.c           |   7 +++
>  tools/perf/util/evsel.h           |   2 +
>  7 files changed, 108 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 07fc792..d4fd947 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -128,7 +128,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
>  		return err;
>  	}
>  
> -	err = symbol__annotate(sym, map, 0);
> +	err = symbol__annotate(sym, map, 0, NULL);
>  	if (err == 0) {
>  out_assign:
>  		top->sym_filter_entry = he;
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 0e106bb..b65a979 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -1031,6 +1031,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>  	int ret = -1;
>  	int nr_pcnt = 1;
>  	size_t sizeof_bdl = sizeof(struct browser_disasm_line);
> +	char *target_arch = NULL;
>  
>  	if (sym == NULL)
>  		return -1;
> @@ -1052,7 +1053,9 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>  		  (nr_pcnt - 1);
>  	}
>  
> -	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
> +	target_arch = perf_evsel__env_arch(evsel);
> +
> +	if (symbol__annotate(sym, map, sizeof_bdl, target_arch) < 0) {
>  		ui__error("%s", ui_helpline__last_msg);
>  		goto out_free_offsets;
>  	}
> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
> index 9c7ff8d..e468c1a 100644
> --- a/tools/perf/ui/gtk/annotate.c
> +++ b/tools/perf/ui/gtk/annotate.c
> @@ -4,7 +4,6 @@
>  #include "util/evsel.h"
>  #include "ui/helpline.h"
>  
> -
>  enum {
>  	ANN_COL__PERCENT,
>  	ANN_COL__OFFSET,
> @@ -162,11 +161,14 @@ static int symbol__gtk_annotate(struct symbol *sym, struct map *map,
>  	GtkWidget *notebook;
>  	GtkWidget *scrolled_window;
>  	GtkWidget *tab_label;
> +	char *target_arch = NULL;
>  
>  	if (map->dso->annotate_warned)
>  		return -1;
>  
> -	if (symbol__annotate(sym, map, 0) < 0) {
> +	target_arch = perf_evsel__env_arch(evsel);
> +
> +	if (symbol__annotate(sym, map, 0, target_arch) < 0) {
>  		ui__error("%s", ui_helpline__current);
>  		return -1;
>  	}
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index b2c7ae4..e0dc7b2 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -20,6 +20,8 @@
>  #include <regex.h>
>  #include <pthread.h>
>  #include <linux/bitops.h>
> +#include <sys/utsname.h>
> +#include "../arch/common.h"
>  
>  const char 	*disassembler_style;
>  const char	*objdump_path;
> @@ -28,6 +30,13 @@ static regex_t	 file_lineno;
>  static struct ins *ins__find(const char *name);
>  static int disasm_line__parse(char *line, char **namep, char **rawp);
>  
> +static struct arch_instructions {
> +	const char *arch;
> +	int	   nmemb;
> +	struct ins *instructions;
> +	struct ins *(*ins__find)(const char *);

Why do we need arch specific find functions? Why not pass the
instructions pointer to it, just like you did with ins__sort().

Probably it is not needed to be global, you just pick the right
instructions table + its ARRAY_SIZE and pass it around, again, like you
did in ins__sort().

- Arnaldo

> +} arch_ins;
> +
>  static void ins__delete(struct ins_operands *ops)
>  {
>  	if (ops == NULL)
> @@ -183,7 +192,7 @@ static int lock__parse(struct ins_operands *ops)
>  	if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0)
>  		goto out_free_ops;
>  
> -	ops->locked.ins = ins__find(name);
> +	ops->locked.ins = arch_ins.ins__find(name);
>  	free(name);
>  
>  	if (ops->locked.ins == NULL)
> @@ -354,26 +363,12 @@ static struct ins_ops nop_ops = {
>  	.scnprintf = nop__scnprintf,
>  };
>  
> -static struct ins instructions[] = {
> +static struct ins instructions_x86[] = {
>  	{ .name = "add",   .ops  = &mov_ops, },
>  	{ .name = "addl",  .ops  = &mov_ops, },
>  	{ .name = "addq",  .ops  = &mov_ops, },
>  	{ .name = "addw",  .ops  = &mov_ops, },
>  	{ .name = "and",   .ops  = &mov_ops, },
> -#ifdef __arm__
> -	{ .name = "b",     .ops  = &jump_ops, }, // might also be a call
> -	{ .name = "bcc",   .ops  = &jump_ops, },
> -	{ .name = "bcs",   .ops  = &jump_ops, },
> -	{ .name = "beq",   .ops  = &jump_ops, },
> -	{ .name = "bge",   .ops  = &jump_ops, },
> -	{ .name = "bgt",   .ops  = &jump_ops, },
> -	{ .name = "bhi",   .ops  = &jump_ops, },
> -	{ .name = "bl",    .ops  = &call_ops, },
> -	{ .name = "bls",   .ops  = &jump_ops, },
> -	{ .name = "blt",   .ops  = &jump_ops, },
> -	{ .name = "blx",   .ops  = &call_ops, },
> -	{ .name = "bne",   .ops  = &jump_ops, },
> -#endif
>  	{ .name = "bts",   .ops  = &mov_ops, },
>  	{ .name = "call",  .ops  = &call_ops, },
>  	{ .name = "callq", .ops  = &call_ops, },
> @@ -446,6 +441,21 @@ static struct ins instructions[] = {
>  	{ .name = "xbeginq", .ops  = &jump_ops, },
>  };
>  
> +static struct ins instructions_arm[] = {
> +	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */
> +	{ .name = "bcc",   .ops  = &jump_ops, },
> +	{ .name = "bcs",   .ops  = &jump_ops, },
> +	{ .name = "beq",   .ops  = &jump_ops, },
> +	{ .name = "bge",   .ops  = &jump_ops, },
> +	{ .name = "bgt",   .ops  = &jump_ops, },
> +	{ .name = "bhi",   .ops  = &jump_ops, },
> +	{ .name = "bl",    .ops  = &call_ops, },
> +	{ .name = "bls",   .ops  = &jump_ops, },
> +	{ .name = "blt",   .ops  = &jump_ops, },
> +	{ .name = "blx",   .ops  = &call_ops, },
> +	{ .name = "bne",   .ops  = &jump_ops, },
> +};
> +
>  static int ins__key_cmp(const void *name, const void *insp)
>  {
>  	const struct ins *ins = insp;
> @@ -461,24 +471,69 @@ static int ins__cmp(const void *a, const void *b)
>  	return strcmp(ia->name, ib->name);
>  }
>  
> -static void ins__sort(void)
> +static void ins__sort(struct ins *instructions, int nmemb)
>  {
> -	const int nmemb = ARRAY_SIZE(instructions);
> -
>  	qsort(instructions, nmemb, sizeof(struct ins), ins__cmp);
>  }
>  
>  static struct ins *ins__find(const char *name)
>  {
> -	const int nmemb = ARRAY_SIZE(instructions);
> -	static bool sorted;
> +	return bsearch(name, arch_ins.instructions, arch_ins.nmemb,
> +		       sizeof(struct ins), ins__key_cmp);
> +}
> +
> +static void __init_arch_ins(const char *arch, struct ins *instructions,
> +			    int size, struct ins *(*func)(const char *))
> +{
> +	ins__sort(instructions, size);
> +
> +	arch_ins.arch = arch;
> +	arch_ins.nmemb = size;
> +	arch_ins.instructions = instructions;
> +	arch_ins.ins__find = func;
> +}
> +
> +static int _init_arch_ins(const char *norm_arch)
> +{
> +	if (!strcmp(norm_arch, PERF_ARCH_X86))
> +		__init_arch_ins(norm_arch, instructions_x86,
> +				ARRAY_SIZE(instructions_x86),
> +				ins__find);
> +	else if (!strcmp(norm_arch, PERF_ARCH_ARM))
> +		__init_arch_ins(norm_arch, instructions_arm,
> +				ARRAY_SIZE(instructions_arm),
> +				ins__find);
> +	else
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int init_arch_ins(char *target_arch)
> +{
> +	const char *norm_arch = NULL;
> +	struct utsname uts;
>  
> -	if (!sorted) {
> -		ins__sort();
> -		sorted = true;
> +	if (!target_arch) { /* Assume we are annotating locally. */
> +		if (uname(&uts) < 0) {
> +			pr_err("Can not annotate. Could not determine architecture.");
> +			return -1;
> +		}
> +		target_arch = uts.machine;
>  	}
>  
> -	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__key_cmp);
> +	norm_arch = normalize_arch(target_arch);
> +
> +	/* retuen if already initialized. */
> +	if (arch_ins.arch && !strcmp(norm_arch, arch_ins.arch))
> +		return 0;
> +
> +	if (_init_arch_ins(norm_arch)) {
> +		pr_err("perf annotate not supported by %s arch\n", target_arch);
> +		return -1;
> +	}
> +
> +	return 0;
>  }
>  
>  int symbol__annotate_init(struct map *map __maybe_unused, struct symbol *sym)
> @@ -707,7 +762,7 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
>  
>  static void disasm_line__init_ins(struct disasm_line *dl)
>  {
> -	dl->ins = ins__find(dl->name);
> +	dl->ins = arch_ins.ins__find(dl->name);
>  
>  	if (dl->ins == NULL)
>  		return;
> @@ -1113,7 +1168,8 @@ static void delete_last_nop(struct symbol *sym)
>  	}
>  }
>  
> -int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
> +int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize,
> +		     char *target_arch)
>  {
>  	struct dso *dso = map->dso;
>  	char *filename = dso__build_id_filename(dso, NULL, 0);
> @@ -1258,6 +1314,9 @@ fallback:
>  		goto out_remove_tmp;
>  	}
>  
> +	if (init_arch_ins(target_arch) < 0)
> +		goto out_arch_err;
> +
>  	nline = 0;
>  	while (!feof(file)) {
>  		if (symbol__parse_objdump_line(sym, map, file, privsize,
> @@ -1269,6 +1328,7 @@ fallback:
>  	if (nline == 0)
>  		pr_err("No output from %s\n", command);
>  
> +out_arch_err:
>  	/*
>  	 * kallsyms does not have symbol sizes so there may a nop at the end.
>  	 * Remove it.
> @@ -1655,7 +1715,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
>  	struct rb_root source_line = RB_ROOT;
>  	u64 len;
>  
> -	if (symbol__annotate(sym, map, 0) < 0)
> +	if (symbol__annotate(sym, map, 0, perf_evsel__env_arch(evsel)) < 0)
>  		return -1;
>  
>  	len = symbol__size(sym);
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 82f3781..f7b669e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -154,7 +154,8 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr);
>  int symbol__alloc_hist(struct symbol *sym);
>  void symbol__annotate_zero_histograms(struct symbol *sym);
>  
> -int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
> +int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize,
> +		     char *target_arch);
>  
>  int symbol__annotate_init(struct map *map, struct symbol *sym);
>  int symbol__annotate_printf(struct symbol *sym, struct map *map,
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 1d8f2bb..0fea724 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2422,3 +2422,10 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
>  			 err, strerror_r(err, sbuf, sizeof(sbuf)),
>  			 perf_evsel__name(evsel));
>  }
> +
> +char *perf_evsel__env_arch(struct perf_evsel *evsel)
> +{
> +	if (evsel && evsel->evlist && evsel->evlist->env)
> +		return evsel->evlist->env->arch;
> +	return NULL;
> +}
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 828ddd1..86fed7a 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -435,4 +435,6 @@ typedef int (*attr__fprintf_f)(FILE *, const char *, const char *, void *);
>  int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
>  			     attr__fprintf_f attr__fprintf, void *priv);
>  
> +char *perf_evsel__env_arch(struct perf_evsel *evsel);
> +
>  #endif /* __PERF_EVSEL_H */
> -- 
> 2.5.5

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

#1432906 — Re: [RFC 3/6] perf annotate: Enable cross arch annotate

On Monday 27 June 2016 10:46 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 24, 2016 at 05:23:57PM +0530, Ravi Bangoria escreveu:
>> Change current data structures and function to enable cross arch annotate
>> and add support for x86 and arm instructions.
>>
>> Current implementation does not contain logic of recording on one arch
>> and annotating on other. This remote annotate is partially possible with
>> current implementation for x86 (or may be arm as well) only. But, to make
>> remote annotation work properly, all architecture instruction tables need
>> to be included in the perf binary. And while annotating, look for
>> instruction table where perf.data was recorded.

...

>>   
>> +static struct arch_instructions {
>> +	const char *arch;
>> +	int	   nmemb;
>> +	struct ins *instructions;
>> +	struct ins *(*ins__find)(const char *);
> Why do we need arch specific find functions? Why not pass the
> instructions pointer to it, just like you did with ins__sort().
>
> Probably it is not needed to be global, you just pick the right
> instructions table + its ARRAY_SIZE and pass it around, again, like you
> did in ins__sort().
>
> - Arnaldo

Thanks Arnaldo for suggestion.

To determine arch in ins__find, I need to pass 'arch' till ins__find and 
which
requires changes in definition of many functions. So, I thought about global
var.

Anyway, I've prepared a patch as you suggested and sent it as a [PATCH].
Please review it.

-Ravi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web