2012-10-15 23:34:00

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 0/6] perf tools: fixes for Android

From: Irina Tirdea <[email protected]>

This is version 4 for the Android fixes.
Thanks for the review! This version contains fixes for issues raised by Arnaldo [1].

Changes v3->v4:
() patch 1: fixed compilation issue when calling make from kernel root directory
() patch 4: renamed try_binutils_path to perf_session_env__lookup_binutils_path
() patch 4: replaced scnprintf + strdup with asprintf
() patch 4: handle errors and print messages in case cross-built objdump is not
found
() patch 3,5: split patch with addr2line changes in 2 patches
() patch 6: set default ret to -ENOMEM in order to avoid setting it every time

[1] https://lkml.org/lkml/2012/10/8/45

Irina Tirdea (5):
perf tools: configure tmp path at build time
perf tools: configure shell path at compile time
perf tools: add --addr2line command line option
perf tools: Try to find cross-built addr2line path
perf stat: implement --big-num grouping

Namhyung Kim (1):
perf tools: Try to find cross-built objdump path

tools/perf/Documentation/android.txt | 19 +++
tools/perf/Documentation/jit-interface.txt | 4 +-
tools/perf/Documentation/perf-annotate.txt | 2 +
tools/perf/Documentation/perf-report.txt | 2 +
tools/perf/Makefile | 36 +++++-
tools/perf/arch/common.c | 184 ++++++++++++++++++++++++++++
tools/perf/arch/common.h | 12 ++
tools/perf/builtin-annotate.c | 16 +++
tools/perf/builtin-help.c | 2 +-
tools/perf/builtin-report.c | 16 +++
tools/perf/builtin-script.c | 12 +-
tools/perf/builtin-stat.c | 106 ++++++++++++++--
tools/perf/config/feature-tests.mak | 18 +++
tools/perf/perf-archive.sh | 13 +-
tools/perf/util/annotate.c | 13 +-
tools/perf/util/annotate.h | 1 -
tools/perf/util/dso-test-data.c | 2 +-
tools/perf/util/map.c | 3 +-
tools/perf/util/pmu.c | 2 +-
tools/perf/util/sort.c | 15 ++-
tools/perf/util/symbol.c | 4 +-
tools/perf/util/trace-event-info.c | 2 +-
tools/perf/util/vdso.c | 2 +-
23 files changed, 456 insertions(+), 30 deletions(-)
create mode 100644 tools/perf/arch/common.c
create mode 100644 tools/perf/arch/common.h

--
1.7.9.5


2012-10-15 23:34:35

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 1/6] perf tools: configure tmp path at build time

From: Irina Tirdea <[email protected]>

Temporary perf files are hardcoded to point to /tmp. Android does not have
a /tmp directory so it needs to set this path at compile time.

Add a compile-time definition (PERF_TMP_DIR) in the Makefile that sets the path
to temp directory. By default it points to /tmp.

Signed-off-by: Irina Tirdea <[email protected]>
---
tools/perf/Documentation/android.txt | 1 +
tools/perf/Documentation/jit-interface.txt | 4 +++-
tools/perf/Makefile | 20 +++++++++++++++++++-
tools/perf/perf-archive.sh | 13 +++++++++++--
tools/perf/util/dso-test-data.c | 2 +-
tools/perf/util/map.c | 3 ++-
tools/perf/util/pmu.c | 2 +-
tools/perf/util/symbol.c | 4 +++-
tools/perf/util/trace-event-info.c | 2 +-
tools/perf/util/vdso.c | 2 +-
10 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/tools/perf/Documentation/android.txt b/tools/perf/Documentation/android.txt
index a39dbbb..24fd01c 100644
--- a/tools/perf/Documentation/android.txt
+++ b/tools/perf/Documentation/android.txt
@@ -68,6 +68,7 @@ Some perf features need environment variables to run properly.
You need to set these before running perf on the target:
adb shell
# PERF_PAGER=cat
+ # PERF_TMP_DIR=/data/local/tmp

IV. Run perf
------------------------------------------------
diff --git a/tools/perf/Documentation/jit-interface.txt b/tools/perf/Documentation/jit-interface.txt
index a8656f5..10cb6ec 100644
--- a/tools/perf/Documentation/jit-interface.txt
+++ b/tools/perf/Documentation/jit-interface.txt
@@ -1,7 +1,9 @@
perf supports a simple JIT interface to resolve symbols for dynamic code generated
by a JIT.

-The JIT has to write a /tmp/perf-%d.map (%d = pid of process) file
+The JIT has to write a $PERF_TMP_DIR/perf-%d.map (%d = pid of process) file.
+You can set $PERF_TMP_DIR at compile time in the Makefile.
+By default it is /tmp.

This is a text file.

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 318bec8..005840b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -141,6 +141,7 @@ sysconfdir = $(prefix)/etc
ETC_PERFCONFIG = etc/perfconfig
endif
lib = lib
+PERF_TMP_DIR = /tmp

export prefix bindir sharedir sysconfdir

@@ -177,6 +178,7 @@ ifeq ($(call try-cc,$(SOURCE_BIONIC),$(CFLAGS)),y)
EXTLIBS := $(filter-out -lrt,$(EXTLIBS))
EXTLIBS := $(filter-out -lpthread,$(EXTLIBS))
BASIC_CFLAGS += -I.
+ PERF_TMP_DIR = /data/local/tmp
endif

# Guard against environment variables
@@ -255,7 +257,8 @@ $(OUTPUT)util/pmu-bison.c: util/pmu.y
$(QUIET_BISON)$(BISON) -v util/pmu.y -d -o $(OUTPUT)util/pmu-bison.c

$(OUTPUT)util/parse-events.o: $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-bison.c
-$(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c
+$(OUTPUT)util/pmu.o: util/pmu.c $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<

LIB_FILE=$(OUTPUT)libperf.a

@@ -889,6 +892,21 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
$(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<

+$(OUTPUT)util/dso-test-data.o: util/dso-test-data.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/map.o: util/map.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/symbol.o: util/symbol.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/trace-event-info.o: util/trace-event-info.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/vdso.o: util/vdso.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
$(OUTPUT)ui/browser.o: ui/browser.c $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<

diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index e919306..058c3b7 100644
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -18,7 +18,16 @@ else
PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
fi

-BUILDIDS=$(mktemp /tmp/perf-archive-buildids.XXXXXX)
+#
+# PERF_TMP_DIR environment variable set by perf
+# path to temp directory, default to /tmp
+#
+if [ -z $PERF_TMP_DIR ]; then
+ PERF_TMP_DIR=/tmp
+fi
+
+
+BUILDIDS=$(mktemp $PERF_TMP_DIR/perf-archive-buildids.XXXXXX)
NOBUILDID=0000000000000000000000000000000000000000

perf buildid-list -i $PERF_DATA --with-hits | grep -v "^$NOBUILDID " > $BUILDIDS
@@ -28,7 +37,7 @@ if [ ! -s $BUILDIDS ] ; then
exit 1
fi

-MANIFEST=$(mktemp /tmp/perf-archive-manifest.XXXXXX)
+MANIFEST=$(mktemp $PERF_TMP_DIR/perf-archive-manifest.XXXXXX)
PERF_BUILDID_LINKDIR=$(readlink -f $PERF_BUILDID_DIR)/

cut -d ' ' -f 1 $BUILDIDS | \
diff --git a/tools/perf/util/dso-test-data.c b/tools/perf/util/dso-test-data.c
index c6caede..ca81e65 100644
--- a/tools/perf/util/dso-test-data.c
+++ b/tools/perf/util/dso-test-data.c
@@ -18,7 +18,7 @@ do { \

static char *test_file(int size)
{
- static char buf_templ[] = "/tmp/test-XXXXXX";
+ static char buf_templ[] = PERF_TMP_DIR "/test-XXXXXX";
char *templ = buf_templ;
int fd, i;
unsigned char *buf;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 6109fa4..4e69b57 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -59,7 +59,8 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
no_dso = is_no_dso_memory(filename);

if (anon) {
- snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", pid);
+ snprintf(newfilename, sizeof(newfilename),
+ PERF_TMP_DIR "/perf-%d.map", pid);
filename = newfilename;
}

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8a2229d..5ebde18 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -637,7 +637,7 @@ static char *test_format_dir_get(void)
static char dir[PATH_MAX];
unsigned int i;

- snprintf(dir, PATH_MAX, "/tmp/perf-pmu-test-format-XXXXXX");
+ snprintf(dir, PATH_MAX, PERF_TMP_DIR "/perf-pmu-test-format-XXXXXX");
if (!mkdtemp(dir))
return NULL;

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e2e8c69..d8aca82 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1051,7 +1051,9 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)

dso->adjust_symbols = 0;

- if (strncmp(dso->name, "/tmp/perf-", 10) == 0) {
+ if (strncmp
+ (dso->name, PERF_TMP_DIR "/perf-",
+ strlen(PERF_TMP_DIR "/perf-")) == 0) {
struct stat st;

if (lstat(dso->name, &st) < 0)
diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index a8d81c3..e50ea61 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -481,7 +481,7 @@ struct tracing_data *tracing_data_get(struct list_head *pattrs,
int temp_fd;

snprintf(tdata->temp_file, sizeof(tdata->temp_file),
- "/tmp/perf-XXXXXX");
+ PERF_TMP_DIR "/perf-XXXXXX");
if (!mkstemp(tdata->temp_file))
die("Can't make temp file");

diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index e60951f..8b9613b 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -14,7 +14,7 @@
#include "linux/string.h"

static bool vdso_found;
-static char vdso_file[] = "/tmp/perf-vdso.so-XXXXXX";
+static char vdso_file[] = PERF_TMP_DIR "/perf-vdso.so-XXXXXX";

static int find_vdso_map(void **start, void **end)
{
--
1.7.9.5

2012-10-15 23:35:11

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 2/6] perf tools: configure shell path at compile time

From: Irina Tirdea <[email protected]>

Shell path /bin/sh is hardcoded in various places in perf. Android has a
different folder structure and does not have /bin/sh.

Set the shell path at compile time in the Makefile by setting PERF_SHELL_PATH.
By default it is set to /bin/sh.

Signed-off-by: Irina Tirdea <[email protected]>
---
tools/perf/Makefile | 8 +++++++-
tools/perf/builtin-help.c | 2 +-
tools/perf/builtin-script.c | 12 ++++++------
3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 005840b..99bf383 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -142,6 +142,7 @@ ETC_PERFCONFIG = etc/perfconfig
endif
lib = lib
PERF_TMP_DIR = /tmp
+PERF_SHELL_PATH = /bin/sh

export prefix bindir sharedir sysconfdir

@@ -179,6 +180,7 @@ ifeq ($(call try-cc,$(SOURCE_BIONIC),$(CFLAGS)),y)
EXTLIBS := $(filter-out -lpthread,$(EXTLIBS))
BASIC_CFLAGS += -I.
PERF_TMP_DIR = /data/local/tmp
+ PERF_SHELL_PATH = /system/bin/sh
endif

# Guard against environment variables
@@ -838,7 +840,11 @@ $(OUTPUT)builtin-help.o: builtin-help.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFL
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) \
'-DPERF_HTML_PATH="$(htmldir_SQ)"' \
'-DPERF_MAN_PATH="$(mandir_SQ)"' \
- '-DPERF_INFO_PATH="$(infodir_SQ)"' $<
+ '-DPERF_INFO_PATH="$(infodir_SQ)"' \
+ '-DPERF_SHELL_PATH="$(PERF_SHELL_PATH)"' $<
+
+$(OUTPUT)builtin-script.o: builtin-script.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_SHELL_PATH='"$(PERF_SHELL_PATH)"' $<

$(OUTPUT)builtin-timechart.o: builtin-timechart.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) \
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 411ee56..542b752 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -154,7 +154,7 @@ static void exec_man_cmd(const char *cmd, const char *page)
{
struct strbuf shell_cmd = STRBUF_INIT;
strbuf_addf(&shell_cmd, "%s %s", cmd, page);
- execl("/bin/sh", "sh", "-c", shell_cmd.buf, NULL);
+ execl(PERF_SHELL_PATH, "sh", "-c", shell_cmd.buf, NULL);
warning("failed to exec '%s': %s", cmd, strerror(errno));
}

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 04ceb07..d0d3a21 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1320,7 +1320,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
goto out;
}

- __argv[j++] = "/bin/sh";
+ __argv[j++] = PERF_SHELL_PATH;
__argv[j++] = rec_script_path;
if (system_wide)
__argv[j++] = "-a";
@@ -1331,7 +1331,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
__argv[j++] = argv[i];
__argv[j++] = NULL;

- execvp("/bin/sh", (char **)__argv);
+ execvp(PERF_SHELL_PATH, (char **)__argv);
free(__argv);
exit(-1);
}
@@ -1347,7 +1347,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
}

j = 0;
- __argv[j++] = "/bin/sh";
+ __argv[j++] = PERF_SHELL_PATH;
__argv[j++] = rep_script_path;
for (i = 1; i < rep_args + 1; i++)
__argv[j++] = argv[i];
@@ -1355,7 +1355,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
__argv[j++] = "-";
__argv[j++] = NULL;

- execvp("/bin/sh", (char **)__argv);
+ execvp(PERF_SHELL_PATH, (char **)__argv);
free(__argv);
exit(-1);
}
@@ -1384,7 +1384,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
goto out;
}

- __argv[j++] = "/bin/sh";
+ __argv[j++] = PERF_SHELL_PATH;
__argv[j++] = script_path;
if (system_wide)
__argv[j++] = "-a";
@@ -1392,7 +1392,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
__argv[j++] = argv[i];
__argv[j++] = NULL;

- execvp("/bin/sh", (char **)__argv);
+ execvp(PERF_SHELL_PATH, (char **)__argv);
free(__argv);
exit(-1);
}
--
1.7.9.5

2012-10-15 23:35:29

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 3/6] perf tools: add --addr2line command line option

From: Irina Tirdea <[email protected]>

When analyzing data recorded on a target with a different architecture
than the host, we must use addr2line from the toolchain for that
architecture.

Add a command line option to allow setting addr2line at runtime.

Signed-off-by: Irina Tirdea <[email protected]>
---
tools/perf/Documentation/perf-annotate.txt | 2 ++
tools/perf/Documentation/perf-report.txt | 2 ++
tools/perf/builtin-annotate.c | 3 +++
tools/perf/builtin-report.c | 3 +++
tools/perf/util/annotate.c | 13 ++++++++++++-
tools/perf/util/sort.c | 14 +++++++++++---
tools/perf/util/sort.h | 1 +
7 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
index c8ffd9f..177906a 100644
--- a/tools/perf/Documentation/perf-annotate.txt
+++ b/tools/perf/Documentation/perf-annotate.txt
@@ -87,6 +87,8 @@ OPTIONS

--objdump=<path>::
Path to objdump binary.
+--addr2line=<path>::
+ Path to addr2line binary.

SEE ALSO
--------
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index f4d91be..b6bb26e 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -170,6 +170,8 @@ OPTIONS

--objdump=<path>::
Path to objdump binary.
+--addr2line=<path>::
+ Path to addr2line binary.

SEE ALSO
--------
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 690fa9a..10d6ca4 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -285,6 +285,9 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
"Specify disassembler style (e.g. -M intel for intel syntax)"),
OPT_STRING(0, "objdump", &objdump_path, "path",
"objdump binary to use for disassembly and annotations"),
+ OPT_STRING(0, "addr2line", &addr2line_path, "path",
+ "addr2line binary to use for obtaining "
+ "file names and line numbers"),
OPT_END()
};

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 5104a40..3d30a9a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -641,6 +641,9 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
"use branch records for histogram filling", parse_branch_mode),
OPT_STRING(0, "objdump", &objdump_path, "path",
"objdump binary to use for disassembly and annotations"),
+ OPT_STRING(0, "addr2line", &addr2line_path, "path",
+ "addr2line binary to use for obtaining "
+ "file names and line numbers"),
OPT_END()
};

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f0a9103..bf5573f 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -18,6 +18,7 @@

const char *disassembler_style;
const char *objdump_path;
+const char *addr2line_path;

static struct ins *ins__find(const char *name);
static int disasm_line__parse(char *line, char **namep, char **rawp);
@@ -894,10 +895,18 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
struct source_line *src_line;
struct annotation *notes = symbol__annotation(sym);
struct sym_hist *h = annotation__histogram(notes, evidx);
+ char symfs_filename[PATH_MAX];

if (!h->sum)
return 0;

+ snprintf(symfs_filename, sizeof(symfs_filename), "%s%s",
+ symbol_conf.symfs, filename);
+ if (access(symfs_filename, R_OK)) {
+ snprintf(symfs_filename, sizeof(symfs_filename), "%s",
+ filename);
+ }
+
src_line = notes->src->lines = calloc(len, sizeof(struct source_line));
if (!notes->src->lines)
return -1;
@@ -915,7 +924,9 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
continue;

offset = start + i;
- sprintf(cmd, "addr2line -e %s %016" PRIx64, filename, offset);
+ sprintf(cmd, "%s -e %s %016" PRIx64,
+ addr2line_path ? addr2line_path : "addr2line",
+ symfs_filename, offset);
fp = popen(cmd, "r");
if (!fp)
continue;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index b5b1b92..d45e406 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -256,12 +256,20 @@ static int hist_entry__srcline_snprintf(struct hist_entry *self, char *bf,
FILE *fp;
char cmd[PATH_MAX + 2], *path = self->srcline, *nl;
size_t line_len;
+ char symfs_dso_name[PATH_MAX];

- if (path != NULL)
+ if (path != NULL || !self->ms.map)
goto out_path;

- snprintf(cmd, sizeof(cmd), "addr2line -e %s %016" PRIx64,
- self->ms.map->dso->long_name, self->ip);
+ snprintf(symfs_dso_name, sizeof(symfs_dso_name), "%s%s",
+ symbol_conf.symfs, self->ms.map->dso->long_name);
+ if (access(symfs_dso_name, R_OK)) {
+ snprintf(symfs_dso_name, sizeof(symfs_dso_name), "%s",
+ self->ms.map->dso->long_name);
+ }
+ snprintf(cmd, sizeof(cmd), "%s -e %s %016" PRIx64,
+ addr2line_path ? addr2line_path : "addr2line",
+ symfs_dso_name, self->ip);
fp = popen(cmd, "r");
if (!fp)
goto out_ip;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 13761d8..c8e58f6 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -139,6 +139,7 @@ struct sort_entry {

extern struct sort_entry sort_thread;
extern struct list_head hist_entry__sort_list;
+extern const char *addr2line_path;

void setup_sorting(const char * const usagestr[], const struct option *opts);
extern int sort_dimension__add(const char *);
--
1.7.9.5

2012-10-15 23:35:48

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 4/6] perf tools: Try to find cross-built objdump path

From: Namhyung Kim <[email protected]>

As we have architecture information of saved perf.data file, we can
try to find cross-built objdump path.

The triplets include support for Android (arm, x86 and mips architectures).

Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Irina Tirdea <[email protected]>
---
tools/perf/Makefile | 2 +
tools/perf/arch/common.c | 178 +++++++++++++++++++++++++++++++++++++++++
tools/perf/arch/common.h | 10 +++
tools/perf/builtin-annotate.c | 7 ++
tools/perf/builtin-report.c | 7 ++
tools/perf/util/annotate.h | 1 -
6 files changed, 204 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/arch/common.c
create mode 100644 tools/perf/arch/common.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 99bf383..5906adb 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -428,6 +428,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
LIB_OBJS += $(OUTPUT)ui/hist.o
LIB_OBJS += $(OUTPUT)ui/stdio/hist.o

+LIB_OBJS += $(OUTPUT)arch/common.o
+
BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
# Benchmark modules
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
new file mode 100644
index 0000000..2367b25
--- /dev/null
+++ b/tools/perf/arch/common.c
@@ -0,0 +1,178 @@
+#include <stdio.h>
+#include <sys/utsname.h>
+#include "common.h"
+#include "../util/debug.h"
+
+const char *const arm_triplets[] = {
+ "arm-eabi-",
+ "arm-linux-androideabi-",
+ "arm-unknown-linux-",
+ "arm-unknown-linux-gnu-",
+ "arm-unknown-linux-gnueabi-",
+ NULL
+};
+
+const char *const powerpc_triplets[] = {
+ "powerpc-unknown-linux-gnu-",
+ "powerpc64-unknown-linux-gnu-",
+ NULL
+};
+
+const char *const s390_triplets[] = {
+ "s390-ibm-linux-",
+ NULL
+};
+
+const char *const sh_triplets[] = {
+ "sh-unknown-linux-gnu-",
+ "sh64-unknown-linux-gnu-",
+ NULL
+};
+
+const char *const sparc_triplets[] = {
+ "sparc-unknown-linux-gnu-",
+ "sparc64-unknown-linux-gnu-",
+ NULL
+};
+
+const char *const x86_triplets[] = {
+ "x86_64-pc-linux-gnu-",
+ "x86_64-unknown-linux-gnu-",
+ "i686-pc-linux-gnu-",
+ "i586-pc-linux-gnu-",
+ "i486-pc-linux-gnu-",
+ "i386-pc-linux-gnu-",
+ "i686-linux-android-",
+ "i686-android-linux-",
+ NULL
+};
+
+const char *const mips_triplets[] = {
+ "mips-unknown-linux-gnu-",
+ "mipsel-linux-android-",
+ NULL
+};
+
+static bool lookup_path(char *name)
+{
+ bool found = false;
+ char *path, *tmp;
+ char buf[PATH_MAX];
+ char *env = getenv("PATH");
+
+ if (!env)
+ return false;
+
+ env = strdup(env);
+ if (!env)
+ return false;
+
+ path = strtok_r(env, ":", &tmp);
+ while (path) {
+ scnprintf(buf, sizeof(buf), "%s/%s", path, name);
+ if (access(buf, F_OK) == 0) {
+ found = true;
+ break;
+ }
+ path = strtok_r(NULL, ":", &tmp);
+ }
+ free(env);
+ return found;
+}
+
+static int lookup_triplets(const char *const *triplets, const char *name)
+{
+ int i;
+ char buf[PATH_MAX];
+
+ for (i = 0; triplets[i] != NULL; i++) {
+ scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
+ if (lookup_path(buf))
+ return i;
+ }
+ return -1;
+}
+
+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;
+ struct utsname uts;
+ const char *const *path_list;
+ char *buf = NULL;
+
+ if (uname(&uts) < 0)
+ goto out;
+
+ /*
+ * 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))
+ goto out;
+
+ cross_env = getenv("CROSS_COMPILE");
+ if (cross_env) {
+ if (asprintf(&buf, "%s%s", cross_env, name) < 0)
+ goto out_error;
+ if (buf[0] == '/') {
+ if (access(buf, F_OK) == 0)
+ goto out;
+ goto out_error;
+ }
+ if (lookup_path(buf))
+ goto out;
+ free(buf);
+ }
+
+ arch = env->arch;
+
+ if (!strcmp(arch, "arm"))
+ path_list = arm_triplets;
+ else if (!strcmp(arch, "powerpc"))
+ path_list = powerpc_triplets;
+ else if (!strcmp(arch, "sh"))
+ path_list = sh_triplets;
+ else if (!strcmp(arch, "s390"))
+ 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"))
+ path_list = x86_triplets;
+ else if (!strcmp(arch, "mips"))
+ path_list = mips_triplets;
+ else {
+ ui__error("binutils for %s not supported.\n", arch);
+ goto out_error;
+ }
+
+ idx = lookup_triplets(path_list, name);
+ if (idx < 0) {
+ ui__error("Please install %s for %s.\n"
+ "You can add it to PATH, set CROSS_COMPILE or "
+ "override the default using --%s.\n",
+ name, arch, name);
+ goto out_error;
+ }
+
+ if (asprintf(&buf, "%s%s", path_list[idx], name) < 0)
+ goto out_error;
+
+out:
+ *path = buf;
+ return 0;
+out_error:
+ free(buf);
+ *path = NULL;
+ return -1;
+}
+
+int perf_session_env__lookup_objdump(struct perf_session_env *env)
+{
+ return perf_session_env__lookup_binutils_path(env, "objdump",
+ &objdump_path);
+}
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
new file mode 100644
index 0000000..ede246e
--- /dev/null
+++ b/tools/perf/arch/common.h
@@ -0,0 +1,10 @@
+#ifndef ARCH_PERF_COMMON_H
+#define ARCH_PERF_COMMON_H
+
+#include "../util/session.h"
+
+extern const char *objdump_path;
+
+int perf_session_env__lookup_objdump(struct perf_session_env *env);
+
+#endif /* ARCH_PERF_COMMON_H */
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 10d6ca4..e4df5da 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -28,6 +28,7 @@
#include "util/hist.h"
#include "util/session.h"
#include "util/tool.h"
+#include "arch/common.h"

#include <linux/bitmap.h>

@@ -186,6 +187,12 @@ static int __cmd_annotate(struct perf_annotate *ann)
goto out_delete;
}

+ if (!objdump_path) {
+ ret = perf_session_env__lookup_objdump(&session->header.env);
+ if (ret)
+ goto out_delete;
+ }
+
ret = perf_session__process_events(session, &ann->tool);
if (ret)
goto out_delete;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3d30a9a..9c0fe68 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -33,6 +33,7 @@
#include "util/thread.h"
#include "util/sort.h"
#include "util/hist.h"
+#include "arch/common.h"

#include <linux/bitmap.h>

@@ -675,6 +676,12 @@ 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;

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 39242dc..a4dd25a 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -154,6 +154,5 @@ static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
#endif

extern const char *disassembler_style;
-extern const char *objdump_path;

#endif /* __PERF_ANNOTATE_H */
--
1.7.9.5

2012-10-15 23:36:13

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 5/6] perf tools: Try to find cross-built addr2line path

From: Irina Tirdea <[email protected]>

As we have architecture information of saved perf.data file, we can
also try to find cross-built addr2line path. The predefined triplets
include support for Android (arm, x86 and mips architectures).

Signed-off-by: Irina Tirdea <[email protected]>
---
tools/perf/Documentation/android.txt | 18 ++++++++++++++++++
tools/perf/arch/common.c | 6 ++++++
tools/perf/arch/common.h | 2 ++
tools/perf/builtin-annotate.c | 6 ++++++
tools/perf/builtin-report.c | 6 ++++++
tools/perf/util/sort.c | 1 +
tools/perf/util/sort.h | 1 -
7 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/android.txt b/tools/perf/Documentation/android.txt
index 24fd01c..c2a8017 100644
--- a/tools/perf/Documentation/android.txt
+++ b/tools/perf/Documentation/android.txt
@@ -74,3 +74,21 @@ IV. Run perf
------------------------------------------------
Run perf on your device/emulator to which you previously connected using adb:
# ./data/perf
+
+V. Analyze data recorded in Android on the host
+------------------------------------------------
+perf report and perf annotate use objdump and addr2line tools from Linux.
+For analyzing data recorded for a different architecture you need to use the
+versions provided in the toolchain for that architecture. The detection of the
+proper toolchain is done at runtime; all you need to do is set some environment
+variables before running perf annotate/report:
+1. set CROSS_COMPILE
+ export CROSS_COMPILE=${NDK_TOOLCHAIN}
+or
+2. add the path to the toolchain to PATH
+ export PATH=$PATH:${ANDROID_TOOLCHAIN}
+or
+3. set the Android environment if you have the source tree
+(this will set $PATH for you as mentioned above)
+ source build/envsetup.sh
+ lunch
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 2367b25..18ea475 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -176,3 +176,9 @@ int perf_session_env__lookup_objdump(struct perf_session_env *env)
return perf_session_env__lookup_binutils_path(env, "objdump",
&objdump_path);
}
+
+int perf_session_env__lookup_addr2line(struct perf_session_env *env)
+{
+ return perf_session_env__lookup_binutils_path(env, "addr2line",
+ &addr2line_path);
+}
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
index ede246e..e78cebd 100644
--- a/tools/perf/arch/common.h
+++ b/tools/perf/arch/common.h
@@ -4,7 +4,9 @@
#include "../util/session.h"

extern const char *objdump_path;
+extern const char *addr2line_path;

int perf_session_env__lookup_objdump(struct perf_session_env *env);
+int perf_session_env__lookup_addr2line(struct perf_session_env *env);

#endif /* ARCH_PERF_COMMON_H */
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index e4df5da..0f46ec6 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -193,6 +193,12 @@ static int __cmd_annotate(struct perf_annotate *ann)
goto out_delete;
}

+ if (!addr2line_path) {
+ ret = perf_session_env__lookup_addr2line(&session->header.env);
+ if (ret)
+ goto out_delete;
+ }
+
ret = perf_session__process_events(session, &ann->tool);
if (ret)
goto out_delete;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 9c0fe68..692462c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -682,6 +682,12 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
goto error;
}

+ if (!addr2line_path) {
+ ret = perf_session_env__lookup_addr2line(&session->header.env);
+ if (ret)
+ goto error;
+ }
+
if (sort__branch_mode == -1 && has_br_stack)
sort__branch_mode = 1;

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d45e406..e526158 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,5 +1,6 @@
#include "sort.h"
#include "hist.h"
+#include "../arch/common.h"

regex_t parent_regex;
const char default_parent_pattern[] = "^sys_|^do_page_fault";
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index c8e58f6..13761d8 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -139,7 +139,6 @@ struct sort_entry {

extern struct sort_entry sort_thread;
extern struct list_head hist_entry__sort_list;
-extern const char *addr2line_path;

void setup_sorting(const char * const usagestr[], const struct option *opts);
extern int sort_dimension__add(const char *);
--
1.7.9.5

2012-10-15 23:36:49

by Irina Tirdea

[permalink] [raw]
Subject: [PATCH v4 6/6] perf stat: implement --big-num grouping

From: Irina Tirdea <[email protected]>

In glibc, printf supports ' to group numbers with thousands' grouping
characters. Bionic does not support ' for printf.

Implement thousands's grouping for numbers according to locale.
The implementation uses the algorithm from glibc
(http://www.gnu.org/software/libc/).

Bionic does not implement locales, so we need to add a configuration
option LOCALE_SUPPORT. If it is not defined, default values for thousands
separator and grouping are used.

Signed-off-by: Irina Tirdea <[email protected]>
---
tools/perf/Makefile | 6 ++
tools/perf/builtin-stat.c | 106 ++++++++++++++++++++++++++++++++---
tools/perf/config/feature-tests.mak | 18 ++++++
3 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 5906adb..002e7c2 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -785,6 +785,12 @@ ifndef NO_BACKTRACE
endif
endif

+ifndef NO_LOCALE
+ ifeq ($(call try-cc,$(SOURCE_LOCALE),),y)
+ BASIC_CFLAGS += -DLOCALE_SUPPORT
+ endif
+endif
+
ifdef ASCIIDOC8
export ASCIIDOC8
endif
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 93b9011..d625f80 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -60,6 +60,8 @@
#include <sys/prctl.h>
#include <locale.h>

+/* max double number have E+308 + \0 + sign */
+#define MAX_NR_STR 310
#define DEFAULT_SEPARATOR " "
#define CNTR_NOT_SUPPORTED "<not supported>"
#define CNTR_NOT_COUNTED "<not counted>"
@@ -631,18 +633,106 @@ static void print_ll_cache_misses(int cpu,
fprintf(output, " of all LL-cache hits ");
}

+/* Group the digits according to the grouping rules of the current locale.
+ The interpretation of GROUPING is as in `struct lconv' from <locale.h>. */
+static int group_number_locale(char *number, char **gnumber)
+{
+ const char *thousands_sep = NULL, *grouping = NULL;
+ int glen, tlen, dest_alloc_size, src_size, ret = -ENOMEM, cnt;
+ char *dest_alloc_ptr, *dest_end, *src_start, *src_end;
+
+#ifdef LOCALE_SUPPORT
+ struct lconv *lc = localeconv();
+ if (lc != NULL) {
+ thousands_sep = lc->thousands_sep;
+ grouping = lc->grouping;
+ }
+#else
+ thousands_sep = ",";
+ grouping = "\x3";
+#endif
+
+ *gnumber = NULL;
+ /* No grouping */
+ if (thousands_sep == NULL || grouping == NULL ||
+ *thousands_sep == '\0' || *grouping == CHAR_MAX || *grouping <= 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ glen = *grouping++;
+ tlen = strlen(thousands_sep);
+
+ src_size = strlen(number);
+ /* Worst case scenario we have 1-character grouping */
+ dest_alloc_size = (src_size + src_size * tlen) * sizeof(char);
+ dest_alloc_ptr = zalloc(dest_alloc_size);
+ if (dest_alloc_ptr == NULL)
+ goto out;
+ /* -1 for '\0' */
+ dest_end = dest_alloc_ptr + dest_alloc_size - 1;
+
+ src_start = number;
+ src_end = number + src_size;
+
+ while (src_end > src_start) {
+ *--dest_end = *--src_end;
+ if (--glen == 0 && src_end > src_start) {
+ /* A new group */
+ cnt = tlen;
+ do
+ *--dest_end = thousands_sep[--cnt];
+ while (cnt > 0);
+
+ if (*grouping == CHAR_MAX || *grouping < 0) {
+ /* No further grouping to be done.
+ Copy the rest of the number. */
+ do
+ *--dest_end = *--src_end;
+ while (src_end > src_start);
+ break;
+ } else if (*grouping != '\0') {
+ glen = *grouping++;
+ } else {
+ /* The previous grouping repeats ad infinitum */
+ glen = grouping[-1];
+ }
+ }
+ }
+
+ /* Make a copy with the exact needed size of the grouped number */
+ *gnumber = strdup(dest_end);
+ if (*gnumber == NULL)
+ goto out_free_dest;
+
+ ret = 0;
+out_free_dest:
+ free(dest_alloc_ptr);
+out:
+ return ret;
+}
+
static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
{
double total, ratio = 0.0;
char cpustr[16] = { '\0', };
const char *fmt;
+ char avgstr[MAX_NR_STR], *pavgstr;
+ int ret;

- if (csv_output)
- fmt = "%s%.0f%s%s";
- else if (big_num)
- fmt = "%s%'18.0f%s%-25s";
- else
- fmt = "%s%18.0f%s%-25s";
+ sprintf(avgstr, "%.0f", avg);
+ pavgstr = avgstr;
+
+ if (csv_output) {
+ fmt = "%s%s%s%s";
+ } else {
+ fmt = "%s%18s%s%-25s";
+ if (big_num) {
+ ret = group_number_locale(avgstr, &pavgstr);
+ if (ret < 0)
+ pavgstr = avgstr;
+ }
+ }

if (no_aggr)
sprintf(cpustr, "CPU%*d%s",
@@ -651,7 +741,9 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
else
cpu = 0;

- fprintf(output, fmt, cpustr, avg, csv_sep, perf_evsel__name(evsel));
+ fprintf(output, fmt, cpustr, pavgstr, csv_sep, perf_evsel__name(evsel));
+ if (pavgstr != avgstr)
+ free(pavgstr);

if (evsel->cgrp)
fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index 3ef5ec9..ec3a6f1 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -222,3 +222,21 @@ int main(void)
return on_exit(NULL, NULL);
}
endef
+
+ifndef NO_LOCALE
+define SOURCE_LOCALE
+#include <locale.h>
+
+int main(void)
+{
+ char *thousands_sep, *grouping;
+
+ struct lconv *lc = localeconv();
+ if (lc != NULL) {
+ thousands_sep = lc->thousands_sep;
+ grouping = lc->grouping;
+ }
+ return 0;
+}
+endef
+endif
--
1.7.9.5

2012-10-16 15:18:28

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] perf tools: configure tmp path at build time

Em Tue, Oct 16, 2012 at 02:33:35AM +0300, Irina Tirdea escreveu:
> From: Irina Tirdea <[email protected]>
>
> Temporary perf files are hardcoded to point to /tmp. Android does not have
> a /tmp directory so it needs to set this path at compile time.
>
> Add a compile-time definition (PERF_TMP_DIR) in the Makefile that sets the path
> to temp directory. By default it points to /tmp.
>
> $(OUTPUT)util/parse-events.o: $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-bison.c
> -$(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c
> +$(OUTPUT)util/pmu.o: util/pmu.c $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>
> LIB_FILE=$(OUTPUT)libperf.a
>
> @@ -889,6 +892,21 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
> $(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
> $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<

Humm, so we need to add an specific rule for each source file that needs
PERF_TMP_DIR? Why not make it available to all source files?

> +$(OUTPUT)util/dso-test-data.o: util/dso-test-data.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
> +
> +$(OUTPUT)util/map.o: util/map.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
> +
> +$(OUTPUT)util/symbol.o: util/symbol.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
> +
> +$(OUTPUT)util/trace-event-info.o: util/trace-event-info.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
> +
> +$(OUTPUT)util/vdso.o: util/vdso.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
> +
> $(OUTPUT)ui/browser.o: ui/browser.c $(OUTPUT)PERF-CFLAGS
> $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
>
> diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
> index e919306..058c3b7 100644
> --- a/tools/perf/perf-archive.sh
> +++ b/tools/perf/perf-archive.sh
> @@ -18,7 +18,16 @@ else
> PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
> fi
>
> -BUILDIDS=$(mktemp /tmp/perf-archive-buildids.XXXXXX)
> +#
> +# PERF_TMP_DIR environment variable set by perf
> +# path to temp directory, default to /tmp
> +#
> +if [ -z $PERF_TMP_DIR ]; then
> + PERF_TMP_DIR=/tmp
> +fi
> +
> +
> +BUILDIDS=$(mktemp $PERF_TMP_DIR/perf-archive-buildids.XXXXXX)
> NOBUILDID=0000000000000000000000000000000000000000
>
> perf buildid-list -i $PERF_DATA --with-hits | grep -v "^$NOBUILDID " > $BUILDIDS
> @@ -28,7 +37,7 @@ if [ ! -s $BUILDIDS ] ; then
> exit 1
> fi
>
> -MANIFEST=$(mktemp /tmp/perf-archive-manifest.XXXXXX)
> +MANIFEST=$(mktemp $PERF_TMP_DIR/perf-archive-manifest.XXXXXX)
> PERF_BUILDID_LINKDIR=$(readlink -f $PERF_BUILDID_DIR)/
>
> cut -d ' ' -f 1 $BUILDIDS | \
> diff --git a/tools/perf/util/dso-test-data.c b/tools/perf/util/dso-test-data.c
> index c6caede..ca81e65 100644
> --- a/tools/perf/util/dso-test-data.c
> +++ b/tools/perf/util/dso-test-data.c
> @@ -18,7 +18,7 @@ do { \
>
> static char *test_file(int size)
> {
> - static char buf_templ[] = "/tmp/test-XXXXXX";
> + static char buf_templ[] = PERF_TMP_DIR "/test-XXXXXX";
> char *templ = buf_templ;
> int fd, i;
> unsigned char *buf;
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 6109fa4..4e69b57 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -59,7 +59,8 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
> no_dso = is_no_dso_memory(filename);
>
> if (anon) {
> - snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", pid);
> + snprintf(newfilename, sizeof(newfilename),
> + PERF_TMP_DIR "/perf-%d.map", pid);
> filename = newfilename;
> }
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 8a2229d..5ebde18 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -637,7 +637,7 @@ static char *test_format_dir_get(void)
> static char dir[PATH_MAX];
> unsigned int i;
>
> - snprintf(dir, PATH_MAX, "/tmp/perf-pmu-test-format-XXXXXX");
> + snprintf(dir, PATH_MAX, PERF_TMP_DIR "/perf-pmu-test-format-XXXXXX");
> if (!mkdtemp(dir))
> return NULL;
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index e2e8c69..d8aca82 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1051,7 +1051,9 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
>
> dso->adjust_symbols = 0;
>
> - if (strncmp(dso->name, "/tmp/perf-", 10) == 0) {
> + if (strncmp
> + (dso->name, PERF_TMP_DIR "/perf-",
> + strlen(PERF_TMP_DIR "/perf-")) == 0) {
> struct stat st;
>
> if (lstat(dso->name, &st) < 0)
> diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
> index a8d81c3..e50ea61 100644
> --- a/tools/perf/util/trace-event-info.c
> +++ b/tools/perf/util/trace-event-info.c
> @@ -481,7 +481,7 @@ struct tracing_data *tracing_data_get(struct list_head *pattrs,
> int temp_fd;
>
> snprintf(tdata->temp_file, sizeof(tdata->temp_file),
> - "/tmp/perf-XXXXXX");
> + PERF_TMP_DIR "/perf-XXXXXX");
> if (!mkstemp(tdata->temp_file))
> die("Can't make temp file");
>
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index e60951f..8b9613b 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -14,7 +14,7 @@
> #include "linux/string.h"
>
> static bool vdso_found;
> -static char vdso_file[] = "/tmp/perf-vdso.so-XXXXXX";
> +static char vdso_file[] = PERF_TMP_DIR "/perf-vdso.so-XXXXXX";
>
> static int find_vdso_map(void **start, void **end)
> {
> --
> 1.7.9.5

2012-10-16 15:19:26

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4 2/6] perf tools: configure shell path at compile time

Em Tue, Oct 16, 2012 at 02:33:36AM +0300, Irina Tirdea escreveu:
> @@ -838,7 +840,11 @@ $(OUTPUT)builtin-help.o: builtin-help.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFL
> $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) \
> '-DPERF_HTML_PATH="$(htmldir_SQ)"' \
> '-DPERF_MAN_PATH="$(mandir_SQ)"' \
> - '-DPERF_INFO_PATH="$(infodir_SQ)"' $<
> + '-DPERF_INFO_PATH="$(infodir_SQ)"' \
> + '-DPERF_SHELL_PATH="$(PERF_SHELL_PATH)"' $<
> +
> +$(OUTPUT)builtin-script.o: builtin-script.c $(OUTPUT)PERF-CFLAGS
> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_SHELL_PATH='"$(PERF_SHELL_PATH)"' $<

Same question as for TMP, why not make it available for all files?

- Arnaldo

2012-10-16 15:22:03

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4 3/6] perf tools: add --addr2line command line option

Em Tue, Oct 16, 2012 at 02:33:37AM +0300, Irina Tirdea escreveu:
> From: Irina Tirdea <[email protected]>
>
> When analyzing data recorded on a target with a different architecture
> than the host, we must use addr2line from the toolchain for that
> architecture.
>
> Add a command line option to allow setting addr2line at runtime.

You're doing two things here:

1. Adding --addr2line

2. fixing a bug for unresolved symbols

Please try not to do that, instead provide two patches, one addressing
each issue.

Furthermore, Namhyung fixed #2 already:

commit a2c1311c4cbd47a087e95be18fee79ead3d0744b
Author: Namhyung Kim <[email protected]>
Date: Mon Oct 15 12:39:42 2012 +0900

perf tools: Fix segfault when using srcline sort key

- Arnaldo

2012-10-16 15:26:27

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4 4/6] perf tools: Try to find cross-built objdump path

Em Tue, Oct 16, 2012 at 02:33:38AM +0300, Irina Tirdea escreveu:
> From: Namhyung Kim <[email protected]>

Well, by now it is not anymore from Namhyung, but based on a previous
patch by him, right?

I'm ok with the patch, thanks for addressing my suggestions, but please
resubmit with you as the patch author, giving credit for the original
patch by Namhyung.

Namhyung, are you ok with it? If so could I have a reviewed-by from you
or at least an acked-by? The SOB is to be dropped, as this is not coming
from you, but directly from Irina and as explained above, its a modified
patch.

- Arnaldo

> As we have architecture information of saved perf.data file, we can
> try to find cross-built objdump path.
>
> The triplets include support for Android (arm, x86 and mips architectures).
>
> Signed-off-by: Namhyung Kim <[email protected]>
> Signed-off-by: Irina Tirdea <[email protected]>
> ---
> tools/perf/Makefile | 2 +
> tools/perf/arch/common.c | 178 +++++++++++++++++++++++++++++++++++++++++
> tools/perf/arch/common.h | 10 +++
> tools/perf/builtin-annotate.c | 7 ++
> tools/perf/builtin-report.c | 7 ++
> tools/perf/util/annotate.h | 1 -
> 6 files changed, 204 insertions(+), 1 deletion(-)
> create mode 100644 tools/perf/arch/common.c
> create mode 100644 tools/perf/arch/common.h
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 99bf383..5906adb 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -428,6 +428,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
> LIB_OBJS += $(OUTPUT)ui/hist.o
> LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
>
> +LIB_OBJS += $(OUTPUT)arch/common.o
> +
> BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
> BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
> # Benchmark modules
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> new file mode 100644
> index 0000000..2367b25
> --- /dev/null
> +++ b/tools/perf/arch/common.c
> @@ -0,0 +1,178 @@
> +#include <stdio.h>
> +#include <sys/utsname.h>
> +#include "common.h"
> +#include "../util/debug.h"
> +
> +const char *const arm_triplets[] = {
> + "arm-eabi-",
> + "arm-linux-androideabi-",
> + "arm-unknown-linux-",
> + "arm-unknown-linux-gnu-",
> + "arm-unknown-linux-gnueabi-",
> + NULL
> +};
> +
> +const char *const powerpc_triplets[] = {
> + "powerpc-unknown-linux-gnu-",
> + "powerpc64-unknown-linux-gnu-",
> + NULL
> +};
> +
> +const char *const s390_triplets[] = {
> + "s390-ibm-linux-",
> + NULL
> +};
> +
> +const char *const sh_triplets[] = {
> + "sh-unknown-linux-gnu-",
> + "sh64-unknown-linux-gnu-",
> + NULL
> +};
> +
> +const char *const sparc_triplets[] = {
> + "sparc-unknown-linux-gnu-",
> + "sparc64-unknown-linux-gnu-",
> + NULL
> +};
> +
> +const char *const x86_triplets[] = {
> + "x86_64-pc-linux-gnu-",
> + "x86_64-unknown-linux-gnu-",
> + "i686-pc-linux-gnu-",
> + "i586-pc-linux-gnu-",
> + "i486-pc-linux-gnu-",
> + "i386-pc-linux-gnu-",
> + "i686-linux-android-",
> + "i686-android-linux-",
> + NULL
> +};
> +
> +const char *const mips_triplets[] = {
> + "mips-unknown-linux-gnu-",
> + "mipsel-linux-android-",
> + NULL
> +};
> +
> +static bool lookup_path(char *name)
> +{
> + bool found = false;
> + char *path, *tmp;
> + char buf[PATH_MAX];
> + char *env = getenv("PATH");
> +
> + if (!env)
> + return false;
> +
> + env = strdup(env);
> + if (!env)
> + return false;
> +
> + path = strtok_r(env, ":", &tmp);
> + while (path) {
> + scnprintf(buf, sizeof(buf), "%s/%s", path, name);
> + if (access(buf, F_OK) == 0) {
> + found = true;
> + break;
> + }
> + path = strtok_r(NULL, ":", &tmp);
> + }
> + free(env);
> + return found;
> +}
> +
> +static int lookup_triplets(const char *const *triplets, const char *name)
> +{
> + int i;
> + char buf[PATH_MAX];
> +
> + for (i = 0; triplets[i] != NULL; i++) {
> + scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
> + if (lookup_path(buf))
> + return i;
> + }
> + return -1;
> +}
> +
> +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;
> + struct utsname uts;
> + const char *const *path_list;
> + char *buf = NULL;
> +
> + if (uname(&uts) < 0)
> + goto out;
> +
> + /*
> + * 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))
> + goto out;
> +
> + cross_env = getenv("CROSS_COMPILE");
> + if (cross_env) {
> + if (asprintf(&buf, "%s%s", cross_env, name) < 0)
> + goto out_error;
> + if (buf[0] == '/') {
> + if (access(buf, F_OK) == 0)
> + goto out;
> + goto out_error;
> + }
> + if (lookup_path(buf))
> + goto out;
> + free(buf);
> + }
> +
> + arch = env->arch;
> +
> + if (!strcmp(arch, "arm"))
> + path_list = arm_triplets;
> + else if (!strcmp(arch, "powerpc"))
> + path_list = powerpc_triplets;
> + else if (!strcmp(arch, "sh"))
> + path_list = sh_triplets;
> + else if (!strcmp(arch, "s390"))
> + 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"))
> + path_list = x86_triplets;
> + else if (!strcmp(arch, "mips"))
> + path_list = mips_triplets;
> + else {
> + ui__error("binutils for %s not supported.\n", arch);
> + goto out_error;
> + }
> +
> + idx = lookup_triplets(path_list, name);
> + if (idx < 0) {
> + ui__error("Please install %s for %s.\n"
> + "You can add it to PATH, set CROSS_COMPILE or "
> + "override the default using --%s.\n",
> + name, arch, name);
> + goto out_error;
> + }
> +
> + if (asprintf(&buf, "%s%s", path_list[idx], name) < 0)
> + goto out_error;
> +
> +out:
> + *path = buf;
> + return 0;
> +out_error:
> + free(buf);
> + *path = NULL;
> + return -1;
> +}
> +
> +int perf_session_env__lookup_objdump(struct perf_session_env *env)
> +{
> + return perf_session_env__lookup_binutils_path(env, "objdump",
> + &objdump_path);
> +}
> diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
> new file mode 100644
> index 0000000..ede246e
> --- /dev/null
> +++ b/tools/perf/arch/common.h
> @@ -0,0 +1,10 @@
> +#ifndef ARCH_PERF_COMMON_H
> +#define ARCH_PERF_COMMON_H
> +
> +#include "../util/session.h"
> +
> +extern const char *objdump_path;
> +
> +int perf_session_env__lookup_objdump(struct perf_session_env *env);
> +
> +#endif /* ARCH_PERF_COMMON_H */
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 10d6ca4..e4df5da 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -28,6 +28,7 @@
> #include "util/hist.h"
> #include "util/session.h"
> #include "util/tool.h"
> +#include "arch/common.h"
>
> #include <linux/bitmap.h>
>
> @@ -186,6 +187,12 @@ static int __cmd_annotate(struct perf_annotate *ann)
> goto out_delete;
> }
>
> + if (!objdump_path) {
> + ret = perf_session_env__lookup_objdump(&session->header.env);
> + if (ret)
> + goto out_delete;
> + }
> +
> ret = perf_session__process_events(session, &ann->tool);
> if (ret)
> goto out_delete;
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 3d30a9a..9c0fe68 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -33,6 +33,7 @@
> #include "util/thread.h"
> #include "util/sort.h"
> #include "util/hist.h"
> +#include "arch/common.h"
>
> #include <linux/bitmap.h>
>
> @@ -675,6 +676,12 @@ 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;
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 39242dc..a4dd25a 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -154,6 +154,5 @@ static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
> #endif
>
> extern const char *disassembler_style;
> -extern const char *objdump_path;
>
> #endif /* __PERF_ANNOTATE_H */
> --
> 1.7.9.5

2012-10-16 15:27:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] perf tools: Try to find cross-built addr2line path

Em Tue, Oct 16, 2012 at 02:33:39AM +0300, Irina Tirdea escreveu:
> From: Irina Tirdea <[email protected]>
>
> As we have architecture information of saved perf.data file, we can
> also try to find cross-built addr2line path. The predefined triplets
> include support for Android (arm, x86 and mips architectures).

This one is all right, but depends on the previous one, introducing the
lookup routines.

- Arnaldo

2012-10-16 16:48:13

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] perf tools: configure tmp path at build time

A system that doesn't have /bin/sh is fundamentally broken. How about
just creating it as a symlink in a simple unix compatibility layer for
Android. That'd help with /tmp as well.

2012-10-16 16:49:33

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] perf stat: implement --big-num grouping

On Tue, Oct 16, 2012 at 02:33:40AM +0300, Irina Tirdea wrote:
> From: Irina Tirdea <[email protected]>
>
> In glibc, printf supports ' to group numbers with thousands' grouping
> characters. Bionic does not support ' for printf.
>
> Implement thousands's grouping for numbers according to locale.
> The implementation uses the algorithm from glibc
> (http://www.gnu.org/software/libc/).
>
> Bionic does not implement locales, so we need to add a configuration
> option LOCALE_SUPPORT. If it is not defined, default values for thousands
> separator and grouping are used.

Duplicating this in perf sounds like a hack. Does gnulib provide this
feature? It's the canonical source for getting standards or helper
functions on systems that don't support them.

2012-10-17 04:22:36

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v4 4/6] perf tools: Try to find cross-built objdump path

Hi Arnaldo and Irina,

On Tue, 16 Oct 2012 08:26:19 -0700, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 16, 2012 at 02:33:38AM +0300, Irina Tirdea escreveu:
>> From: Namhyung Kim <[email protected]>
>
> Well, by now it is not anymore from Namhyung, but based on a previous
> patch by him, right?
>
> I'm ok with the patch, thanks for addressing my suggestions, but please
> resubmit with you as the patch author, giving credit for the original
> patch by Namhyung.
>
> Namhyung, are you ok with it? If so could I have a reviewed-by from you
> or at least an acked-by? The SOB is to be dropped, as this is not coming
> from you, but directly from Irina and as explained above, its a modified
> patch.

Sure. Please feel free to add my

Acked-by: Namhyung Kim <[email protected]>

Thanks,
Namhyung

2012-10-21 16:25:04

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] perf tools: configure tmp path at build time


* Christoph Hellwig <[email protected]> wrote:

> A system that doesn't have /bin/sh is fundamentally broken.

No, such a system is simply different.

> How about just creating it as a symlink in a simple unix
> compatibility layer for Android. That'd help with /tmp as
> well.

That might make sense to pursue, are you volunteering for it?

Meanwhile here on earth we are making stuff work, regardless of
dogmatic declarations.

Thanks,

Ingo

2012-10-21 16:28:56

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] perf stat: implement --big-num grouping


* Christoph Hellwig <[email protected]> wrote:

> On Tue, Oct 16, 2012 at 02:33:40AM +0300, Irina Tirdea wrote:
> > From: Irina Tirdea <[email protected]>
> >
> > In glibc, printf supports ' to group numbers with thousands'
> > grouping characters. Bionic does not support ' for printf.
> >
> > Implement thousands's grouping for numbers according to
> > locale. The implementation uses the algorithm from glibc
> > (http://www.gnu.org/software/libc/).
> >
> > Bionic does not implement locales, so we need to add a
> > configuration option LOCALE_SUPPORT. If it is not defined,
> > default values for thousands separator and grouping are
> > used.
>
> Duplicating this in perf sounds like a hack. [...]

There's countless utility and compatibility 'hacks' in the Linux
kernel too, which makes it a practical solution.

> [...] Does gnulib provide this feature? It's the canonical
> source for getting standards or helper functions on systems
> that don't support them.

Unless Android comes with "gnulib" installed by default that's
obviously not a solution.

What's your point?

Thanks,

Ingo

2012-10-22 07:39:13

by Irina Tirdea

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] perf tools: configure tmp path at build time

On Tue, Oct 16, 2012 at 6:18 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Tue, Oct 16, 2012 at 02:33:35AM +0300, Irina Tirdea escreveu:
>> From: Irina Tirdea <[email protected]>
>>
>> Temporary perf files are hardcoded to point to /tmp. Android does not have
>> a /tmp directory so it needs to set this path at compile time.
>>
>> Add a compile-time definition (PERF_TMP_DIR) in the Makefile that sets the path
>> to temp directory. By default it points to /tmp.
>>
>> $(OUTPUT)util/parse-events.o: $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-bison.c
>> -$(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c
>> +$(OUTPUT)util/pmu.o: util/pmu.c $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c $(OUTPUT)PERF-CFLAGS
>> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>>
>> LIB_FILE=$(OUTPUT)libperf.a
>>
>> @@ -889,6 +892,21 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
>> $(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
>> $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
>
> Humm, so we need to add an specific rule for each source file that needs
> PERF_TMP_DIR? Why not make it available to all source files?
>

I've seen this is the way other defines are handled (PERF_HTML_PATH,
PERF_INFO_PATH, etc). But PERF_TMP_DIR is used in more places so it
makes sense to make it available to all source files. I'll make the
changes (for both PERF_TMP_DIR and PERF_SHELL_PATH) and resubmit.

>> +$(OUTPUT)util/dso-test-data.o: util/dso-test-data.c $(OUTPUT)PERF-CFLAGS
>> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>> +
>> +$(OUTPUT)util/map.o: util/map.c $(OUTPUT)PERF-CFLAGS
>> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>> +
>> +$(OUTPUT)util/symbol.o: util/symbol.c $(OUTPUT)PERF-CFLAGS
>> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>> +
>> +$(OUTPUT)util/trace-event-info.o: util/trace-event-info.c $(OUTPUT)PERF-CFLAGS
>> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>> +
>> +$(OUTPUT)util/vdso.o: util/vdso.c $(OUTPUT)PERF-CFLAGS
>> + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
>> +
>> $(OUTPUT)ui/browser.o: ui/browser.c $(OUTPUT)PERF-CFLAGS
>> $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
>>

Thanks,
Irina

2012-10-22 09:06:30

by Irina Tirdea

[permalink] [raw]
Subject: Re: [PATCH v4 3/6] perf tools: add --addr2line command line option

On Tue, Oct 16, 2012 at 6:21 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Tue, Oct 16, 2012 at 02:33:37AM +0300, Irina Tirdea escreveu:
>> From: Irina Tirdea <[email protected]>
>>
>> When analyzing data recorded on a target with a different architecture
>> than the host, we must use addr2line from the toolchain for that
>> architecture.
>>
>> Add a command line option to allow setting addr2line at runtime.
>
> You're doing two things here:
>
> 1. Adding --addr2line
>
> 2. fixing a bug for unresolved symbols
>
> Please try not to do that, instead provide two patches, one addressing
> each issue.
>

You're absolutely right. Missed this.

> Furthermore, Namhyung fixed #2 already:
>
> commit a2c1311c4cbd47a087e95be18fee79ead3d0744b
> Author: Namhyung Kim <[email protected]>
> Date: Mon Oct 15 12:39:42 2012 +0900
>
> perf tools: Fix segfault when using srcline sort key
>
> - Arnaldo

Thanks,
Irina

2012-10-25 08:00:56

by Tirdea, Irina

[permalink] [raw]
Subject: [tip:perf/core] perf tools: Try to find cross-built objdump path

Commit-ID: 68e94f4eb56d92ccb617a98fcac5e575702ec4fd
Gitweb: http://git.kernel.org/tip/68e94f4eb56d92ccb617a98fcac5e575702ec4fd
Author: Irina Tirdea <[email protected]>
AuthorDate: Tue, 16 Oct 2012 02:33:38 +0300
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 24 Oct 2012 14:20:11 -0200

perf tools: Try to find cross-built objdump path

As we have architecture information of saved perf.data file, we can try
to find cross-built objdump path.

The triplets include support for Android (arm, x86 and mips
architectures).

Signed-off-by: Irina Tirdea <[email protected]>
Originally-by: Namhyung Kim <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 2 +
tools/perf/arch/common.c | 178 +++++++++++++++++++++++++++++++++++++++++
tools/perf/arch/common.h | 10 +++
tools/perf/builtin-annotate.c | 7 ++
tools/perf/builtin-report.c | 7 ++
tools/perf/util/annotate.h | 1 -
6 files changed, 204 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 6790cb4..78a81ed 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -426,6 +426,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
LIB_OBJS += $(OUTPUT)ui/hist.o
LIB_OBJS += $(OUTPUT)ui/stdio/hist.o

+LIB_OBJS += $(OUTPUT)arch/common.o
+
BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
# Benchmark modules
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
new file mode 100644
index 0000000..2367b25
--- /dev/null
+++ b/tools/perf/arch/common.c
@@ -0,0 +1,178 @@
+#include <stdio.h>
+#include <sys/utsname.h>
+#include "common.h"
+#include "../util/debug.h"
+
+const char *const arm_triplets[] = {
+ "arm-eabi-",
+ "arm-linux-androideabi-",
+ "arm-unknown-linux-",
+ "arm-unknown-linux-gnu-",
+ "arm-unknown-linux-gnueabi-",
+ NULL
+};
+
+const char *const powerpc_triplets[] = {
+ "powerpc-unknown-linux-gnu-",
+ "powerpc64-unknown-linux-gnu-",
+ NULL
+};
+
+const char *const s390_triplets[] = {
+ "s390-ibm-linux-",
+ NULL
+};
+
+const char *const sh_triplets[] = {
+ "sh-unknown-linux-gnu-",
+ "sh64-unknown-linux-gnu-",
+ NULL
+};
+
+const char *const sparc_triplets[] = {
+ "sparc-unknown-linux-gnu-",
+ "sparc64-unknown-linux-gnu-",
+ NULL
+};
+
+const char *const x86_triplets[] = {
+ "x86_64-pc-linux-gnu-",
+ "x86_64-unknown-linux-gnu-",
+ "i686-pc-linux-gnu-",
+ "i586-pc-linux-gnu-",
+ "i486-pc-linux-gnu-",
+ "i386-pc-linux-gnu-",
+ "i686-linux-android-",
+ "i686-android-linux-",
+ NULL
+};
+
+const char *const mips_triplets[] = {
+ "mips-unknown-linux-gnu-",
+ "mipsel-linux-android-",
+ NULL
+};
+
+static bool lookup_path(char *name)
+{
+ bool found = false;
+ char *path, *tmp;
+ char buf[PATH_MAX];
+ char *env = getenv("PATH");
+
+ if (!env)
+ return false;
+
+ env = strdup(env);
+ if (!env)
+ return false;
+
+ path = strtok_r(env, ":", &tmp);
+ while (path) {
+ scnprintf(buf, sizeof(buf), "%s/%s", path, name);
+ if (access(buf, F_OK) == 0) {
+ found = true;
+ break;
+ }
+ path = strtok_r(NULL, ":", &tmp);
+ }
+ free(env);
+ return found;
+}
+
+static int lookup_triplets(const char *const *triplets, const char *name)
+{
+ int i;
+ char buf[PATH_MAX];
+
+ for (i = 0; triplets[i] != NULL; i++) {
+ scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
+ if (lookup_path(buf))
+ return i;
+ }
+ return -1;
+}
+
+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;
+ struct utsname uts;
+ const char *const *path_list;
+ char *buf = NULL;
+
+ if (uname(&uts) < 0)
+ goto out;
+
+ /*
+ * 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))
+ goto out;
+
+ cross_env = getenv("CROSS_COMPILE");
+ if (cross_env) {
+ if (asprintf(&buf, "%s%s", cross_env, name) < 0)
+ goto out_error;
+ if (buf[0] == '/') {
+ if (access(buf, F_OK) == 0)
+ goto out;
+ goto out_error;
+ }
+ if (lookup_path(buf))
+ goto out;
+ free(buf);
+ }
+
+ arch = env->arch;
+
+ if (!strcmp(arch, "arm"))
+ path_list = arm_triplets;
+ else if (!strcmp(arch, "powerpc"))
+ path_list = powerpc_triplets;
+ else if (!strcmp(arch, "sh"))
+ path_list = sh_triplets;
+ else if (!strcmp(arch, "s390"))
+ 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"))
+ path_list = x86_triplets;
+ else if (!strcmp(arch, "mips"))
+ path_list = mips_triplets;
+ else {
+ ui__error("binutils for %s not supported.\n", arch);
+ goto out_error;
+ }
+
+ idx = lookup_triplets(path_list, name);
+ if (idx < 0) {
+ ui__error("Please install %s for %s.\n"
+ "You can add it to PATH, set CROSS_COMPILE or "
+ "override the default using --%s.\n",
+ name, arch, name);
+ goto out_error;
+ }
+
+ if (asprintf(&buf, "%s%s", path_list[idx], name) < 0)
+ goto out_error;
+
+out:
+ *path = buf;
+ return 0;
+out_error:
+ free(buf);
+ *path = NULL;
+ return -1;
+}
+
+int perf_session_env__lookup_objdump(struct perf_session_env *env)
+{
+ return perf_session_env__lookup_binutils_path(env, "objdump",
+ &objdump_path);
+}
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
new file mode 100644
index 0000000..ede246e
--- /dev/null
+++ b/tools/perf/arch/common.h
@@ -0,0 +1,10 @@
+#ifndef ARCH_PERF_COMMON_H
+#define ARCH_PERF_COMMON_H
+
+#include "../util/session.h"
+
+extern const char *objdump_path;
+
+int perf_session_env__lookup_objdump(struct perf_session_env *env);
+
+#endif /* ARCH_PERF_COMMON_H */
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 690fa9a..c4bb645 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -28,6 +28,7 @@
#include "util/hist.h"
#include "util/session.h"
#include "util/tool.h"
+#include "arch/common.h"

#include <linux/bitmap.h>

@@ -186,6 +187,12 @@ static int __cmd_annotate(struct perf_annotate *ann)
goto out_delete;
}

+ if (!objdump_path) {
+ ret = perf_session_env__lookup_objdump(&session->header.env);
+ if (ret)
+ goto out_delete;
+ }
+
ret = perf_session__process_events(session, &ann->tool);
if (ret)
goto out_delete;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 5104a40..90d1162 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -33,6 +33,7 @@
#include "util/thread.h"
#include "util/sort.h"
#include "util/hist.h"
+#include "arch/common.h"

#include <linux/bitmap.h>

@@ -672,6 +673,12 @@ 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;

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 39242dc..a4dd25a 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -154,6 +154,5 @@ static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
#endif

extern const char *disassembler_style;
-extern const char *objdump_path;

#endif /* __PERF_ANNOTATE_H */