2010-12-16 12:00:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/4] perf/urgent fixes

Hi Ingo,

Please consider pulling from:

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

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (1):
perf buildid-list: Fix error return for success

Franck Bui-Huu (3):
perf probe: Fix use of kernel image path given by 'k' option
perf symbols: Stop using vmlinux files with no symbols
perf buildid-cache: Fix symbolic link handling

tools/perf/builtin-buildid-list.c | 3 +--
tools/perf/builtin-probe.c | 5 +++++
tools/perf/util/header.c | 10 ++++++----
tools/perf/util/probe-event.c | 15 ++++++++++++---
tools/perf/util/symbol.c | 4 ++--
tools/perf/util/symbol.h | 2 ++
6 files changed, 28 insertions(+), 11 deletions(-)


2010-12-16 12:00:35

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/4] perf probe: Fix use of kernel image path given by 'k' option

From: Franck Bui-Huu <[email protected]>

Users were not being able to have the explicitely specified vmlinux
pathname used, instead a search on the vmlinux path was always being
made.

Reported-by: Francis Moreau <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: [email protected]
Cc: Francis Moreau <[email protected]>
Cc: Franck Bui-Huu <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: Franck Bui-Huu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 5 +++++
tools/perf/util/probe-event.c | 15 ++++++++++++---
tools/perf/util/symbol.c | 4 ++--
tools/perf/util/symbol.h | 2 ++
4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 2e000c0..add163c 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -249,6 +249,11 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
!params.show_lines))
usage_with_options(probe_usage, options);

+ /*
+ * Only consider the user's kernel image path if given.
+ */
+ symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
+
if (params.list_events) {
if (params.mod_events) {
pr_err(" Error: Don't use --list with --add/--del.\n");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3b6a529..b71acd6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -114,6 +114,8 @@ static struct symbol *__find_kernel_function_by_name(const char *name,
const char *kernel_get_module_path(const char *module)
{
struct dso *dso;
+ struct map *map;
+ const char *vmlinux_name;

if (module) {
list_for_each_entry(dso, &machine.kernel_dsos, node) {
@@ -123,10 +125,17 @@ const char *kernel_get_module_path(const char *module)
}
pr_debug("Failed to find module %s.\n", module);
return NULL;
+ }
+
+ map = machine.vmlinux_maps[MAP__FUNCTION];
+ dso = map->dso;
+
+ vmlinux_name = symbol_conf.vmlinux_name;
+ if (vmlinux_name) {
+ if (dso__load_vmlinux(dso, map, vmlinux_name, NULL) <= 0)
+ return NULL;
} else {
- dso = machine.vmlinux_maps[MAP__FUNCTION]->dso;
- if (dso__load_vmlinux_path(dso,
- machine.vmlinux_maps[MAP__FUNCTION], NULL) < 0) {
+ if (dso__load_vmlinux_path(dso, map, NULL) < 0) {
pr_debug("Failed to load kernel map.\n");
return NULL;
}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index d628c8d..439ab94 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1780,8 +1780,8 @@ out_failure:
return -1;
}

-static int dso__load_vmlinux(struct dso *self, struct map *map,
- const char *vmlinux, symbol_filter_t filter)
+int dso__load_vmlinux(struct dso *self, struct map *map,
+ const char *vmlinux, symbol_filter_t filter)
{
int err = -1, fd;

diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 038f220..6c6eafd 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -166,6 +166,8 @@ void dso__sort_by_name(struct dso *self, enum map_type type);
struct dso *__dsos__findnew(struct list_head *head, const char *name);

int dso__load(struct dso *self, struct map *map, symbol_filter_t filter);
+int dso__load_vmlinux(struct dso *self, struct map *map,
+ const char *vmlinux, symbol_filter_t filter);
int dso__load_vmlinux_path(struct dso *self, struct map *map,
symbol_filter_t filter);
int dso__load_kallsyms(struct dso *self, const char *filename, struct map *map,
--
1.6.2.5

2010-12-16 12:00:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/4] perf symbols: Stop using vmlinux files with no symbols

From: Franck Bui-Huu <[email protected]>

Fail if the kernel image contains no symbol, allowing using other images
in the vmlinux search path that may have a usable symtab.

Acked-by: Masami Hiramatsu <[email protected]>
Cc: [email protected]
Cc: Francis Moreau <[email protected]>
Cc: Franck Bui-Huu <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: Franck Bui-Huu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b71acd6..61191c6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -135,7 +135,7 @@ const char *kernel_get_module_path(const char *module)
if (dso__load_vmlinux(dso, map, vmlinux_name, NULL) <= 0)
return NULL;
} else {
- if (dso__load_vmlinux_path(dso, map, NULL) < 0) {
+ if (dso__load_vmlinux_path(dso, map, NULL) <= 0) {
pr_debug("Failed to load kernel map.\n");
return NULL;
}
--
1.6.2.5

2010-12-16 12:00:37

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 4/4] perf buildid-list: Fix error return for success

From: Arnaldo Carvalho de Melo <[email protected]>

It was always returning -1 (255), confusing test scripts.

Reported-by: Han Pingtian <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Han Pingtian <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-buildid-list.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 44a47e1..c49837d 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -36,7 +36,6 @@ static const struct option options[] = {

static int __cmd_buildid_list(void)
{
- int err = -1;
struct perf_session *session;

session = perf_session__new(input_name, O_RDONLY, force, false);
@@ -49,7 +48,7 @@ static int __cmd_buildid_list(void)
perf_session__fprintf_dsos_buildid(session, stdout, with_hits);

perf_session__delete(session);
- return err;
+ return 0;
}

int cmd_buildid_list(int argc, const char **argv, const char *prefix __used)
--
1.6.2.5

2010-12-16 12:01:25

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/4] perf buildid-cache: Fix symbolic link handling

From: Franck Bui-Huu <[email protected]>

This was broken since link(2) doesn't dereference symbolic
links. Instead 'filename' becomes a symbolic link to the same file
that 'name' refers to.

This had the bad effect to create dangling symlinks in the case that
even can't be removed with perf-buildid-cache(1).

LKML-Reference: <[email protected]>
Signed-off-by: Franck Bui-Huu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/header.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 64a85ba..7cba055 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -265,15 +265,16 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
const char *name, bool is_kallsyms)
{
const size_t size = PATH_MAX;
- char *filename = malloc(size),
+ char *realname = realpath(name, NULL),
+ *filename = malloc(size),
*linkname = malloc(size), *targetname;
int len, err = -1;

- if (filename == NULL || linkname == NULL)
+ if (realname == NULL || filename == NULL || linkname == NULL)
goto out_free;

len = snprintf(filename, size, "%s%s%s",
- debugdir, is_kallsyms ? "/" : "", name);
+ debugdir, is_kallsyms ? "/" : "", realname);
if (mkdir_p(filename, 0755))
goto out_free;

@@ -283,7 +284,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
if (is_kallsyms) {
if (copyfile("/proc/kallsyms", filename))
goto out_free;
- } else if (link(name, filename) && copyfile(name, filename))
+ } else if (link(realname, filename) && copyfile(name, filename))
goto out_free;
}

@@ -300,6 +301,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
if (symlink(targetname, linkname) == 0)
err = 0;
out_free:
+ free(realname);
free(filename);
free(linkname);
return err;
--
1.6.2.5