Skip to contentSkip to conversation

perf tools: Use normalized arch name for searching objdump path

PATCH RESEND In review Namhyung Kim 3 participants 9 messages Nov 2, 2012 – Nov 14, 2012 perf events

lore
Share & tools
lore

Namhyung Kim

author maintainer
#0 lore raw

PATCH RESEND 1/3 perf tools: Use normalized arch name for searching objdump path

From: Namhyung Kim <[email protected]>

David reported that perf report for i686 target data on x86_64 host failed to work because it tried to find out cross-compiled objdump.

However objdump for x86_64 is compatible to i686 so that it doesn't need to do it at all. To prevent similar artifacts, normalize arch name when comparing host and file architectures.

---
 tools/perf/arch/common.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)
tools/perf/arch/common.c +33 -7
index 2367b253f039..5683529135b1 100644
@@ -93,16 +93,46 @@ static int lookup_triplets(const char *const *triplets, const char *name)
 	return -1;
 }
 
+/*
+ * Return architecture name in a normalized form.
+ * The conversion logic comes from the Makefile.
+ */
+static const char *normalize_arch(char *arch)
+{
+	if (!strcmp(arch, "x86_64"))
+		return "x86";
+	if (arch[0] == 'i' && arch[2] == '8' && arch[3] == '6')
+		return "x86";
+	if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5))
+		return "sparc";
+	if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110"))
+		return "arm";
+	if (!strncmp(arch, "s390", 4))
+		return "s390";
+	if (!strncmp(arch, "parisc", 6))
+		return "parisc";
+	if (!strncmp(arch, "powerpc", 7) || !strncmp(arch, "ppc", 3))
+		return "powerpc";
+	if (!strncmp(arch, "mips", 4))
+		return "mips";
+	if (!strncmp(arch, "sh", 2) && isdigit(arch[2]))
+		return "sh";
+
+	return arch;
+}
+
 static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 						  const char *name,
 						  const char **path)
 {
 	int idx;
-	char *arch, *cross_env;
+	const char *arch, *cross_env;
 	struct utsname uts;
 	const char *const *path_list;
 	char *buf = NULL;
 
+	arch = normalize_arch(env->arch);
+
 	if (uname(&uts) < 0)
 		goto out;
 
@@ -110,7 +140,7 @@ static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 	 * We don't need to try to find objdump path for native system.
 	 * Just use default binutils path (e.g.: "objdump").
 	 */
-	if (!strcmp(uts.machine, env->arch))
+	if (!strcmp(normalize_arch(uts.machine), arch))
 		goto out;
 
 	cross_env = getenv("CROSS_COMPILE");
@@ -127,8 +157,6 @@ static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 		free(buf);
 	}
 
-	arch = env->arch;
-
 	if (!strcmp(arch, "arm"))
 		path_list = arm_triplets;
 	else if (!strcmp(arch, "powerpc"))
@@ -139,9 +167,7 @@ static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 		path_list = s390_triplets;
 	else if (!strcmp(arch, "sparc"))
 		path_list = sparc_triplets;
-	else if (!strcmp(arch, "x86") || !strcmp(arch, "i386") ||
-		 !strcmp(arch, "i486") || !strcmp(arch, "i586") ||
-		 !strcmp(arch, "i686"))
+	else if (!strcmp(arch, "x86"))
 		path_list = x86_triplets;
 	else if (!strcmp(arch, "mips"))
 		path_list = mips_triplets;
Signature · 1 line
1.7.11.7
author maintainer
#1 lore raw

PATCH 2/3 perf tools: Introduce struct hist_browser_timer

From: Namhyung Kim <[email protected]>

Currently various hist browser functions receive 3 arguments for refreshing histogram but only used from a few places. Also it's only for perf top command so that it can be NULL for other (and probably most) cases. Pack them into a struct in order to reduce number of those unused arguments.

This is a mechanical change and does not intend a functional change.

Cc David Ahern <[email protected]>
Cc Irina Tirdea <[email protected]>
---
 tools/perf/builtin-annotate.c     |  2 +-
 tools/perf/builtin-report.c       |  4 ++--
 tools/perf/builtin-top.c          |  9 +++++---
 tools/perf/ui/browsers/annotate.c | 27 +++++++++++-------------
 tools/perf/ui/browsers/hists.c    | 43 +++++++++++++++++----------------------
 tools/perf/ui/gtk/browser.c       |  4 +---
 tools/perf/util/annotate.h        |  8 ++++----
 tools/perf/util/hist.h            | 28 ++++++++++++-------------
 8 files changed, 58 insertions(+), 67 deletions(-)
tools/perf/builtin-annotate.c +1 -1
index cb234765ce3d..dc870cf31b79 100644
@@ -139,7 +139,7 @@ find_next:
 		}
 
 		if (use_browser > 0) {
-			key = hist_entry__tui_annotate(he, evidx, NULL, NULL, 0);
+			key = hist_entry__tui_annotate(he, evidx, NULL);
 			switch (key) {
 			case K_RIGHT:
 				next = rb_next(nd);
tools/perf/builtin-report.c +2 -2
index f07eae73e692..234f34d466e3 100644
@@ -428,10 +428,10 @@ static int __cmd_report(struct perf_report *rep)
 	if (use_browser > 0) {
 		if (use_browser == 1) {
 			perf_evlist__tui_browse_hists(session->evlist, help,
-						      NULL, NULL, 0);
+						      NULL);
 		} else if (use_browser == 2) {
 			perf_evlist__gtk_browse_hists(session->evlist, help,
-						      NULL, NULL, 0);
+						      NULL);
 		}
 	} else
 		perf_evlist__tty_browse_hists(session->evlist, rep, help);
tools/perf/builtin-top.c +6 -3
index f2ecd498c72d..102b43c9905d 100644
@@ -582,6 +582,11 @@ static void *display_thread_tui(void *arg)
 	struct perf_evsel *pos;
 	struct perf_top *top = arg;
 	const char *help = "For a higher level overview, try: perf top --sort comm,dso";
+	struct hist_browser_timer hbt = {
+		.timer		= perf_top__sort_new_samples,
+		.arg		= top,
+		.refresh	= top->delay_secs,
+	};
 
 	perf_top__sort_new_samples(top);
 
@@ -593,9 +598,7 @@ static void *display_thread_tui(void *arg)
 	list_for_each_entry(pos, &top->evlist->entries, node)
 		pos->hists.uid_filter_str = top->target.uid_str;
 
-	perf_evlist__tui_browse_hists(top->evlist, help,
-				      perf_top__sort_new_samples,
-				      top, top->delay_secs);
+	perf_evlist__tui_browse_hists(top->evlist, help, &hbt);
 
 	exit_browser(0);
 	exit(0);
tools/perf/ui/browsers/annotate.c +12 -15
index 28f8aab73aee..3eff17f703f3 100644
@@ -386,9 +386,8 @@ static void annotate_browser__init_asm_mode(struct annotate_browser *browser)
 	browser->b.nr_entries = browser->nr_asm_entries;
 }
 
-static bool annotate_browser__callq(struct annotate_browser *browser,
-				    int evidx, void (*timer)(void *arg),
-				    void *arg, int delay_secs)
+static bool annotate_browser__callq(struct annotate_browser *browser, int evidx,
+				    struct hist_browser_timer *hbt)
 {
 	struct map_symbol *ms = browser->b.priv;
 	struct disasm_line *dl = browser->selection;
@@ -418,7 +417,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	}
 
 	pthread_mutex_unlock(&notes->lock);
-	symbol__tui_annotate(target, ms->map, evidx, timer, arg, delay_secs);
+	symbol__tui_annotate(target, ms->map, evidx, hbt);
 	ui_browser__show_title(&browser->b, sym->name);
 	return true;
 }
@@ -602,13 +601,13 @@ static void annotate_browser__update_addr_width(struct annotate_browser *browser
 }
 
 static int annotate_browser__run(struct annotate_browser *browser, int evidx,
-				 void(*timer)(void *arg),
-				 void *arg, int delay_secs)
+				 struct hist_browser_timer *hbt)
 {
 	struct rb_node *nd = NULL;
 	struct map_symbol *ms = browser->b.priv;
 	struct symbol *sym = ms->sym;
 	const char *help = "Press 'h' for help on key bindings";
+	int delay_secs = hbt ? hbt->refresh : 0;
 	int key;
 
 	if (ui_browser__show(&browser->b, sym->name, help) < 0)
@@ -639,8 +638,8 @@ static int annotate_browser__run(struct annotate_browser *browser, int evidx,
 
 		switch (key) {
 		case K_TIMER:
-			if (timer != NULL)
-				timer(arg);
+			if (hbt)
+				hbt->timer(hbt->arg);
 
 			if (delay_secs != 0)
 				symbol__annotate_decay_histogram(sym, evidx);
@@ -740,7 +739,7 @@ show_help:
 					goto show_sup_ins;
 				goto out;
 			} else if (!(annotate_browser__jump(browser) ||
-				     annotate_browser__callq(browser, evidx, timer, arg, delay_secs))) {
+				     annotate_browser__callq(browser, evidx, hbt))) {
 show_sup_ins:
 				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
 			}
@@ -763,10 +762,9 @@ out:
 }
 
 int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
-			     void(*timer)(void *arg), void *arg, int delay_secs)
+			     struct hist_browser_timer *hbt)
 {
-	return symbol__tui_annotate(he->ms.sym, he->ms.map, evidx,
-				    timer, arg, delay_secs);
+	return symbol__tui_annotate(he->ms.sym, he->ms.map, evidx, hbt);
 }
 
 static void annotate_browser__mark_jump_targets(struct annotate_browser *browser,
@@ -816,8 +814,7 @@ static inline int width_jumps(int n)
 }
 
 int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
-			 void(*timer)(void *arg), void *arg,
-			 int delay_secs)
+			 struct hist_browser_timer *hbt)
 {
 	struct disasm_line *pos, *n;
 	struct annotation *notes;
@@ -899,7 +896,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 
 	annotate_browser__update_addr_width(&browser);
 
-	ret = annotate_browser__run(&browser, evidx, timer, arg, delay_secs);
+	ret = annotate_browser__run(&browser, evidx, hbt);
 	list_for_each_entry_safe(pos, n, &notes->src->source, node) {
 		list_del(&pos->node);
 		disasm_line__free(pos);
tools/perf/ui/browsers/hists.c +19 -24

133 lines not on this page · raw message

tools/perf/ui/gtk/browser.c +1 -3
index 4125c6284114..253b6219a39e 100644
@@ -237,9 +237,7 @@ static GtkWidget *perf_gtk__setup_statusbar(void)
 
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 				  const char *help,
-				  void (*timer) (void *arg)__maybe_unused,
-				  void *arg __maybe_unused,
-				  int delay_secs __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused)
 {
 	struct perf_evsel *pos;
 	GtkWidget *vbox;
tools/perf/util/annotate.h +4 -4
index a4dd25a61a07..c6272011625a 100644
@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include "types.h"
 #include "symbol.h"
+#include "hist.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <pthread.h>
@@ -140,14 +141,13 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 
 #ifdef NEWT_SUPPORT
 int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
-			 void(*timer)(void *arg), void *arg, int delay_secs);
+			 struct hist_browser_timer *hbt);
 #else
 static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
 				       struct map *map __maybe_unused,
 				       int evidx __maybe_unused,
-				       void(*timer)(void *arg) __maybe_unused,
-				       void *arg __maybe_unused,
-				       int delay_secs __maybe_unused)
+				       struct hist_browser_timer *hbt
+				       __maybe_unused)
 {
 	return 0;
 }
tools/perf/util/hist.h +13 -15
index b87460971736..96664cce7c7b 100644
@@ -157,22 +157,25 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
 
 struct perf_evlist;
 
+struct hist_browser_timer {
+	void (*timer)(void *arg);
+	void *arg;
+	int refresh;
+};
+
 #ifdef NEWT_SUPPORT
 #include "../ui/keysyms.h"
 int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
-			     void(*timer)(void *arg), void *arg, int delay_secs);
+			     struct hist_browser_timer *hbt);
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
-				  void(*timer)(void *arg), void *arg,
-				  int refresh);
+				  struct hist_browser_timer *hbt);
 int script_browse(const char *script_opt);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
-				  void(*timer)(void *arg) __maybe_unused,
-				  void *arg __maybe_unused,
-				  int refresh __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused)
 {
 	return 0;
 }
@@ -180,10 +183,8 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 static inline int hist_entry__tui_annotate(struct hist_entry *self
 					   __maybe_unused,
 					   int evidx __maybe_unused,
-					   void(*timer)(void *arg)
-					   __maybe_unused,
-					   void *arg __maybe_unused,
-					   int delay_secs __maybe_unused)
+					   struct hist_browser_timer *hbt
+					   __maybe_unused)
 {
 	return 0;
 }
@@ -199,15 +200,12 @@ static inline int script_browse(const char *script_opt)
 
 #ifdef GTK2_SUPPORT
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist, const char *help,
-				  void(*timer)(void *arg), void *arg,
-				  int refresh);
+				  struct hist_browser_timer *hbt __maybe_unused);
 #else
 static inline
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
-				  void(*timer)(void *arg) __maybe_unused,
-				  void *arg __maybe_unused,
-				  int refresh __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused)
 {
 	return 0;
 }
Signature · 1 line
1.7.11.7
Tested-by on v1 2/3
#2 lore raw

Re: [PATCH 2/3] perf tools: Introduce struct hist_browser_timer

Quoted 13 lines · Namhyung Kim

From: Namhyung Kim <[email protected]>

Currently various hist browser functions receive 3 arguments for refreshing histogram but only used from a few places. Also it's only for perf top command so that it can be NULL for other (and probably most) cases. Pack them into a struct in order to reduce number of those unused arguments.

This is a mechanical change and does not intend a functional change.

Cc David Ahern <[email protected]>
Cc Irina Tirdea <[email protected]>
Quoted 10 lines
---
  tools/perf/builtin-annotate.c     |  2 +-
  tools/perf/builtin-report.c       |  4 ++--
  tools/perf/builtin-top.c          |  9 +++++---
  tools/perf/ui/browsers/annotate.c | 27 +++++++++++-------------
  tools/perf/ui/browsers/hists.c    | 43 +++++++++++++++++----------------------
  tools/perf/ui/gtk/browser.c       |  4 +---
  tools/perf/util/annotate.h        |  8 ++++----
  tools/perf/util/hist.h            | 28 ++++++++++++-------------
  8 files changed, 58 insertions(+), 67 deletions(-)
#3 lore raw

[tip:perf/core] perf tools: Introduce struct hist_browser_timer

In reply to Namhyung Kim
Commit-ID:  9783adf777a445a1e9d0db4857a3a896a9f42d4a
Gitweb:     http://git.kernel.org/tip/9783adf777a445a1e9d0db4857a3a896a9f42d4a
Author:     Namhyung Kim <[email protected]>
AuthorDate: Fri, 2 Nov 2012 14:50:05 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Nov 2012 14:03:58 -0300

perf tools: Introduce struct hist_browser_timer

Currently various hist browser functions receive 3 arguments for refreshing histogram but only used from a few places. Also it's only for perf top command so that it can be NULL for other (and probably most) cases. Pack them into a struct in order to reduce number of those unused arguments.

This is a mechanical change and does not intend a functional change.

---
 tools/perf/builtin-annotate.c     |    2 +-
 tools/perf/builtin-report.c       |    4 +-
 tools/perf/builtin-top.c          |    9 +++++--
 tools/perf/ui/browsers/annotate.c |   27 ++++++++++-------------
 tools/perf/ui/browsers/hists.c    |   43 ++++++++++++++++--------------------
 tools/perf/ui/gtk/browser.c       |    4 +--
 tools/perf/util/annotate.h        |    8 +++---
 tools/perf/util/hist.h            |   28 +++++++++++-------------
 8 files changed, 58 insertions(+), 67 deletions(-)
tools/perf/builtin-annotate.c +1 -1
index cb23476..dc870cf 100644
@@ -139,7 +139,7 @@ find_next:
 		}
 
 		if (use_browser > 0) {
-			key = hist_entry__tui_annotate(he, evidx, NULL, NULL, 0);
+			key = hist_entry__tui_annotate(he, evidx, NULL);
 			switch (key) {
 			case K_RIGHT:
 				next = rb_next(nd);
tools/perf/builtin-report.c +2 -2
index f07eae7..234f34d 100644
@@ -428,10 +428,10 @@ static int __cmd_report(struct perf_report *rep)
 	if (use_browser > 0) {
 		if (use_browser == 1) {
 			perf_evlist__tui_browse_hists(session->evlist, help,
-						      NULL, NULL, 0);
+						      NULL);
 		} else if (use_browser == 2) {
 			perf_evlist__gtk_browse_hists(session->evlist, help,
-						      NULL, NULL, 0);
+						      NULL);
 		}
 	} else
 		perf_evlist__tty_browse_hists(session->evlist, rep, help);
tools/perf/builtin-top.c +6 -3
index f2ecd49..102b43c 100644
@@ -582,6 +582,11 @@ static void *display_thread_tui(void *arg)
 	struct perf_evsel *pos;
 	struct perf_top *top = arg;
 	const char *help = "For a higher level overview, try: perf top --sort comm,dso";
+	struct hist_browser_timer hbt = {
+		.timer		= perf_top__sort_new_samples,
+		.arg		= top,
+		.refresh	= top->delay_secs,
+	};
 
 	perf_top__sort_new_samples(top);
 
@@ -593,9 +598,7 @@ static void *display_thread_tui(void *arg)
 	list_for_each_entry(pos, &top->evlist->entries, node)
 		pos->hists.uid_filter_str = top->target.uid_str;
 
-	perf_evlist__tui_browse_hists(top->evlist, help,
-				      perf_top__sort_new_samples,
-				      top, top->delay_secs);
+	perf_evlist__tui_browse_hists(top->evlist, help, &hbt);
 
 	exit_browser(0);
 	exit(0);
tools/perf/ui/browsers/annotate.c +12 -15
index 28f8aab..3eff17f 100644
@@ -386,9 +386,8 @@ static void annotate_browser__init_asm_mode(struct annotate_browser *browser)
 	browser->b.nr_entries = browser->nr_asm_entries;
 }
 
-static bool annotate_browser__callq(struct annotate_browser *browser,
-				    int evidx, void (*timer)(void *arg),
-				    void *arg, int delay_secs)
+static bool annotate_browser__callq(struct annotate_browser *browser, int evidx,
+				    struct hist_browser_timer *hbt)
 {
 	struct map_symbol *ms = browser->b.priv;
 	struct disasm_line *dl = browser->selection;
@@ -418,7 +417,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	}
 
 	pthread_mutex_unlock(&notes->lock);
-	symbol__tui_annotate(target, ms->map, evidx, timer, arg, delay_secs);
+	symbol__tui_annotate(target, ms->map, evidx, hbt);
 	ui_browser__show_title(&browser->b, sym->name);
 	return true;
 }
@@ -602,13 +601,13 @@ static void annotate_browser__update_addr_width(struct annotate_browser *browser
 }
 
 static int annotate_browser__run(struct annotate_browser *browser, int evidx,
-				 void(*timer)(void *arg),
-				 void *arg, int delay_secs)
+				 struct hist_browser_timer *hbt)
 {
 	struct rb_node *nd = NULL;
 	struct map_symbol *ms = browser->b.priv;
 	struct symbol *sym = ms->sym;
 	const char *help = "Press 'h' for help on key bindings";
+	int delay_secs = hbt ? hbt->refresh : 0;
 	int key;
 
 	if (ui_browser__show(&browser->b, sym->name, help) < 0)
@@ -639,8 +638,8 @@ static int annotate_browser__run(struct annotate_browser *browser, int evidx,
 
 		switch (key) {
 		case K_TIMER:
-			if (timer != NULL)
-				timer(arg);
+			if (hbt)
+				hbt->timer(hbt->arg);
 
 			if (delay_secs != 0)
 				symbol__annotate_decay_histogram(sym, evidx);
@@ -740,7 +739,7 @@ show_help:
 					goto show_sup_ins;
 				goto out;
 			} else if (!(annotate_browser__jump(browser) ||
-				     annotate_browser__callq(browser, evidx, timer, arg, delay_secs))) {
+				     annotate_browser__callq(browser, evidx, hbt))) {
 show_sup_ins:
 				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
 			}
@@ -763,10 +762,9 @@ out:
 }
 
 int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
-			     void(*timer)(void *arg), void *arg, int delay_secs)
+			     struct hist_browser_timer *hbt)
 {
-	return symbol__tui_annotate(he->ms.sym, he->ms.map, evidx,
-				    timer, arg, delay_secs);
+	return symbol__tui_annotate(he->ms.sym, he->ms.map, evidx, hbt);
 }
 
 static void annotate_browser__mark_jump_targets(struct annotate_browser *browser,
@@ -816,8 +814,7 @@ static inline int width_jumps(int n)
 }
 
 int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
-			 void(*timer)(void *arg), void *arg,
-			 int delay_secs)
+			 struct hist_browser_timer *hbt)
 {
 	struct disasm_line *pos, *n;
 	struct annotation *notes;
@@ -899,7 +896,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 
 	annotate_browser__update_addr_width(&browser);
 
-	ret = annotate_browser__run(&browser, evidx, timer, arg, delay_secs);
+	ret = annotate_browser__run(&browser, evidx, hbt);
 	list_for_each_entry_safe(pos, n, &notes->src->source, node) {
 		list_del(&pos->node);
 		disasm_line__free(pos);
tools/perf/ui/browsers/hists.c +19 -24

133 lines not on this page · raw message

tools/perf/ui/gtk/browser.c +1 -3
index 4125c62..253b621 100644
@@ -237,9 +237,7 @@ static GtkWidget *perf_gtk__setup_statusbar(void)
 
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 				  const char *help,
-				  void (*timer) (void *arg)__maybe_unused,
-				  void *arg __maybe_unused,
-				  int delay_secs __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused)
 {
 	struct perf_evsel *pos;
 	GtkWidget *vbox;
tools/perf/util/annotate.h +4 -4
index a4dd25a..c627201 100644
@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include "types.h"
 #include "symbol.h"
+#include "hist.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <pthread.h>
@@ -140,14 +141,13 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 
 #ifdef NEWT_SUPPORT
 int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
-			 void(*timer)(void *arg), void *arg, int delay_secs);
+			 struct hist_browser_timer *hbt);
 #else
 static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
 				       struct map *map __maybe_unused,
 				       int evidx __maybe_unused,
-				       void(*timer)(void *arg) __maybe_unused,
-				       void *arg __maybe_unused,
-				       int delay_secs __maybe_unused)
+				       struct hist_browser_timer *hbt
+				       __maybe_unused)
 {
 	return 0;
 }
tools/perf/util/hist.h +13 -15
index b874609..96664cc 100644
@@ -157,22 +157,25 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
 
 struct perf_evlist;
 
+struct hist_browser_timer {
+	void (*timer)(void *arg);
+	void *arg;
+	int refresh;
+};
+
 #ifdef NEWT_SUPPORT
 #include "../ui/keysyms.h"
 int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
-			     void(*timer)(void *arg), void *arg, int delay_secs);
+			     struct hist_browser_timer *hbt);
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
-				  void(*timer)(void *arg), void *arg,
-				  int refresh);
+				  struct hist_browser_timer *hbt);
 int script_browse(const char *script_opt);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
-				  void(*timer)(void *arg) __maybe_unused,
-				  void *arg __maybe_unused,
-				  int refresh __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused)
 {
 	return 0;
 }
@@ -180,10 +183,8 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 static inline int hist_entry__tui_annotate(struct hist_entry *self
 					   __maybe_unused,
 					   int evidx __maybe_unused,
-					   void(*timer)(void *arg)
-					   __maybe_unused,
-					   void *arg __maybe_unused,
-					   int delay_secs __maybe_unused)
+					   struct hist_browser_timer *hbt
+					   __maybe_unused)
 {
 	return 0;
 }
@@ -199,15 +200,12 @@ static inline int script_browse(const char *script_opt)
 
 #ifdef GTK2_SUPPORT
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist, const char *help,
-				  void(*timer)(void *arg), void *arg,
-				  int refresh);
+				  struct hist_browser_timer *hbt __maybe_unused);
 #else
 static inline
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
-				  void(*timer)(void *arg) __maybe_unused,
-				  void *arg __maybe_unused,
-				  int refresh __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused)
 {
 	return 0;
 }
author maintainer
#4 lore raw

PATCH 3/3 perf report: Postpone objdump check until annotation requested

In reply to Namhyung Kim

From: Namhyung Kim <[email protected]>

David reported that current perf report refused to run on a data file captured from a different machine because of objdump. Since the objdump tools won't be used unless annotation was requested, checking its presence at init time doens't make sense.

---
 tools/perf/builtin-report.c    |  9 ++-------
 tools/perf/builtin-top.c       |  3 ++-
 tools/perf/ui/browsers/hists.c | 22 ++++++++++++++++------
 tools/perf/util/hist.h         |  7 +++++--
 4 files changed, 25 insertions(+), 16 deletions(-)
tools/perf/builtin-report.c +2 -7
index 234f34d466e3..fc251005dd3d 100644
@@ -428,7 +428,8 @@ static int __cmd_report(struct perf_report *rep)
 	if (use_browser > 0) {
 		if (use_browser == 1) {
 			perf_evlist__tui_browse_hists(session->evlist, help,
-						      NULL);
+						      NULL,
+						      &session->header.env);
 		} else if (use_browser == 2) {
 			perf_evlist__gtk_browse_hists(session->evlist, help,
 						      NULL);
@@ -672,12 +673,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
-	if (!objdump_path) {
-		ret = perf_session_env__lookup_objdump(&session->header.env);
-		if (ret)
-			goto error;
-	}
-
 	if (sort__branch_mode == -1 && has_br_stack)
 		sort__branch_mode = 1;
 
tools/perf/builtin-top.c +2 -1
index 102b43c9905d..c9ff3950cd4b 100644
@@ -598,7 +598,8 @@ static void *display_thread_tui(void *arg)
 	list_for_each_entry(pos, &top->evlist->entries, node)
 		pos->hists.uid_filter_str = top->target.uid_str;
 
-	perf_evlist__tui_browse_hists(top->evlist, help, &hbt);
+	perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
+				      &top->session->header.env);
 
 	exit_browser(0);
 	exit(0);
tools/perf/ui/browsers/hists.c +16 -6
index c7d32edb8057..ccc4bd161420 100644
@@ -11,6 +11,7 @@
 #include "../../util/pstack.h"
 #include "../../util/sort.h"
 #include "../../util/util.h"
+#include "../../arch/common.h"
 
 #include "../browser.h"
 #include "../helpline.h"
@@ -1137,7 +1138,8 @@ static inline bool is_report_browser(void *timer)
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				    const char *helpline, const char *ev_name,
 				    bool left_exits,
-				    struct hist_browser_timer *hbt)
+				    struct hist_browser_timer *hbt,
+				    struct perf_session_env *env)
 {
 	struct hists *hists = &evsel->hists;
 	struct hist_browser *browser = hist_browser__new(hists);
@@ -1367,6 +1369,9 @@ retry_popup_menu:
 			struct hist_entry *he;
 			int err;
 do_annotate:
+			if (!objdump_path && perf_session_env__lookup_objdump(env))
+				continue;
+
 			he = hist_browser__selected_entry(browser);
 			if (he == NULL)
 				continue;
@@ -1470,6 +1475,7 @@ struct perf_evsel_menu {
 	struct ui_browser b;
 	struct perf_evsel *selection;
 	bool lost_events, lost_events_warned;
+	struct perf_session_env *env;
 };
 
 static void perf_evsel_menu__write(struct ui_browser *browser,
@@ -1551,7 +1557,8 @@ browse_hists:
 				hbt->timer(hbt->arg);
 			ev_name = perf_evsel__name(pos);
 			key = perf_evsel__hists_browse(pos, nr_events, help,
-						       ev_name, true, hbt);
+						       ev_name, true, hbt,
+						       menu->env);
 			ui_browser__show_title(&menu->b, title);
 			switch (key) {
 			case K_TAB:
@@ -1599,7 +1606,8 @@ out:
 
 static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 					   const char *help,
-					   struct hist_browser_timer *hbt)
+					   struct hist_browser_timer *hbt,
+					   struct perf_session_env *env)
 {
 	struct perf_evsel *pos;
 	struct perf_evsel_menu menu = {
@@ -1611,6 +1619,7 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 			.nr_entries = evlist->nr_entries,
 			.priv	    = evlist,
 		},
+		.env = env,
 	};
 
 	ui_helpline__push("Press ESC to exit");
@@ -1627,15 +1636,16 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 }
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
-				  struct hist_browser_timer *hbt)
+				  struct hist_browser_timer *hbt,
+				  struct perf_session_env *env)
 {
 	if (evlist->nr_entries == 1) {
 		struct perf_evsel *first = list_entry(evlist->entries.next,
 						      struct perf_evsel, node);
 		const char *ev_name = perf_evsel__name(first);
 		return perf_evsel__hists_browse(first, evlist->nr_entries, help,
-						ev_name, false, hbt);
+						ev_name, false, hbt, env);
 	}
 
-	return __perf_evlist__tui_browse_hists(evlist, help, hbt);
+	return __perf_evlist__tui_browse_hists(evlist, help, hbt, env);
 }
tools/perf/util/hist.h +5 -2
index 96664cce7c7b..a4f45dd04697 100644
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 #include <pthread.h>
 #include "callchain.h"
+#include "header.h"
 
 extern struct callchain_param callchain_param;
 
@@ -169,13 +170,15 @@ int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
 			     struct hist_browser_timer *hbt);
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
-				  struct hist_browser_timer *hbt);
+				  struct hist_browser_timer *hbt,
+				  struct perf_session_env *env);
 int script_browse(const char *script_opt);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
-				  struct hist_browser_timer *hbt __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused,
+				  struct perf_session_env *env __maybe_unused)
 {
 	return 0;
 }
Signature · 1 line
1.7.11.7
#5 lore raw

Re: [PATCH 3/3] perf report: Postpone objdump check until annotation requested

Quoted 10 lines · Namhyung Kim

From: Namhyung Kim <[email protected]>

David reported that current perf report refused to run on a data file captured from a different machine because of objdump. Since the objdump tools won't be used unless annotation was requested, checking its presence at init time doens't make sense.

Quoted 6 lines
---
  tools/perf/builtin-report.c    |  9 ++-------
  tools/perf/builtin-top.c       |  3 ++-
  tools/perf/ui/browsers/hists.c | 22 ++++++++++++++++------
  tools/perf/util/hist.h         |  7 +++++--
  4 files changed, 25 insertions(+), 16 deletions(-)

Reviewed-by/Tested-by: David Ahern <[email protected]>

#6 lore raw

[tip:perf/core] perf report: Postpone objdump check until annotation requested

In reply to Namhyung Kim
Commit-ID:  68d807586ba83d9cb77f12c8fb7c97ea438d34ad
Gitweb:     http://git.kernel.org/tip/68d807586ba83d9cb77f12c8fb7c97ea438d34ad
Author:     Namhyung Kim <[email protected]>
AuthorDate: Fri, 2 Nov 2012 14:50:06 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Nov 2012 14:03:58 -0300

perf report: Postpone objdump check until annotation requested

David reported that current perf report refused to run on a data file captured from a different machine because of objdump.

Since the objdump tools won't be used unless annotation was requested, checking its presence at init time doesn't make sense.

---
 tools/perf/builtin-report.c    |    9 ++-------
 tools/perf/builtin-top.c       |    3 ++-
 tools/perf/ui/browsers/hists.c |   22 ++++++++++++++++------
 tools/perf/util/hist.h         |    7 +++++--
 4 files changed, 25 insertions(+), 16 deletions(-)
tools/perf/builtin-report.c +2 -7
index 234f34d..fc25100 100644
@@ -428,7 +428,8 @@ static int __cmd_report(struct perf_report *rep)
 	if (use_browser > 0) {
 		if (use_browser == 1) {
 			perf_evlist__tui_browse_hists(session->evlist, help,
-						      NULL);
+						      NULL,
+						      &session->header.env);
 		} else if (use_browser == 2) {
 			perf_evlist__gtk_browse_hists(session->evlist, help,
 						      NULL);
@@ -672,12 +673,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
-	if (!objdump_path) {
-		ret = perf_session_env__lookup_objdump(&session->header.env);
-		if (ret)
-			goto error;
-	}
-
 	if (sort__branch_mode == -1 && has_br_stack)
 		sort__branch_mode = 1;
 
tools/perf/builtin-top.c +2 -1
index 102b43c..c9ff395 100644
@@ -598,7 +598,8 @@ static void *display_thread_tui(void *arg)
 	list_for_each_entry(pos, &top->evlist->entries, node)
 		pos->hists.uid_filter_str = top->target.uid_str;
 
-	perf_evlist__tui_browse_hists(top->evlist, help, &hbt);
+	perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
+				      &top->session->header.env);
 
 	exit_browser(0);
 	exit(0);
tools/perf/ui/browsers/hists.c +16 -6
index c7d32ed..ccc4bd1 100644
@@ -11,6 +11,7 @@
 #include "../../util/pstack.h"
 #include "../../util/sort.h"
 #include "../../util/util.h"
+#include "../../arch/common.h"
 
 #include "../browser.h"
 #include "../helpline.h"
@@ -1137,7 +1138,8 @@ static inline bool is_report_browser(void *timer)
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				    const char *helpline, const char *ev_name,
 				    bool left_exits,
-				    struct hist_browser_timer *hbt)
+				    struct hist_browser_timer *hbt,
+				    struct perf_session_env *env)
 {
 	struct hists *hists = &evsel->hists;
 	struct hist_browser *browser = hist_browser__new(hists);
@@ -1367,6 +1369,9 @@ retry_popup_menu:
 			struct hist_entry *he;
 			int err;
 do_annotate:
+			if (!objdump_path && perf_session_env__lookup_objdump(env))
+				continue;
+
 			he = hist_browser__selected_entry(browser);
 			if (he == NULL)
 				continue;
@@ -1470,6 +1475,7 @@ struct perf_evsel_menu {
 	struct ui_browser b;
 	struct perf_evsel *selection;
 	bool lost_events, lost_events_warned;
+	struct perf_session_env *env;
 };
 
 static void perf_evsel_menu__write(struct ui_browser *browser,
@@ -1551,7 +1557,8 @@ browse_hists:
 				hbt->timer(hbt->arg);
 			ev_name = perf_evsel__name(pos);
 			key = perf_evsel__hists_browse(pos, nr_events, help,
-						       ev_name, true, hbt);
+						       ev_name, true, hbt,
+						       menu->env);
 			ui_browser__show_title(&menu->b, title);
 			switch (key) {
 			case K_TAB:
@@ -1599,7 +1606,8 @@ out:
 
 static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 					   const char *help,
-					   struct hist_browser_timer *hbt)
+					   struct hist_browser_timer *hbt,
+					   struct perf_session_env *env)
 {
 	struct perf_evsel *pos;
 	struct perf_evsel_menu menu = {
@@ -1611,6 +1619,7 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 			.nr_entries = evlist->nr_entries,
 			.priv	    = evlist,
 		},
+		.env = env,
 	};
 
 	ui_helpline__push("Press ESC to exit");
@@ -1627,15 +1636,16 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 }
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
-				  struct hist_browser_timer *hbt)
+				  struct hist_browser_timer *hbt,
+				  struct perf_session_env *env)
 {
 	if (evlist->nr_entries == 1) {
 		struct perf_evsel *first = list_entry(evlist->entries.next,
 						      struct perf_evsel, node);
 		const char *ev_name = perf_evsel__name(first);
 		return perf_evsel__hists_browse(first, evlist->nr_entries, help,
-						ev_name, false, hbt);
+						ev_name, false, hbt, env);
 	}
 
-	return __perf_evlist__tui_browse_hists(evlist, help, hbt);
+	return __perf_evlist__tui_browse_hists(evlist, help, hbt, env);
 }
tools/perf/util/hist.h +5 -2
index 96664cc..a4f45dd 100644
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 #include <pthread.h>
 #include "callchain.h"
+#include "header.h"
 
 extern struct callchain_param callchain_param;
 
@@ -169,13 +170,15 @@ int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
 			     struct hist_browser_timer *hbt);
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
-				  struct hist_browser_timer *hbt);
+				  struct hist_browser_timer *hbt,
+				  struct perf_session_env *env);
 int script_browse(const char *script_opt);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
-				  struct hist_browser_timer *hbt __maybe_unused)
+				  struct hist_browser_timer *hbt __maybe_unused,
+				  struct perf_session_env *env __maybe_unused)
 {
 	return 0;
 }
#7 lore raw
In reply to Namhyung Kim
Quoted 9 lines · Namhyung Kim

From: Namhyung Kim <[email protected]>

David reported that perf report for i686 target data on x86_64 host failed to work because it tried to find out cross-compiled objdump.

However objdump for x86_64 is compatible to i686 so that it doesn't need to do it at all. To prevent similar artifacts, normalize arch name when comparing host and file architectures.

Quoted 7 lines
---
  tools/perf/arch/common.c | 40 +++++++++++++++++++++++++++++++++-------
  1 file changed, 33 insertions(+), 7 deletions(-)

Reviewed-by/Tested-by: David Ahern <[email protected]>

#8 lore raw
In reply to Namhyung Kim
Commit-ID:  48ed0ece1b8063313284812ef048b26c3c4250af
Gitweb:     http://git.kernel.org/tip/48ed0ece1b8063313284812ef048b26c3c4250af
Author:     Namhyung Kim <[email protected]>
AuthorDate: Fri, 2 Nov 2012 14:50:04 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Nov 2012 14:03:58 -0300

perf tools: Use normalized arch name for searching objdump path

David reported that perf report for i686 target data on x86_64 host failed to work because it tried to find out cross-compiled objdump.

However objdump for x86_64 is compatible to i686 so that it doesn't need to do it at all. To prevent similar artifacts, normalize arch name when comparing host and file architectures.

---
 tools/perf/arch/common.c |   40 +++++++++++++++++++++++++++++++++-------
 1 files changed, 33 insertions(+), 7 deletions(-)
tools/perf/arch/common.c +33 -7
index 2367b25..5683529 100644
@@ -93,16 +93,46 @@ static int lookup_triplets(const char *const *triplets, const char *name)
 	return -1;
 }
 
+/*
+ * Return architecture name in a normalized form.
+ * The conversion logic comes from the Makefile.
+ */
+static const char *normalize_arch(char *arch)
+{
+	if (!strcmp(arch, "x86_64"))
+		return "x86";
+	if (arch[0] == 'i' && arch[2] == '8' && arch[3] == '6')
+		return "x86";
+	if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5))
+		return "sparc";
+	if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110"))
+		return "arm";
+	if (!strncmp(arch, "s390", 4))
+		return "s390";
+	if (!strncmp(arch, "parisc", 6))
+		return "parisc";
+	if (!strncmp(arch, "powerpc", 7) || !strncmp(arch, "ppc", 3))
+		return "powerpc";
+	if (!strncmp(arch, "mips", 4))
+		return "mips";
+	if (!strncmp(arch, "sh", 2) && isdigit(arch[2]))
+		return "sh";
+
+	return arch;
+}
+
 static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 						  const char *name,
 						  const char **path)
 {
 	int idx;
-	char *arch, *cross_env;
+	const char *arch, *cross_env;
 	struct utsname uts;
 	const char *const *path_list;
 	char *buf = NULL;
 
+	arch = normalize_arch(env->arch);
+
 	if (uname(&uts) < 0)
 		goto out;
 
@@ -110,7 +140,7 @@ static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 	 * We don't need to try to find objdump path for native system.
 	 * Just use default binutils path (e.g.: "objdump").
 	 */
-	if (!strcmp(uts.machine, env->arch))
+	if (!strcmp(normalize_arch(uts.machine), arch))
 		goto out;
 
 	cross_env = getenv("CROSS_COMPILE");
@@ -127,8 +157,6 @@ static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 		free(buf);
 	}
 
-	arch = env->arch;
-
 	if (!strcmp(arch, "arm"))
 		path_list = arm_triplets;
 	else if (!strcmp(arch, "powerpc"))
@@ -139,9 +167,7 @@ static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
 		path_list = s390_triplets;
 	else if (!strcmp(arch, "sparc"))
 		path_list = sparc_triplets;
-	else if (!strcmp(arch, "x86") || !strcmp(arch, "i386") ||
-		 !strcmp(arch, "i486") || !strcmp(arch, "i586") ||
-		 !strcmp(arch, "i686"))
+	else if (!strcmp(arch, "x86"))
 		path_list = x86_triplets;
 	else if (!strcmp(arch, "mips"))
 		path_list = mips_triplets;