2010-07-16 18:09:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/3] perf/core fixes and improvements

Hi Ingo,

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core

Regards,

- Arnaldo

Masami Hiramatsu (3):
perf probe: Fix error message if get_real_path() failed
perf probe: Support comp_dir to find an absolute source path
perf probe: Fix the logic of die_compare_name

tools/perf/util/probe-event.c | 69 ++++++++++++++++++++++++++++
tools/perf/util/probe-event.h | 1 +
tools/perf/util/probe-finder.c | 96 ++++++++++++++--------------------------
3 files changed, 103 insertions(+), 63 deletions(-)


2010-07-16 18:09:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/3] perf probe: Fix the logic of die_compare_name

From: Masami Hiramatsu <[email protected]>

Invert the return value of die_compare_name(), because it returns a 'bool'
result which should be expeced true if the die's name is same as compared
string.

LKML-Reference: <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-finder.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 37dcdb6..f88070e 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -158,7 +158,7 @@ static bool die_compare_name(Dwarf_Die *dw_die, const char *tname)
{
const char *name;
name = dwarf_diename(dw_die);
- return name ? strcmp(tname, name) : -1;
+ return name ? (strcmp(tname, name) == 0) : false;
}

/* Get type die, but skip qualifiers and typedef */
@@ -329,7 +329,7 @@ static int __die_find_variable_cb(Dwarf_Die *die_mem, void *data)
tag = dwarf_tag(die_mem);
if ((tag == DW_TAG_formal_parameter ||
tag == DW_TAG_variable) &&
- (die_compare_name(die_mem, name) == 0))
+ die_compare_name(die_mem, name))
return DIE_FIND_CB_FOUND;

return DIE_FIND_CB_CONTINUE;
@@ -348,7 +348,7 @@ static int __die_find_member_cb(Dwarf_Die *die_mem, void *data)
const char *name = data;

if ((dwarf_tag(die_mem) == DW_TAG_member) &&
- (die_compare_name(die_mem, name) == 0))
+ die_compare_name(die_mem, name))
return DIE_FIND_CB_FOUND;

return DIE_FIND_CB_SIBLING;
@@ -506,8 +506,8 @@ static int convert_variable_type(Dwarf_Die *vr_die,
return -ENOMEM;
}
}
- if (die_compare_name(&type, "char") != 0 &&
- die_compare_name(&type, "unsigned char") != 0) {
+ if (!die_compare_name(&type, "char") &&
+ !die_compare_name(&type, "unsigned char")) {
pr_warning("Failed to cast into string: "
"%s is not (unsigned) char *.",
dwarf_diename(vr_die));
@@ -1017,7 +1017,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)

/* Check tag and diename */
if (dwarf_tag(sp_die) != DW_TAG_subprogram ||
- die_compare_name(sp_die, pp->function) != 0)
+ !die_compare_name(sp_die, pp->function))
return DWARF_CB_OK;

pf->fname = dwarf_decl_file(sp_die);
@@ -1340,7 +1340,7 @@ static int line_range_search_cb(Dwarf_Die *sp_die, void *data)
struct line_range *lr = lf->lr;

if (dwarf_tag(sp_die) == DW_TAG_subprogram &&
- die_compare_name(sp_die, lr->function) == 0) {
+ die_compare_name(sp_die, lr->function)) {
lf->fname = dwarf_decl_file(sp_die);
dwarf_decl_line(sp_die, &lr->offset);
pr_debug("fname: %s, lineno:%d\n", lf->fname, lr->offset);
--
1.6.2.5

2010-07-16 18:09:49

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/3] perf probe: Fix error message if get_real_path() failed

From: Masami Hiramatsu <[email protected]>

Perf probe -L shows incorrect error message (Dwarf error) if it fails to find
source file. This can confuse users.

# ./perf probe -s /nowhere -L vfs_read
Debuginfo analysis failed. (-2)
Error: Failed to show lines. (-2)

With this patch, it shows correct message.

# ./perf probe -s /nowhere -L vfs_read
Failed to find source file. (-2)
Error: Failed to show lines. (-2)

LKML-Reference: <[email protected]>
Cc: Chase Douglas <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Acked-by: Chase Douglas <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 59 ++++++++++++++++++++++++++++++++++++++
tools/perf/util/probe-finder.c | 61 +++------------------------------------
2 files changed, 64 insertions(+), 56 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 09cf546..8d08e75 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -195,6 +195,55 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
return ntevs;
}

+/*
+ * Find a src file from a DWARF tag path. Prepend optional source path prefix
+ * and chop off leading directories that do not exist. Result is passed back as
+ * a newly allocated path on success.
+ * Return 0 if file was found and readable, -errno otherwise.
+ */
+static int get_real_path(const char *raw_path, char **new_path)
+{
+ if (!symbol_conf.source_prefix) {
+ if (access(raw_path, R_OK) == 0) {
+ *new_path = strdup(raw_path);
+ return 0;
+ } else
+ return -errno;
+ }
+
+ *new_path = malloc((strlen(symbol_conf.source_prefix) +
+ strlen(raw_path) + 2));
+ if (!*new_path)
+ return -ENOMEM;
+
+ for (;;) {
+ sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
+ raw_path);
+
+ if (access(*new_path, R_OK) == 0)
+ return 0;
+
+ switch (errno) {
+ case ENAMETOOLONG:
+ case ENOENT:
+ case EROFS:
+ case EFAULT:
+ raw_path = strchr(++raw_path, '/');
+ if (!raw_path) {
+ free(*new_path);
+ *new_path = NULL;
+ return -ENOENT;
+ }
+ continue;
+
+ default:
+ free(*new_path);
+ *new_path = NULL;
+ return -errno;
+ }
+ }
+}
+
#define LINEBUF_SIZE 256
#define NR_ADDITIONAL_LINES 2

@@ -244,6 +293,7 @@ int show_line_range(struct line_range *lr)
struct line_node *ln;
FILE *fp;
int fd, ret;
+ char *tmp;

/* Search a line range */
ret = init_vmlinux();
@@ -266,6 +316,15 @@ int show_line_range(struct line_range *lr)
return ret;
}

+ /* Convert source file path */
+ tmp = lr->path;
+ ret = get_real_path(tmp, &lr->path);
+ free(tmp); /* Free old path */
+ if (ret < 0) {
+ pr_warning("Failed to find source file. (%d)\n", ret);
+ return ret;
+ }
+
setup_pager();

if (lr->function)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 3e64e1f..a934a36 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -58,55 +58,6 @@ static int strtailcmp(const char *s1, const char *s2)
return 0;
}

-/*
- * Find a src file from a DWARF tag path. Prepend optional source path prefix
- * and chop off leading directories that do not exist. Result is passed back as
- * a newly allocated path on success.
- * Return 0 if file was found and readable, -errno otherwise.
- */
-static int get_real_path(const char *raw_path, char **new_path)
-{
- if (!symbol_conf.source_prefix) {
- if (access(raw_path, R_OK) == 0) {
- *new_path = strdup(raw_path);
- return 0;
- } else
- return -errno;
- }
-
- *new_path = malloc((strlen(symbol_conf.source_prefix) +
- strlen(raw_path) + 2));
- if (!*new_path)
- return -ENOMEM;
-
- for (;;) {
- sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
- raw_path);
-
- if (access(*new_path, R_OK) == 0)
- return 0;
-
- switch (errno) {
- case ENAMETOOLONG:
- case ENOENT:
- case EROFS:
- case EFAULT:
- raw_path = strchr(++raw_path, '/');
- if (!raw_path) {
- free(*new_path);
- *new_path = NULL;
- return -ENOENT;
- }
- continue;
-
- default:
- free(*new_path);
- *new_path = NULL;
- return -errno;
- }
- }
-}
-
/* Line number list operations */

/* Add a line to line number list */
@@ -1256,13 +1207,11 @@ end:
static int line_range_add_line(const char *src, unsigned int lineno,
struct line_range *lr)
{
- int ret;
-
- /* Copy real path */
+ /* Copy source path */
if (!lr->path) {
- ret = get_real_path(src, &lr->path);
- if (ret != 0)
- return ret;
+ lr->path = strdup(src);
+ if (lr->path == NULL)
+ return -ENOMEM;
}
return line_list__add_line(&lr->line_list, lineno);
}
@@ -1460,7 +1409,7 @@ int find_line_range(int fd, struct line_range *lr)
}
off = noff;
}
- pr_debug("path: %lx\n", (unsigned long)lr->path);
+ pr_debug("path: %s\n", lr->path);
dwarf_end(dbg);

return (ret < 0) ? ret : lf.found;
--
1.6.2.5

2010-07-16 18:10:16

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/3] perf probe: Support comp_dir to find an absolute source path

From: Masami Hiramatsu <[email protected]>

Gcc generates DW_AT_comp_dir and stores relative source path if building kernel
without O= option. In that case, perf probe --line sometimes doesn't work
without --source option, because it tries to access relative source path.

This adds DW_AT_comp_dir support to perf probe for finding an absolute source
path when no --source option.

LKML-Reference: <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 34 ++++++++++++++++++++++------------
tools/perf/util/probe-event.h | 1 +
tools/perf/util/probe-finder.c | 21 +++++++++++++++++++++
3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8d08e75..4445a1e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -201,28 +201,38 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
* a newly allocated path on success.
* Return 0 if file was found and readable, -errno otherwise.
*/
-static int get_real_path(const char *raw_path, char **new_path)
+static int get_real_path(const char *raw_path, const char *comp_dir,
+ char **new_path)
{
- if (!symbol_conf.source_prefix) {
- if (access(raw_path, R_OK) == 0) {
- *new_path = strdup(raw_path);
- return 0;
- } else
- return -errno;
+ const char *prefix = symbol_conf.source_prefix;
+
+ if (!prefix) {
+ if (raw_path[0] != '/' && comp_dir)
+ /* If not an absolute path, try to use comp_dir */
+ prefix = comp_dir;
+ else {
+ if (access(raw_path, R_OK) == 0) {
+ *new_path = strdup(raw_path);
+ return 0;
+ } else
+ return -errno;
+ }
}

- *new_path = malloc((strlen(symbol_conf.source_prefix) +
- strlen(raw_path) + 2));
+ *new_path = malloc((strlen(prefix) + strlen(raw_path) + 2));
if (!*new_path)
return -ENOMEM;

for (;;) {
- sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
- raw_path);
+ sprintf(*new_path, "%s/%s", prefix, raw_path);

if (access(*new_path, R_OK) == 0)
return 0;

+ if (!symbol_conf.source_prefix)
+ /* In case of searching comp_dir, don't retry */
+ return -errno;
+
switch (errno) {
case ENAMETOOLONG:
case ENOENT:
@@ -318,7 +328,7 @@ int show_line_range(struct line_range *lr)

/* Convert source file path */
tmp = lr->path;
- ret = get_real_path(tmp, &lr->path);
+ ret = get_real_path(tmp, lr->comp_dir, &lr->path);
free(tmp); /* Free old path */
if (ret < 0) {
pr_warning("Failed to find source file. (%d)\n", ret);
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index bc06d3e..ed362ac 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -86,6 +86,7 @@ struct line_range {
int end; /* End line number */
int offset; /* Start line offset */
char *path; /* Real path name */
+ char *comp_dir; /* Compile directory */
struct list_head line_list; /* Visible lines */
};

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index a934a36..37dcdb6 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -144,6 +144,15 @@ static const char *cu_find_realpath(Dwarf_Die *cu_die, const char *fname)
return src;
}

+/* Get DW_AT_comp_dir (should be NULL with older gcc) */
+static const char *cu_get_comp_dir(Dwarf_Die *cu_die)
+{
+ Dwarf_Attribute attr;
+ if (dwarf_attr(cu_die, DW_AT_comp_dir, &attr) == NULL)
+ return NULL;
+ return dwarf_formstring(&attr);
+}
+
/* Compare diename and tname */
static bool die_compare_name(Dwarf_Die *dw_die, const char *tname)
{
@@ -1374,6 +1383,7 @@ int find_line_range(int fd, struct line_range *lr)
size_t cuhl;
Dwarf_Die *diep;
Dwarf *dbg;
+ const char *comp_dir;

dbg = dwarf_begin(fd, DWARF_C_READ);
if (!dbg) {
@@ -1409,6 +1419,17 @@ int find_line_range(int fd, struct line_range *lr)
}
off = noff;
}
+
+ /* Store comp_dir */
+ if (lf.found) {
+ comp_dir = cu_get_comp_dir(&lf.cu_die);
+ if (comp_dir) {
+ lr->comp_dir = strdup(comp_dir);
+ if (!lr->comp_dir)
+ ret = -ENOMEM;
+ }
+ }
+
pr_debug("path: %s\n", lr->path);
dwarf_end(dbg);

--
1.6.2.5

2010-07-17 09:35:44

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/3] perf/core fixes and improvements


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
>
> Regards,
>
> - Arnaldo
>
> Masami Hiramatsu (3):
> perf probe: Fix error message if get_real_path() failed
> perf probe: Support comp_dir to find an absolute source path
> perf probe: Fix the logic of die_compare_name
>
> tools/perf/util/probe-event.c | 69 ++++++++++++++++++++++++++++
> tools/perf/util/probe-event.h | 1 +
> tools/perf/util/probe-finder.c | 96 ++++++++++++++--------------------------
> 3 files changed, 103 insertions(+), 63 deletions(-)

Pulled, thanks Arnaldo!

Ingo