* [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer
@ 2010-02-03 18:52 Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 2/9] perf symbols: Fixup vsyscall maps Arnaldo Carvalho de Melo
` (8 more replies)
0 siblings, 9 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
I noticed while writing the first test in 'perf regtest' that to just test the
symbol handling routines one needs to create a perf session, that is a layer
centered on a perf.data file, events, etc, so I untied these layers.
This reduces the complexity for the users as the number of parameters to most
of the symbols and session APIs now was reduced while not adding more state to
all the map instances by only having data that is needed to split the kernel
(kallsyms and ELF symtab sections) maps and do vmlinux relocation on the main
kernel map.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-probe.c | 5 +-
tools/perf/util/event.c | 6 +--
tools/perf/util/map.c | 20 ++++---
tools/perf/util/map.h | 22 ++++++--
tools/perf/util/session.c | 35 +++++++++----
tools/perf/util/session.h | 22 +++++---
tools/perf/util/symbol.c | 122 ++++++++++++++++++++-----------------------
tools/perf/util/symbol.h | 19 ++++---
tools/perf/util/thread.c | 3 +-
tools/perf/util/thread.h | 14 +++--
11 files changed, 149 insertions(+), 121 deletions(-)
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 5d5dc6b..924a951 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -369,7 +369,7 @@ static void __print_result(struct rb_root *root, struct perf_session *session,
if (is_caller) {
addr = data->call_site;
if (!raw_ip)
- sym = map_groups__find_function(&session->kmaps, session, addr, NULL);
+ sym = map_groups__find_function(&session->kmaps, addr, NULL);
} else
addr = data->ptr;
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 34f2acb..4fa73ec 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -122,8 +122,7 @@ static int opt_del_probe_event(const struct option *opt __used,
static void evaluate_probe_point(struct probe_point *pp)
{
struct symbol *sym;
- sym = map__find_symbol_by_name(session.kmap, pp->function,
- session.psession, NULL);
+ sym = map__find_symbol_by_name(session.kmap, pp->function, NULL);
if (!sym)
die("Kernel symbol \'%s\' not found - probe not added.",
pp->function);
@@ -132,7 +131,7 @@ static void evaluate_probe_point(struct probe_point *pp)
#ifndef NO_LIBDWARF
static int open_vmlinux(void)
{
- if (map__load(session.kmap, session.psession, NULL) < 0) {
+ if (map__load(session.kmap, NULL) < 0) {
pr_debug("Failed to load kernel map.\n");
return -EINVAL;
}
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index bbaee61..c3831f6 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -374,9 +374,7 @@ int event__process_mmap(event_t *self, struct perf_session *session)
goto out_problem;
kernel->kernel = 1;
- if (__map_groups__create_kernel_maps(&session->kmaps,
- session->vmlinux_maps,
- kernel) < 0)
+ if (__perf_session__create_kernel_maps(session, kernel) < 0)
goto out_problem;
session->vmlinux_maps[MAP__FUNCTION]->start = self->mmap.start;
@@ -476,7 +474,7 @@ void thread__find_addr_location(struct thread *self,
{
thread__find_addr_map(self, session, cpumode, type, addr, al);
if (al->map != NULL)
- al->sym = map__find_symbol(al->map, session, al->addr, filter);
+ al->sym = map__find_symbol(al->map, al->addr, filter);
else
al->sym = NULL;
}
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index c4d55a0..36ff0bf 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -104,8 +104,7 @@ void map__fixup_end(struct map *self)
#define DSO__DELETED "(deleted)"
-int map__load(struct map *self, struct perf_session *session,
- symbol_filter_t filter)
+int map__load(struct map *self, symbol_filter_t filter)
{
const char *name = self->dso->long_name;
int nr;
@@ -113,7 +112,7 @@ int map__load(struct map *self, struct perf_session *session,
if (dso__loaded(self->dso, self->type))
return 0;
- nr = dso__load(self->dso, self, session, filter);
+ nr = dso__load(self->dso, self, filter);
if (nr < 0) {
if (self->dso->has_build_id) {
char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -144,24 +143,29 @@ int map__load(struct map *self, struct perf_session *session,
return -1;
}
+ /*
+ * Only applies to the kernel, as its symtabs aren't relative like the
+ * module ones.
+ */
+ if (self->dso->kernel)
+ map__reloc_vmlinux(self);
return 0;
}
-struct symbol *map__find_symbol(struct map *self, struct perf_session *session,
- u64 addr, symbol_filter_t filter)
+struct symbol *map__find_symbol(struct map *self, u64 addr,
+ symbol_filter_t filter)
{
- if (map__load(self, session, filter) < 0)
+ if (map__load(self, filter) < 0)
return NULL;
return dso__find_symbol(self->dso, self->type, addr);
}
struct symbol *map__find_symbol_by_name(struct map *self, const char *name,
- struct perf_session *session,
symbol_filter_t filter)
{
- if (map__load(self, session, filter) < 0)
+ if (map__load(self, filter) < 0)
return NULL;
if (!dso__sorted_by_name(self->dso, self->type))
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 72f0b6a..de04839 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -14,6 +14,8 @@ enum map_type {
#define MAP__NR_TYPES (MAP__VARIABLE + 1)
struct dso;
+struct ref_reloc_sym;
+struct map_groups;
struct map {
union {
@@ -29,6 +31,16 @@ struct map {
struct dso *dso;
};
+struct kmap {
+ struct ref_reloc_sym *ref_reloc_sym;
+ struct map_groups *kmaps;
+};
+
+static inline struct kmap *map__kmap(struct map *self)
+{
+ return (struct kmap *)(self + 1);
+}
+
static inline u64 map__map_ip(struct map *map, u64 ip)
{
return ip - map->start + map->pgoff;
@@ -58,16 +70,14 @@ struct map *map__clone(struct map *self);
int map__overlap(struct map *l, struct map *r);
size_t map__fprintf(struct map *self, FILE *fp);
-struct perf_session;
-
-int map__load(struct map *self, struct perf_session *session,
- symbol_filter_t filter);
-struct symbol *map__find_symbol(struct map *self, struct perf_session *session,
+int map__load(struct map *self, symbol_filter_t filter);
+struct symbol *map__find_symbol(struct map *self,
u64 addr, symbol_filter_t filter);
struct symbol *map__find_symbol_by_name(struct map *self, const char *name,
- struct perf_session *session,
symbol_filter_t filter);
void map__fixup_start(struct map *self);
void map__fixup_end(struct map *self);
+void map__reloc_vmlinux(struct map *self);
+
#endif /* __PERF_MAP_H */
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index cf91d09..aa8a031 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -53,6 +53,11 @@ out_close:
return -1;
}
+static inline int perf_session__create_kernel_maps(struct perf_session *self)
+{
+ return map_groups__create_kernel_maps(&self->kmaps, self->vmlinux_maps);
+}
+
struct perf_session *perf_session__new(const char *filename, int mode, bool force)
{
size_t len = filename ? strlen(filename) + 1 : 0;
@@ -507,6 +512,7 @@ int perf_session__set_kallsyms_ref_reloc_sym(struct perf_session *self,
u64 addr)
{
char *bracket;
+ enum map_type i;
self->ref_reloc_sym.name = strdup(symbol_name);
if (self->ref_reloc_sym.name == NULL)
@@ -517,6 +523,12 @@ int perf_session__set_kallsyms_ref_reloc_sym(struct perf_session *self,
*bracket = '\0';
self->ref_reloc_sym.addr = addr;
+
+ for (i = 0; i < MAP__NR_TYPES; ++i) {
+ struct kmap *kmap = map__kmap(self->vmlinux_maps[i]);
+ kmap->ref_reloc_sym = &self->ref_reloc_sym;
+ }
+
return 0;
}
@@ -530,20 +542,21 @@ static u64 map__reloc_unmap_ip(struct map *map, u64 ip)
return ip - (s64)map->pgoff;
}
-void perf_session__reloc_vmlinux_maps(struct perf_session *self,
- u64 unrelocated_addr)
+void map__reloc_vmlinux(struct map *self)
{
- enum map_type type;
- s64 reloc = unrelocated_addr - self->ref_reloc_sym.addr;
+ struct kmap *kmap = map__kmap(self);
+ s64 reloc;
- if (!reloc)
+ if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->unrelocated_addr)
return;
- for (type = 0; type < MAP__NR_TYPES; ++type) {
- struct map *map = self->vmlinux_maps[type];
+ reloc = (kmap->ref_reloc_sym->unrelocated_addr -
+ kmap->ref_reloc_sym->addr);
- map->map_ip = map__reloc_map_ip;
- map->unmap_ip = map__reloc_unmap_ip;
- map->pgoff = reloc;
- }
+ if (!reloc)
+ return;
+
+ self->map_ip = map__reloc_map_ip;
+ self->unmap_ip = map__reloc_unmap_ip;
+ self->pgoff = reloc;
}
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 36d1a80..752d75a 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -3,13 +3,13 @@
#include "event.h"
#include "header.h"
+#include "symbol.h"
#include "thread.h"
#include <linux/rbtree.h>
#include "../../../include/linux/perf_event.h"
struct ip_callchain;
struct thread;
-struct symbol;
struct perf_session {
struct perf_header header;
@@ -24,10 +24,7 @@ struct perf_session {
unsigned long unknown_events;
struct rb_root hists;
u64 sample_type;
- struct {
- const char *name;
- u64 addr;
- } ref_reloc_sym;
+ struct ref_reloc_sym ref_reloc_sym;
int fd;
int cwdlen;
char *cwd;
@@ -69,9 +66,20 @@ int perf_header__read_build_ids(struct perf_header *self, int input,
int perf_session__set_kallsyms_ref_reloc_sym(struct perf_session *self,
const char *symbol_name,
u64 addr);
-void perf_session__reloc_vmlinux_maps(struct perf_session *self,
- u64 unrelocated_addr);
void mem_bswap_64(void *src, int byte_size);
+static inline int __perf_session__create_kernel_maps(struct perf_session *self,
+ struct dso *kernel)
+{
+ return __map_groups__create_kernel_maps(&self->kmaps,
+ self->vmlinux_maps, kernel);
+}
+
+static inline struct map *
+ perf_session__new_module_map(struct perf_session *self,
+ u64 start, const char *filename)
+{
+ return map_groups__new_module(&self->kmaps, start, filename);
+}
#endif /* __PERF_SESSION_H */
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f9049d1..6138742 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1,6 +1,5 @@
#include "util.h"
#include "../perf.h"
-#include "session.h"
#include "sort.h"
#include "string.h"
#include "symbol.h"
@@ -34,7 +33,7 @@ enum dso_origin {
static void dsos__add(struct list_head *head, struct dso *dso);
static struct map *map__new2(u64 start, struct dso *dso, enum map_type type);
static int dso__load_kernel_sym(struct dso *self, struct map *map,
- struct perf_session *session, symbol_filter_t filter);
+ symbol_filter_t filter);
static int vmlinux_path__nr_entries;
static char **vmlinux_path;
@@ -480,8 +479,9 @@ static int dso__load_all_kallsyms(struct dso *self, const char *filename,
* the original ELF section names vmlinux have.
*/
static int dso__split_kallsyms(struct dso *self, struct map *map,
- struct perf_session *session, symbol_filter_t filter)
+ symbol_filter_t filter)
{
+ struct map_groups *kmaps = map__kmap(map)->kmaps;
struct map *curr_map = map;
struct symbol *pos;
int count = 0;
@@ -503,7 +503,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
*module++ = '\0';
if (strcmp(curr_map->dso->short_name, module)) {
- curr_map = map_groups__find_by_name(&session->kmaps, map->type, module);
+ curr_map = map_groups__find_by_name(kmaps, map->type, module);
if (curr_map == NULL) {
pr_debug("/proc/{kallsyms,modules} "
"inconsistency while looking "
@@ -538,7 +538,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
}
curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
- map_groups__insert(&session->kmaps, curr_map);
+ map_groups__insert(kmaps, curr_map);
++kernel_range;
}
@@ -557,9 +557,8 @@ discard_symbol: rb_erase(&pos->rb_node, root);
return count;
}
-
-static int dso__load_kallsyms(struct dso *self, const char *filename, struct map *map,
- struct perf_session *session, symbol_filter_t filter)
+int dso__load_kallsyms(struct dso *self, const char *filename,
+ struct map *map, symbol_filter_t filter)
{
if (dso__load_all_kallsyms(self, filename, map) < 0)
return -1;
@@ -567,7 +566,7 @@ static int dso__load_kallsyms(struct dso *self, const char *filename, struct map
symbols__fixup_end(&self->symbols[map->type]);
self->origin = DSO__ORIG_KERNEL;
- return dso__split_kallsyms(self, map, session, filter);
+ return dso__split_kallsyms(self, map, filter);
}
static int dso__load_perf_map(struct dso *self, struct map *map,
@@ -893,10 +892,10 @@ static bool elf_sec__is_a(GElf_Shdr *self, Elf_Data *secstrs, enum map_type type
}
}
-static int dso__load_sym(struct dso *self, struct map *map,
- struct perf_session *session, const char *name, int fd,
- symbol_filter_t filter, int kernel, int kmodule)
+static int dso__load_sym(struct dso *self, struct map *map, const char *name,
+ int fd, symbol_filter_t filter, int kmodule)
{
+ struct kmap *kmap = self->kernel ? map__kmap(map) : NULL;
struct map *curr_map = map;
struct dso *curr_dso = self;
size_t dso_name_len = strlen(self->short_name);
@@ -953,7 +952,7 @@ static int dso__load_sym(struct dso *self, struct map *map,
nr_syms = shdr.sh_size / shdr.sh_entsize;
memset(&sym, 0, sizeof(sym));
- if (!kernel) {
+ if (!self->kernel) {
self->adjust_symbols = (ehdr.e_type == ET_EXEC ||
elf_section_by_name(elf, &ehdr, &shdr,
".gnu.prelink_undo",
@@ -967,9 +966,9 @@ static int dso__load_sym(struct dso *self, struct map *map,
int is_label = elf_sym__is_label(&sym);
const char *section_name;
- if (kernel && session->ref_reloc_sym.name != NULL &&
- strcmp(elf_name, session->ref_reloc_sym.name) == 0)
- perf_session__reloc_vmlinux_maps(session, sym.st_value);
+ if (kmap && kmap->ref_reloc_sym && kmap->ref_reloc_sym->name &&
+ strcmp(elf_name, kmap->ref_reloc_sym->name) == 0)
+ kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
if (!is_label && !elf_sym__is_a(&sym, map->type))
continue;
@@ -985,7 +984,7 @@ static int dso__load_sym(struct dso *self, struct map *map,
section_name = elf_sec__name(&shdr, secstrs);
- if (kernel || kmodule) {
+ if (self->kernel || kmodule) {
char dso_name[PATH_MAX];
if (strcmp(section_name,
@@ -1001,7 +1000,7 @@ static int dso__load_sym(struct dso *self, struct map *map,
snprintf(dso_name, sizeof(dso_name),
"%s%s", self->short_name, section_name);
- curr_map = map_groups__find_by_name(&session->kmaps, map->type, dso_name);
+ curr_map = map_groups__find_by_name(kmap->kmaps, map->type, dso_name);
if (curr_map == NULL) {
u64 start = sym.st_value;
@@ -1020,7 +1019,7 @@ static int dso__load_sym(struct dso *self, struct map *map,
curr_map->map_ip = identity__map_ip;
curr_map->unmap_ip = identity__map_ip;
curr_dso->origin = DSO__ORIG_KERNEL;
- map_groups__insert(&session->kmaps, curr_map);
+ map_groups__insert(kmap->kmaps, curr_map);
dsos__add(&dsos__kernel, curr_dso);
} else
curr_dso = curr_map->dso;
@@ -1236,8 +1235,7 @@ char dso__symtab_origin(const struct dso *self)
return origin[self->origin];
}
-int dso__load(struct dso *self, struct map *map, struct perf_session *session,
- symbol_filter_t filter)
+int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
{
int size = PATH_MAX;
char *name;
@@ -1249,7 +1247,7 @@ int dso__load(struct dso *self, struct map *map, struct perf_session *session,
dso__set_loaded(self, map->type);
if (self->kernel)
- return dso__load_kernel_sym(self, map, session, filter);
+ return dso__load_kernel_sym(self, map, filter);
name = malloc(size);
if (!name)
@@ -1320,7 +1318,7 @@ open_file:
fd = open(name, O_RDONLY);
} while (fd < 0);
- ret = dso__load_sym(self, map, NULL, name, fd, filter, 0, 0);
+ ret = dso__load_sym(self, map, name, fd, filter, 0);
close(fd);
/*
@@ -1376,7 +1374,7 @@ static int dso__kernel_module_get_build_id(struct dso *self)
return 0;
}
-static int perf_session__set_modules_path_dir(struct perf_session *self, char *dirname)
+static int map_groups__set_modules_path_dir(struct map_groups *self, char *dirname)
{
struct dirent *dent;
DIR *dir = opendir(dirname);
@@ -1396,7 +1394,7 @@ static int perf_session__set_modules_path_dir(struct perf_session *self, char *d
snprintf(path, sizeof(path), "%s/%s",
dirname, dent->d_name);
- if (perf_session__set_modules_path_dir(self, path) < 0)
+ if (map_groups__set_modules_path_dir(self, path) < 0)
goto failure;
} else {
char *dot = strrchr(dent->d_name, '.'),
@@ -1410,7 +1408,7 @@ static int perf_session__set_modules_path_dir(struct perf_session *self, char *d
(int)(dot - dent->d_name), dent->d_name);
strxfrchar(dso_name, '-', '_');
- map = map_groups__find_by_name(&self->kmaps, MAP__FUNCTION, dso_name);
+ map = map_groups__find_by_name(self, MAP__FUNCTION, dso_name);
if (map == NULL)
continue;
@@ -1431,7 +1429,7 @@ failure:
return -1;
}
-static int perf_session__set_modules_path(struct perf_session *self)
+static int map_groups__set_modules_path(struct map_groups *self)
{
struct utsname uts;
char modules_path[PATH_MAX];
@@ -1442,7 +1440,7 @@ static int perf_session__set_modules_path(struct perf_session *self)
snprintf(modules_path, sizeof(modules_path), "/lib/modules/%s/kernel",
uts.release);
- return perf_session__set_modules_path_dir(self, modules_path);
+ return map_groups__set_modules_path_dir(self, modules_path);
}
/*
@@ -1452,8 +1450,8 @@ static int perf_session__set_modules_path(struct perf_session *self)
*/
static struct map *map__new2(u64 start, struct dso *dso, enum map_type type)
{
- struct map *self = malloc(sizeof(*self));
-
+ struct map *self = zalloc(sizeof(*self) +
+ (dso->kernel ? sizeof(struct kmap) : 0));
if (self != NULL) {
/*
* ->end will be filled after we load all the symbols
@@ -1464,8 +1462,8 @@ static struct map *map__new2(u64 start, struct dso *dso, enum map_type type)
return self;
}
-struct map *perf_session__new_module_map(struct perf_session *self, u64 start,
- const char *filename)
+struct map *map_groups__new_module(struct map_groups *self, u64 start,
+ const char *filename)
{
struct map *map;
struct dso *dso = __dsos__findnew(&dsos__kernel, filename);
@@ -1478,11 +1476,11 @@ struct map *perf_session__new_module_map(struct perf_session *self, u64 start,
return NULL;
dso->origin = DSO__ORIG_KMODULE;
- map_groups__insert(&self->kmaps, map);
+ map_groups__insert(self, map);
return map;
}
-static int perf_session__create_module_maps(struct perf_session *self)
+static int map_groups__create_modules(struct map_groups *self)
{
char *line = NULL;
size_t n;
@@ -1520,7 +1518,7 @@ static int perf_session__create_module_maps(struct perf_session *self)
*sep = '\0';
snprintf(name, sizeof(name), "[%s]", line);
- map = perf_session__new_module_map(self, start, name);
+ map = map_groups__new_module(self, start, name);
if (map == NULL)
goto out_delete_line;
dso__kernel_module_get_build_id(map->dso);
@@ -1529,7 +1527,7 @@ static int perf_session__create_module_maps(struct perf_session *self)
free(line);
fclose(file);
- return perf_session__set_modules_path(self);
+ return map_groups__set_modules_path(self);
out_delete_line:
free(line);
@@ -1538,7 +1536,6 @@ out_failure:
}
static int dso__load_vmlinux(struct dso *self, struct map *map,
- struct perf_session *session,
const char *vmlinux, symbol_filter_t filter)
{
int err = -1, fd;
@@ -1572,14 +1569,14 @@ static int dso__load_vmlinux(struct dso *self, struct map *map,
return -1;
dso__set_loaded(self, map->type);
- err = dso__load_sym(self, map, session, vmlinux, fd, filter, 1, 0);
+ err = dso__load_sym(self, map, vmlinux, fd, filter, 0);
close(fd);
return err;
}
int dso__load_vmlinux_path(struct dso *self, struct map *map,
- struct perf_session *session, symbol_filter_t filter)
+ symbol_filter_t filter)
{
int i, err = 0;
@@ -1587,8 +1584,7 @@ int dso__load_vmlinux_path(struct dso *self, struct map *map,
vmlinux_path__nr_entries);
for (i = 0; i < vmlinux_path__nr_entries; ++i) {
- err = dso__load_vmlinux(self, map, session, vmlinux_path[i],
- filter);
+ err = dso__load_vmlinux(self, map, vmlinux_path[i], filter);
if (err > 0) {
pr_debug("Using %s for symbols\n", vmlinux_path[i]);
dso__set_long_name(self, strdup(vmlinux_path[i]));
@@ -1600,7 +1596,7 @@ int dso__load_vmlinux_path(struct dso *self, struct map *map,
}
static int dso__load_kernel_sym(struct dso *self, struct map *map,
- struct perf_session *session, symbol_filter_t filter)
+ symbol_filter_t filter)
{
int err;
const char *kallsyms_filename = NULL;
@@ -1621,13 +1617,13 @@ static int dso__load_kernel_sym(struct dso *self, struct map *map,
* match.
*/
if (symbol_conf.vmlinux_name != NULL) {
- err = dso__load_vmlinux(self, map, session,
+ err = dso__load_vmlinux(self, map,
symbol_conf.vmlinux_name, filter);
goto out_try_fixup;
}
if (vmlinux_path != NULL) {
- err = dso__load_vmlinux_path(self, map, session, filter);
+ err = dso__load_vmlinux_path(self, map, filter);
if (err > 0)
goto out_fixup;
}
@@ -1675,7 +1671,7 @@ static int dso__load_kernel_sym(struct dso *self, struct map *map,
}
do_kallsyms:
- err = dso__load_kallsyms(self, kallsyms_filename, map, session, filter);
+ err = dso__load_kallsyms(self, kallsyms_filename, map, filter);
free(kallsyms_allocated_filename);
out_try_fixup:
@@ -1812,30 +1808,23 @@ int __map_groups__create_kernel_maps(struct map_groups *self,
enum map_type type;
for (type = 0; type < MAP__NR_TYPES; ++type) {
+ struct kmap *kmap;
+
vmlinux_maps[type] = map__new2(0, kernel, type);
if (vmlinux_maps[type] == NULL)
return -1;
vmlinux_maps[type]->map_ip =
vmlinux_maps[type]->unmap_ip = identity__map_ip;
+
+ kmap = map__kmap(vmlinux_maps[type]);
+ kmap->kmaps = self;
map_groups__insert(self, vmlinux_maps[type]);
}
return 0;
}
-static int map_groups__create_kernel_maps(struct map_groups *self,
- struct map *vmlinux_maps[MAP__NR_TYPES],
- const char *vmlinux)
-{
- struct dso *kernel = dsos__create_kernel(vmlinux);
-
- if (kernel == NULL)
- return -1;
-
- return __map_groups__create_kernel_maps(self, vmlinux_maps, kernel);
-}
-
static void vmlinux_path__exit(void)
{
while (--vmlinux_path__nr_entries >= 0) {
@@ -1941,19 +1930,22 @@ out_free_comm_list:
return -1;
}
-int perf_session__create_kernel_maps(struct perf_session *self)
+int map_groups__create_kernel_maps(struct map_groups *self,
+ struct map *vmlinux_maps[MAP__NR_TYPES])
{
- if (map_groups__create_kernel_maps(&self->kmaps, self->vmlinux_maps,
- symbol_conf.vmlinux_name) < 0)
+ struct dso *kernel = dsos__create_kernel(symbol_conf.vmlinux_name);
+
+ if (kernel == NULL)
+ return -1;
+
+ if (__map_groups__create_kernel_maps(self, vmlinux_maps, kernel) < 0)
return -1;
- if (symbol_conf.use_modules &&
- perf_session__create_module_maps(self) < 0)
- pr_debug("Failed to load list of modules for session %s, "
- "continuing...\n", self->filename);
+ if (symbol_conf.use_modules && map_groups__create_modules(self) < 0)
+ return -1;
/*
* Now that we have all the maps created, just set the ->end of them:
*/
- map_groups__fixup_end(&self->kmaps);
+ map_groups__fixup_end(self);
return 0;
}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1243027..e6a59e5 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -80,6 +80,12 @@ static inline void *symbol__priv(struct symbol *self)
return ((void *)self) - symbol_conf.priv_size;
}
+struct ref_reloc_sym {
+ const char *name;
+ u64 addr;
+ u64 unrelocated_addr;
+};
+
struct addr_location {
struct thread *thread;
struct map *map;
@@ -126,12 +132,11 @@ static inline struct dso *dsos__findnew(const char *name)
return __dsos__findnew(&dsos__user, name);
}
-struct perf_session;
-
-int dso__load(struct dso *self, struct map *map, struct perf_session *session,
- symbol_filter_t filter);
+int dso__load(struct dso *self, struct map *map, symbol_filter_t filter);
int dso__load_vmlinux_path(struct dso *self, struct map *map,
- struct perf_session *session, symbol_filter_t filter);
+ symbol_filter_t filter);
+int dso__load_kallsyms(struct dso *self, const char *filename, struct map *map,
+ symbol_filter_t filter);
void dsos__fprintf(FILE *fp);
size_t dsos__fprintf_buildid(FILE *fp, bool with_hits);
@@ -156,9 +161,5 @@ int kallsyms__parse(const char *filename, void *arg,
int symbol__init(void);
bool symbol_type__is_a(char symbol_type, enum map_type map_type);
-int perf_session__create_kernel_maps(struct perf_session *self);
-
-struct map *perf_session__new_module_map(struct perf_session *self, u64 start,
- const char *filename);
extern struct dso *vdso;
#endif /* __PERF_SYMBOL */
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 4a08dcf..634b7f7 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -282,14 +282,13 @@ size_t perf_session__fprintf(struct perf_session *self, FILE *fp)
}
struct symbol *map_groups__find_symbol(struct map_groups *self,
- struct perf_session *session,
enum map_type type, u64 addr,
symbol_filter_t filter)
{
struct map *map = map_groups__find(self, type, addr);
if (map != NULL)
- return map__find_symbol(map, session, map->map_ip(map, addr), filter);
+ return map__find_symbol(map, map->map_ip(map, addr), filter);
return NULL;
}
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index e35653c..56f317b 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -59,15 +59,14 @@ void thread__find_addr_location(struct thread *self,
struct addr_location *al,
symbol_filter_t filter);
struct symbol *map_groups__find_symbol(struct map_groups *self,
- struct perf_session *session,
enum map_type type, u64 addr,
symbol_filter_t filter);
-static inline struct symbol *
-map_groups__find_function(struct map_groups *self, struct perf_session *session,
- u64 addr, symbol_filter_t filter)
+static inline struct symbol *map_groups__find_function(struct map_groups *self,
+ u64 addr,
+ symbol_filter_t filter)
{
- return map_groups__find_symbol(self, session, MAP__FUNCTION, addr, filter);
+ return map_groups__find_symbol(self, MAP__FUNCTION, addr, filter);
}
struct map *map_groups__find_by_name(struct map_groups *self,
@@ -76,4 +75,9 @@ struct map *map_groups__find_by_name(struct map_groups *self,
int __map_groups__create_kernel_maps(struct map_groups *self,
struct map *vmlinux_maps[MAP__NR_TYPES],
struct dso *kernel);
+int map_groups__create_kernel_maps(struct map_groups *self,
+ struct map *vmlinux_maps[MAP__NR_TYPES]);
+
+struct map *map_groups__new_module(struct map_groups *self, u64 start,
+ const char *filename);
#endif /* __PERF_THREAD_H */
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/9] perf symbols: Fixup vsyscall maps
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 3/9] perf symbols: Ditch vdso global variable Arnaldo Carvalho de Melo
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Pekka Enberg,
Peter Zijlstra, Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
While debugging a problem reported by Pekka Enberg by printing the IP
and all the maps for a thread when we don't find a map for an IP I
noticed that dso__load_sym needs to fixup these extra maps it creates to
hold symbols in different ELF sections than the main kernel one.
Now we're back showing things like:
[root@doppio linux-2.6-tip]# perf report | grep vsyscall
0.02% mutt [kernel.kallsyms].vsyscall_fn [.] vread_hpet
0.01% named [kernel.kallsyms].vsyscall_fn [.] vread_hpet
0.01% NetworkManager [kernel.kallsyms].vsyscall_fn [.] vread_hpet
0.01% gconfd-2 [kernel.kallsyms].vsyscall_0 [.] vgettimeofday
0.01% hald-addon-rfki [kernel.kallsyms].vsyscall_fn [.] vread_hpet
0.00% dbus-daemon [kernel.kallsyms].vsyscall_fn [.] vread_hpet
[root@doppio linux-2.6-tip]#
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/symbol.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 6138742..051d71b 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1011,7 +1011,7 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
if (curr_dso == NULL)
goto out_elf_end;
curr_map = map__new2(start, curr_dso,
- MAP__FUNCTION);
+ map->type);
if (curr_map == NULL) {
dso__delete(curr_dso);
goto out_elf_end;
@@ -1021,6 +1021,7 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
curr_dso->origin = DSO__ORIG_KERNEL;
map_groups__insert(kmap->kmaps, curr_map);
dsos__add(&dsos__kernel, curr_dso);
+ dso__set_loaded(curr_dso, map->type);
} else
curr_dso = curr_map->dso;
@@ -1058,8 +1059,16 @@ new_symbol:
/*
* For misannotated, zeroed, ASM function sizes.
*/
- if (nr > 0)
+ if (nr > 0) {
symbols__fixup_end(&self->symbols[map->type]);
+ if (kmap) {
+ /*
+ * We need to fixup this here too because we create new
+ * maps here, for things like vsyscall sections.
+ */
+ __map_groups__fixup_end(kmap->kmaps, map->type);
+ }
+ }
err = nr;
out_elf_end:
elf_end(elf);
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/9] perf symbols: Ditch vdso global variable
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 2/9] perf symbols: Fixup vsyscall maps Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 4/9] perf probe: Don't use a perf_session instance just to resolve symbols Arnaldo Carvalho de Melo
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
We can check using strcmp, most DSOs don't start with '[' so the test is
cheap enough and we had to test it there anyway since when reading
perf.data files we weren't calling the routine that created this global
variable and thus weren't setting it as "loaded", which was causing a
bogus:
Failed to open [vdso], continuing without symbols
Message as the first line of 'perf report'.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/map.c | 7 ++++++-
tools/perf/util/symbol.c | 26 ++++----------------------
tools/perf/util/symbol.h | 6 +++++-
3 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 36ff0bf..f6626cc 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -68,8 +68,13 @@ struct map *map__new(struct mmap_event *event, enum map_type type,
map__init(self, type, event->start, event->start + event->len,
event->pgoff, dso);
- if (self->dso == vdso || anon)
+ if (anon) {
+set_identity:
self->map_ip = self->unmap_ip = identity__map_ip;
+ } else if (strcmp(filename, "[vdso]") == 0) {
+ dso__set_loaded(dso, self->type);
+ goto set_identity;
+ }
}
return self;
out_delete:
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 051d71b..e752837 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -53,11 +53,6 @@ bool dso__sorted_by_name(const struct dso *self, enum map_type type)
return self->sorted_by_name & (1 << type);
}
-static void dso__set_loaded(struct dso *self, enum map_type type)
-{
- self->loaded |= (1 << type);
-}
-
static void dso__set_sorted_by_name(struct dso *self, enum map_type type)
{
self->sorted_by_name |= (1 << type);
@@ -1697,7 +1692,6 @@ out_fixup:
LIST_HEAD(dsos__user);
LIST_HEAD(dsos__kernel);
-struct dso *vdso;
static void dsos__add(struct list_head *head, struct dso *dso)
{
@@ -1790,24 +1784,12 @@ static struct dso *dsos__create_kernel(const char *vmlinux)
{
struct dso *kernel = dso__new_kernel(vmlinux);
- if (kernel == NULL)
- return NULL;
-
- vdso = dso__new("[vdso]");
- if (vdso == NULL)
- goto out_delete_kernel_dso;
- dso__set_loaded(vdso, MAP__FUNCTION);
-
- dso__read_running_kernel_build_id(kernel);
-
- dsos__add(&dsos__kernel, kernel);
- dsos__add(&dsos__user, vdso);
+ if (kernel != NULL) {
+ dso__read_running_kernel_build_id(kernel);
+ dsos__add(&dsos__kernel, kernel);
+ }
return kernel;
-
-out_delete_kernel_dso:
- dso__delete(kernel);
- return NULL;
}
int __map_groups__create_kernel_maps(struct map_groups *self,
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index e6a59e5..e90568a 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -121,6 +121,11 @@ void dso__delete(struct dso *self);
bool dso__loaded(const struct dso *self, enum map_type type);
bool dso__sorted_by_name(const struct dso *self, enum map_type type);
+static inline void dso__set_loaded(struct dso *self, enum map_type type)
+{
+ self->loaded |= (1 << type);
+}
+
void dso__sort_by_name(struct dso *self, enum map_type type);
extern struct list_head dsos__user, dsos__kernel;
@@ -161,5 +166,4 @@ int kallsyms__parse(const char *filename, void *arg,
int symbol__init(void);
bool symbol_type__is_a(char symbol_type, enum map_type map_type);
-extern struct dso *vdso;
#endif /* __PERF_SYMBOL */
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/9] perf probe: Don't use a perf_session instance just to resolve symbols
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 2/9] perf symbols: Fixup vsyscall maps Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 3/9] perf symbols: Ditch vdso global variable Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 5/9] perf build-id: Move the routine to find DSOs with hits to the lib Arnaldo Carvalho de Melo
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Masami Hiramatsu, Mike Galbraith,
Peter Zijlstra, Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
With the recent modifications done to untie the session and symbol layers,
'perf probe' now can use just the symbols layer.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Acked-by: Masami Hiramatsu <mhiramat@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-probe.c | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 4fa73ec..ad47bd4 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -41,7 +41,6 @@
#include "util/debugfs.h"
#include "util/symbol.h"
#include "util/thread.h"
-#include "util/session.h"
#include "util/parse-options.h"
#include "util/parse-events.h" /* For debugfs_path */
#include "util/probe-finder.h"
@@ -59,8 +58,8 @@ static struct {
int nr_probe;
struct probe_point probes[MAX_PROBES];
struct strlist *dellist;
- struct perf_session *psession;
- struct map *kmap;
+ struct map_groups kmap_groups;
+ struct map *kmaps[MAP__NR_TYPES];
struct line_range line_range;
} session;
@@ -122,7 +121,8 @@ static int opt_del_probe_event(const struct option *opt __used,
static void evaluate_probe_point(struct probe_point *pp)
{
struct symbol *sym;
- sym = map__find_symbol_by_name(session.kmap, pp->function, NULL);
+ sym = map__find_symbol_by_name(session.kmaps[MAP__FUNCTION],
+ pp->function, NULL);
if (!sym)
die("Kernel symbol \'%s\' not found - probe not added.",
pp->function);
@@ -131,12 +131,13 @@ static void evaluate_probe_point(struct probe_point *pp)
#ifndef NO_LIBDWARF
static int open_vmlinux(void)
{
- if (map__load(session.kmap, NULL) < 0) {
+ if (map__load(session.kmaps[MAP__FUNCTION], NULL) < 0) {
pr_debug("Failed to load kernel map.\n");
return -EINVAL;
}
- pr_debug("Try to open %s\n", session.kmap->dso->long_name);
- return open(session.kmap->dso->long_name, O_RDONLY);
+ pr_debug("Try to open %s\n",
+ session.kmaps[MAP__FUNCTION]->dso->long_name);
+ return open(session.kmaps[MAP__FUNCTION]->dso->long_name, O_RDONLY);
}
static int opt_show_lines(const struct option *opt __used,
@@ -212,12 +213,11 @@ static void init_vmlinux(void)
pr_debug("Use vmlinux: %s\n", symbol_conf.vmlinux_name);
if (symbol__init() < 0)
die("Failed to init symbol map.");
- session.psession = perf_session__new(NULL, O_WRONLY, false);
- if (session.psession == NULL)
- die("Failed to init perf_session.");
- session.kmap = session.psession->vmlinux_maps[MAP__FUNCTION];
- if (!session.kmap)
- die("Could not find kernel map.\n");
+
+ map_groups__init(&session.kmap_groups);
+ if (map_groups__create_kernel_maps(&session.kmap_groups,
+ session.kmaps) < 0)
+ die("Failed to create kernel maps.");
}
int cmd_probe(int argc, const char **argv, const char *prefix __used)
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/9] perf build-id: Move the routine to find DSOs with hits to the lib
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2010-02-03 18:52 ` [PATCH 4/9] perf probe: Don't use a perf_session instance just to resolve symbols Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 6/9] perf record: Stop intercepting events, use postprocessing to get build-ids Arnaldo Carvalho de Melo
` (4 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Because 'perf record' will have to find the build-ids in after we stop
recording, so as to reduce even more the impact in the workload while we do the
measurement.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Makefile | 2 +
tools/perf/builtin-buildid-list.c | 31 +---------------------------
tools/perf/util/build-id.c | 39 +++++++++++++++++++++++++++++++++++++
tools/perf/util/build-id.h | 8 +++++++
4 files changed, 51 insertions(+), 29 deletions(-)
create mode 100644 tools/perf/util/build-id.c
create mode 100644 tools/perf/util/build-id.h
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 4296930..3a5fb36 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -357,6 +357,7 @@ LIB_H += util/include/asm/uaccess.h
LIB_H += perf.h
LIB_H += util/cache.h
LIB_H += util/callchain.h
+LIB_H += util/build-id.h
LIB_H += util/debug.h
LIB_H += util/debugfs.h
LIB_H += util/event.h
@@ -390,6 +391,7 @@ LIB_H += util/probe-event.h
LIB_OBJS += util/abspath.o
LIB_OBJS += util/alias.o
+LIB_OBJS += util/build-id.o
LIB_OBJS += util/config.o
LIB_OBJS += util/ctype.o
LIB_OBJS += util/debugfs.o
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 431f204..d0675c0 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -8,6 +8,7 @@
*/
#include "builtin.h"
#include "perf.h"
+#include "util/build-id.h"
#include "util/cache.h"
#include "util/debug.h"
#include "util/parse-options.h"
@@ -33,34 +34,6 @@ static const struct option options[] = {
OPT_END()
};
-static int build_id_list__process_event(event_t *event,
- struct perf_session *session)
-{
- struct addr_location al;
- u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
- struct thread *thread = perf_session__findnew(session, event->ip.pid);
-
- if (thread == NULL) {
- pr_err("problem processing %d event, skipping it.\n",
- event->header.type);
- return -1;
- }
-
- thread__find_addr_map(thread, session, cpumode, MAP__FUNCTION,
- event->ip.ip, &al);
-
- if (al.map != NULL)
- al.map->dso->hit = 1;
-
- return 0;
-}
-
-static struct perf_event_ops build_id_list__event_ops = {
- .sample = build_id_list__process_event,
- .mmap = event__process_mmap,
- .fork = event__process_task,
-};
-
static int __cmd_buildid_list(void)
{
int err = -1;
@@ -71,7 +44,7 @@ static int __cmd_buildid_list(void)
return -1;
if (with_hits)
- perf_session__process_events(session, &build_id_list__event_ops);
+ perf_session__process_events(session, &build_id__mark_dso_hit_ops);
dsos__fprintf_buildid(stdout, with_hits);
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
new file mode 100644
index 0000000..04904b3
--- /dev/null
+++ b/tools/perf/util/build-id.c
@@ -0,0 +1,39 @@
+/*
+ * build-id.c
+ *
+ * build-id support
+ *
+ * Copyright (C) 2009, 2010 Red Hat Inc.
+ * Copyright (C) 2009, 2010 Arnaldo Carvalho de Melo <acme@redhat.com>
+ */
+#include "build-id.h"
+#include "event.h"
+#include "symbol.h"
+#include <linux/kernel.h>
+
+static int build_id__mark_dso_hit(event_t *event, struct perf_session *session)
+{
+ struct addr_location al;
+ u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
+ struct thread *thread = perf_session__findnew(session, event->ip.pid);
+
+ if (thread == NULL) {
+ pr_err("problem processing %d event, skipping it.\n",
+ event->header.type);
+ return -1;
+ }
+
+ thread__find_addr_map(thread, session, cpumode, MAP__FUNCTION,
+ event->ip.ip, &al);
+
+ if (al.map != NULL)
+ al.map->dso->hit = 1;
+
+ return 0;
+}
+
+struct perf_event_ops build_id__mark_dso_hit_ops = {
+ .sample = build_id__mark_dso_hit,
+ .mmap = event__process_mmap,
+ .fork = event__process_task,
+};
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
new file mode 100644
index 0000000..1d981d6
--- /dev/null
+++ b/tools/perf/util/build-id.h
@@ -0,0 +1,8 @@
+#ifndef PERF_BUILD_ID_H_
+#define PERF_BUILD_ID_H_ 1
+
+#include "session.h"
+
+extern struct perf_event_ops build_id__mark_dso_hit_ops;
+
+#endif
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/9] perf record: Stop intercepting events, use postprocessing to get build-ids
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2010-02-03 18:52 ` [PATCH 5/9] perf build-id: Move the routine to find DSOs with hits to the lib Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 7/9] perf tools: Adjust some verbosity levels Arnaldo Carvalho de Melo
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Xiao Guangrong,
Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
We want to stream events as fast as possible to perf.data, and also in the
future we want to have splice working, when no interception will be possible.
Using build_id__mark_dso_hit_ops to create the list of DSOs that back MMAPs we
also optimize disk usage in the build-id cache by only caching DSOs that had
hits.
Suggested-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-record.c | 37 ++++++++++++-------------
| 7 +++-
tools/perf/util/session.c | 64 +++++++++++++++++++++++++-----------------
tools/perf/util/session.h | 3 ++
tools/perf/util/symbol.c | 13 +++++---
tools/perf/util/symbol.h | 2 +-
6 files changed, 73 insertions(+), 53 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 949167e..706f001 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -12,6 +12,7 @@
#include "perf.h"
+#include "util/build-id.h"
#include "util/util.h"
#include "util/parse-options.h"
#include "util/parse-events.h"
@@ -65,6 +66,7 @@ static int nr_poll = 0;
static int nr_cpu = 0;
static int file_new = 1;
+static off_t post_processing_offset;
static struct perf_session *session;
@@ -114,26 +116,10 @@ static void write_output(void *buf, size_t size)
}
}
-static void write_event(event_t *buf, size_t size)
-{
- /*
- * Add it to the list of DSOs, so that when we finish this
- * record session we can pick the available build-ids.
- */
- if (buf->header.type == PERF_RECORD_MMAP) {
- struct list_head *head = &dsos__user;
- if (buf->mmap.header.misc == 1)
- head = &dsos__kernel;
- __dsos__findnew(head, buf->mmap.filename);
- }
-
- write_output(buf, size);
-}
-
static int process_synthesized_event(event_t *event,
struct perf_session *self __used)
{
- write_event(event, event->header.size);
+ write_output(event, event->header.size);
return 0;
}
@@ -185,14 +171,14 @@ static void mmap_read(struct mmap_data *md)
size = md->mask + 1 - (old & md->mask);
old += size;
- write_event(buf, size);
+ write_output(buf, size);
}
buf = &data[old & md->mask];
size = head - old;
old += size;
- write_event(buf, size);
+ write_output(buf, size);
md->prev = old;
mmap_write_tail(md, old);
@@ -402,10 +388,21 @@ static void open_counters(int cpu, pid_t pid)
nr_cpu++;
}
+static int process_buildids(void)
+{
+ u64 size = lseek(output, 0, SEEK_CUR);
+
+ session->fd = output;
+ return __perf_session__process_events(session, post_processing_offset,
+ size - post_processing_offset,
+ size, &build_id__mark_dso_hit_ops);
+}
+
static void atexit_header(void)
{
session->header.data_size += bytes_written;
+ process_buildids();
perf_header__write(&session->header, output, true);
}
@@ -558,6 +555,8 @@ static int __cmd_record(int argc, const char **argv)
return err;
}
+ post_processing_offset = lseek(output, 0, SEEK_CUR);
+
err = event__synthesize_kernel_mmap(process_synthesized_event,
session, "_text");
if (err < 0) {
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ed3efd7..d5facd5 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -205,8 +205,11 @@ static int __dsos__write_buildid_table(struct list_head *head, u16 misc, int fd)
dsos__for_each_with_build_id(pos, head) {
int err;
struct build_id_event b;
- size_t len = pos->long_name_len + 1;
+ size_t len;
+ if (!pos->hit)
+ continue;
+ len = pos->long_name_len + 1;
len = ALIGN(len, NAME_ALIGN);
memset(&b, 0, sizeof(b));
memcpy(&b.build_id, pos->build_id, sizeof(pos->build_id));
@@ -371,7 +374,7 @@ static int perf_header__adds_write(struct perf_header *self, int fd)
u64 sec_start;
int idx = 0, err;
- if (dsos__read_build_ids())
+ if (dsos__read_build_ids(true))
perf_header__set_feat(self, HEADER_BUILD_ID);
nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index aa8a031..74cbc64 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -385,8 +385,9 @@ static struct thread *perf_session__register_idle_thread(struct perf_session *se
return thread;
}
-int perf_session__process_events(struct perf_session *self,
- struct perf_event_ops *ops)
+int __perf_session__process_events(struct perf_session *self,
+ u64 data_offset, u64 data_size,
+ u64 file_size, struct perf_event_ops *ops)
{
int err, mmap_prot, mmap_flags;
u64 head, shift;
@@ -396,32 +397,11 @@ int perf_session__process_events(struct perf_session *self,
uint32_t size;
char *buf;
- if (perf_session__register_idle_thread(self) == NULL)
- return -ENOMEM;
-
perf_event_ops__fill_defaults(ops);
page_size = sysconf(_SC_PAGESIZE);
- head = self->header.data_offset;
-
- if (!symbol_conf.full_paths) {
- char bf[PATH_MAX];
-
- if (getcwd(bf, sizeof(bf)) == NULL) {
- err = -errno;
-out_getcwd_err:
- pr_err("failed to get the current directory\n");
- goto out_err;
- }
- self->cwd = strdup(bf);
- if (self->cwd == NULL) {
- err = -ENOMEM;
- goto out_getcwd_err;
- }
- self->cwdlen = strlen(self->cwd);
- }
-
+ head = data_offset;
shift = page_size * (head / page_size);
offset += shift;
head -= shift;
@@ -486,10 +466,10 @@ more:
head += size;
- if (offset + head >= self->header.data_offset + self->header.data_size)
+ if (offset + head >= data_offset + data_size)
goto done;
- if (offset + head < self->size)
+ if (offset + head < file_size)
goto more;
done:
err = 0;
@@ -497,6 +477,38 @@ out_err:
return err;
}
+int perf_session__process_events(struct perf_session *self,
+ struct perf_event_ops *ops)
+{
+ int err;
+
+ if (perf_session__register_idle_thread(self) == NULL)
+ return -ENOMEM;
+
+ if (!symbol_conf.full_paths) {
+ char bf[PATH_MAX];
+
+ if (getcwd(bf, sizeof(bf)) == NULL) {
+ err = -errno;
+out_getcwd_err:
+ pr_err("failed to get the current directory\n");
+ goto out_err;
+ }
+ self->cwd = strdup(bf);
+ if (self->cwd == NULL) {
+ err = -ENOMEM;
+ goto out_getcwd_err;
+ }
+ self->cwdlen = strlen(self->cwd);
+ }
+
+ err = __perf_session__process_events(self, self->header.data_offset,
+ self->header.data_size,
+ self->size, ops);
+out_err:
+ return err;
+}
+
bool perf_session__has_traces(struct perf_session *self, const char *msg)
{
if (!(self->sample_type & PERF_SAMPLE_RAW)) {
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 752d75a..31950fc 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -50,6 +50,9 @@ void perf_session__delete(struct perf_session *self);
void perf_event_header__bswap(struct perf_event_header *self);
+int __perf_session__process_events(struct perf_session *self,
+ u64 data_offset, u64 data_size, u64 size,
+ struct perf_event_ops *ops);
int perf_session__process_events(struct perf_session *self,
struct perf_event_ops *event_ops);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e752837..bfb0554 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1076,25 +1076,28 @@ static bool dso__build_id_equal(const struct dso *self, u8 *build_id)
return memcmp(self->build_id, build_id, sizeof(self->build_id)) == 0;
}
-static bool __dsos__read_build_ids(struct list_head *head)
+static bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
{
bool have_build_id = false;
struct dso *pos;
- list_for_each_entry(pos, head, node)
+ list_for_each_entry(pos, head, node) {
+ if (with_hits && !pos->hit)
+ continue;
if (filename__read_build_id(pos->long_name, pos->build_id,
sizeof(pos->build_id)) > 0) {
have_build_id = true;
pos->has_build_id = true;
}
+ }
return have_build_id;
}
-bool dsos__read_build_ids(void)
+bool dsos__read_build_ids(bool with_hits)
{
- bool kbuildids = __dsos__read_build_ids(&dsos__kernel),
- ubuildids = __dsos__read_build_ids(&dsos__user);
+ bool kbuildids = __dsos__read_build_ids(&dsos__kernel, with_hits),
+ ubuildids = __dsos__read_build_ids(&dsos__user, with_hits);
return kbuildids || ubuildids;
}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index e90568a..1b4192e 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -157,7 +157,7 @@ struct symbol *dso__find_symbol_by_name(struct dso *self, enum map_type type,
int filename__read_build_id(const char *filename, void *bf, size_t size);
int sysfs__read_build_id(const char *filename, void *bf, size_t size);
-bool dsos__read_build_ids(void);
+bool dsos__read_build_ids(bool with_hits);
int build_id__sprintf(const u8 *self, int len, char *bf);
int kallsyms__parse(const char *filename, void *arg,
int (*process_symbol)(void *arg, const char *name,
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 7/9] perf tools: Adjust some verbosity levels
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2010-02-03 18:52 ` [PATCH 6/9] perf record: Stop intercepting events, use postprocessing to get build-ids Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Arnaldo Carvalho de Melo
` (2 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
Paul Mackerras
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Not to pollute too much 'perf annotate' debugging sessions.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-annotate.c | 33 +++++++++++--------------------
tools/perf/util/include/linux/kernel.h | 1 +
tools/perf/util/symbol.c | 9 ++++---
3 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 73c202e..4fc3899 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -97,9 +97,7 @@ static void hist_hit(struct hist_entry *he, u64 ip)
sym_size = sym->end - sym->start;
offset = ip - sym->start;
- if (verbose)
- fprintf(stderr, "%s: ip=%Lx\n", __func__,
- he->map->unmap_ip(he->map, ip));
+ pr_debug3("%s: ip=%#Lx\n", __func__, he->map->unmap_ip(he->map, ip));
if (offset >= sym_size)
return;
@@ -108,12 +106,8 @@ static void hist_hit(struct hist_entry *he, u64 ip)
h->sum++;
h->ip[offset]++;
- if (verbose >= 3)
- printf("%p %s: count++ [ip: %p, %08Lx] => %Ld\n",
- (void *)(unsigned long)he->sym->start,
- he->sym->name,
- (void *)(unsigned long)ip, ip - he->sym->start,
- h->ip[offset]);
+ pr_debug3("%#Lx %s: count++ [ip: %#Lx, %#Lx] => %Ld\n", he->sym->start,
+ he->sym->name, ip, ip - he->sym->start, h->ip[offset]);
}
static int perf_session__add_hist_entry(struct perf_session *self,
@@ -136,14 +130,14 @@ static int process_sample_event(event_t *event, struct perf_session *session)
event->ip.pid, event->ip.ip);
if (event__preprocess_sample(event, session, &al, symbol_filter) < 0) {
- fprintf(stderr, "problem processing %d event, skipping it.\n",
- event->header.type);
+ pr_warning("problem processing %d event, skipping it.\n",
+ event->header.type);
return -1;
}
if (!al.filtered && perf_session__add_hist_entry(session, &al, 1)) {
- fprintf(stderr, "problem incrementing symbol count, "
- "skipping event\n");
+ pr_warning("problem incrementing symbol count, "
+ "skipping event\n");
return -1;
}
@@ -378,11 +372,9 @@ static void annotate_sym(struct hist_entry *he)
if (!filename)
return;
- if (verbose)
- fprintf(stderr, "%s: filename=%s, sym=%s, start=%Lx, end=%Lx\n",
- __func__, filename, sym->name,
- map->unmap_ip(map, sym->start),
- map->unmap_ip(map, sym->end));
+ pr_debug("%s: filename=%s, sym=%s, start=%#Lx, end=%#Lx\n", __func__,
+ filename, sym->name, map->unmap_ip(map, sym->start),
+ map->unmap_ip(map, sym->end));
if (full_paths)
d_filename = filename;
@@ -542,9 +534,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __used)
setup_pager();
if (field_sep && *field_sep == '.') {
- fputs("'.' is the only non valid --field-separator argument\n",
- stderr);
- exit(129);
+ pr_err("'.' is the only non valid --field-separator argument\n");
+ return -1;
}
return __cmd_annotate();
diff --git a/tools/perf/util/include/linux/kernel.h b/tools/perf/util/include/linux/kernel.h
index 21c0274..f261165 100644
--- a/tools/perf/util/include/linux/kernel.h
+++ b/tools/perf/util/include/linux/kernel.h
@@ -101,5 +101,6 @@ simple_strtoul(const char *nptr, char **endptr, int base)
eprintf(n, pr_fmt(fmt), ##__VA_ARGS__)
#define pr_debug2(fmt, ...) pr_debugN(2, pr_fmt(fmt), ##__VA_ARGS__)
#define pr_debug3(fmt, ...) pr_debugN(3, pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_debug4(fmt, ...) pr_debugN(4, pr_fmt(fmt), ##__VA_ARGS__)
#endif
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index bfb0554..a60ba2b 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -137,7 +137,7 @@ static struct symbol *symbol__new(u64 start, u64 len, const char *name)
self->start = start;
self->end = len ? start + len - 1 : start;
- pr_debug3("%s: %s %#Lx-%#Lx\n", __func__, name, start, self->end);
+ pr_debug4("%s: %s %#Lx-%#Lx\n", __func__, name, start, self->end);
memcpy(self->name, name, namelen);
@@ -1024,9 +1024,10 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
}
if (curr_dso->adjust_symbols) {
- pr_debug2("adjusting symbol: st_value: %Lx sh_addr: "
- "%Lx sh_offset: %Lx\n", (u64)sym.st_value,
- (u64)shdr.sh_addr, (u64)shdr.sh_offset);
+ pr_debug4("%s: adjusting symbol: st_value: %#Lx "
+ "sh_addr: %#Lx sh_offset: %#Lx\n", __func__,
+ (u64)sym.st_value, (u64)shdr.sh_addr,
+ (u64)shdr.sh_offset);
sym.st_value -= shdr.sh_addr - shdr.sh_offset;
}
/*
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 8/9] perf annotate: fix it for non-prelinked *.so
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
` (5 preceding siblings ...)
2010-02-03 18:52 ` [PATCH 7/9] perf tools: Adjust some verbosity levels Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-04 6:31 ` Mike Galbraith
2010-02-04 9:54 ` [tip:perf/core] perf annotate: Fix " tip-bot for Kirill Smelkov
2010-02-03 18:52 ` [PATCH 9/9] perf top: teach it to autolocate vmlinux Arnaldo Carvalho de Melo
2010-02-04 9:26 ` [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Ingo Molnar
8 siblings, 2 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Kirill Smelkov, Mike Galbraith, Arnaldo Carvalho de Melo
From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
The problem was we were incorrectly calculating objdump addresses for
sym->start and sym->end, look:
For simple ET_DYN type DSO (*.so) with one function, objdump -dS output
is something like this:
000004ac <my_strlen>:
int my_strlen(const char *s)
4ac: 55 push %ebp
4ad: 89 e5 mov %esp,%ebp
4af: 83 ec 10 sub $0x10,%esp
{
i.e. we have relative-to-dso-mapping IPs (=RIP) there.
For ET_EXEC type and probably for prelinked libs as well (sorry can't
test - I don't use prelink) objdump outputs absolute IPs, e.g.
08048604 <zz_strlen>:
extern "C"
int zz_strlen(const char *s)
8048604: 55 push %ebp
8048605: 89 e5 mov %esp,%ebp
8048607: 83 ec 10 sub $0x10,%esp
{
So, if sym->start is always relative to dso mapping(*), we'll have to
unmap it for ET_EXEC like cases, and leave as is for ET_DYN cases.
(*) and it is - we've explicitely made it relative. Look for
adjust_symbols handling in dso__load_sym()
Previously we were always unmapping sym->start and for ET_DYN dsos
resulting addresses were wrong, and so objdump output was empty.
The end result was that perf annotate output for symbols from
non-prelinked *.so had always 0.00% percents only, which is wrong.
To fix it, let's introduce a helper for converting rip to objdump
address, and also let's document what map_ip() and unmap_ip() do -- I
had to study sources for several hours to understand it.
Cc: Mike Galbraith <efault@gmx.de>
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-annotate.c | 5 +++--
tools/perf/util/map.c | 12 ++++++++++++
tools/perf/util/map.h | 9 +++++++++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4fc3899..28ea4e0 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -189,7 +189,7 @@ static int parse_line(FILE *file, struct hist_entry *he, u64 len)
line_ip = -1;
}
- start = he->map->unmap_ip(he->map, sym->start);
+ start = map__rip_2objdump(he->map, sym->start);
if (line_ip != -1) {
const char *path = NULL;
@@ -397,7 +397,8 @@ static void annotate_sym(struct hist_entry *he)
dso, dso->long_name, sym, sym->name);
sprintf(command, "objdump --start-address=0x%016Lx --stop-address=0x%016Lx -dS %s|grep -v %s",
- map->unmap_ip(map, sym->start), map->unmap_ip(map, sym->end),
+ map__rip_2objdump(map, sym->start),
+ map__rip_2objdump(map, sym->end),
filename, filename);
if (verbose >= 3)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index f6626cc..af5805f 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -210,3 +210,15 @@ size_t map__fprintf(struct map *self, FILE *fp)
return fprintf(fp, " %Lx-%Lx %Lx %s\n",
self->start, self->end, self->pgoff, self->dso->name);
}
+
+/*
+ * objdump wants/reports absolute IPs for ET_EXEC, and RIPs for ET_DYN.
+ * map->dso->adjust_symbols==1 for ET_EXEC-like cases.
+ */
+u64 map__rip_2objdump(struct map *map, u64 rip)
+{
+ u64 addr = map->dso->adjust_symbols ?
+ map->unmap_ip(map, rip) : /* RIP -> IP */
+ rip;
+ return addr;
+}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index de04839..9cee9c7 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -26,8 +26,12 @@ struct map {
u64 end;
enum map_type type;
u64 pgoff;
+
+ /* ip -> dso rip */
u64 (*map_ip)(struct map *, u64);
+ /* dso rip -> ip */
u64 (*unmap_ip)(struct map *, u64);
+
struct dso *dso;
};
@@ -56,6 +60,11 @@ static inline u64 identity__map_ip(struct map *map __used, u64 ip)
return ip;
}
+
+/* rip -> addr suitable for passing to `objdump --start-address=` */
+u64 map__rip_2objdump(struct map *map, u64 rip);
+
+
struct symbol;
struct mmap_event;
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 9/9] perf top: teach it to autolocate vmlinux
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
` (6 preceding siblings ...)
2010-02-03 18:52 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Arnaldo Carvalho de Melo
@ 2010-02-03 18:52 ` Arnaldo Carvalho de Melo
2010-02-04 9:54 ` [tip:perf/core] perf top: Teach " tip-bot for Kirill Smelkov
2010-02-04 9:26 ` [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Ingo Molnar
8 siblings, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-03 18:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Kirill Smelkov, Mike Galbraith, Arnaldo Carvalho de Melo
From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
By relying on logic in dso__load_kernel_sym(), we can automatically load
vmlinux.
The only thing which needs to be adjusted, is how --sym-annotate option
is handled - now we can't rely on vmlinux been loaded until full
successful pass of dso__load_vmlinux(), but that's not the case if we'll
do sym_filter_entry setup in symbol_filter().
So move this step right after event__process_sample() where we know the
whole dso__load_kernel_sym() pass is done.
By the way, though conceptually similar `perf top` still can't annotate
userspace - see next patches with fixes.
Cc: Mike Galbraith <efault@gmx.de>
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-top.txt | 2 +-
tools/perf/builtin-top.c | 39 +++++++++++++++++++-------------
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index 4a7d558..785b9fc 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -74,7 +74,7 @@ OPTIONS
-s <symbol>::
--sym-annotate=<symbol>::
- Annotate this symbol. Requires -k option.
+ Annotate this symbol.
-v::
--verbose::
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 1fc018e..83c09c8 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -94,6 +94,7 @@ struct source_line {
static char *sym_filter = NULL;
struct sym_entry *sym_filter_entry = NULL;
+struct sym_entry *sym_filter_entry_sched = NULL;
static int sym_pcnt_filter = 5;
static int sym_counter = 0;
static int display_weighted = -1;
@@ -695,11 +696,9 @@ static void print_mapped_keys(void)
fprintf(stdout, "\t[f] profile display filter (count). \t(%d)\n", count_filter);
- if (symbol_conf.vmlinux_name) {
- fprintf(stdout, "\t[F] annotate display filter (percent). \t(%d%%)\n", sym_pcnt_filter);
- fprintf(stdout, "\t[s] annotate symbol. \t(%s)\n", name?: "NULL");
- fprintf(stdout, "\t[S] stop annotation.\n");
- }
+ fprintf(stdout, "\t[F] annotate display filter (percent). \t(%d%%)\n", sym_pcnt_filter);
+ fprintf(stdout, "\t[s] annotate symbol. \t(%s)\n", name?: "NULL");
+ fprintf(stdout, "\t[S] stop annotation.\n");
if (nr_counters > 1)
fprintf(stdout, "\t[w] toggle display weighted/count[E]r. \t(%d)\n", display_weighted ? 1 : 0);
@@ -725,14 +724,13 @@ static int key_mapped(int c)
case 'Q':
case 'K':
case 'U':
+ case 'F':
+ case 's':
+ case 'S':
return 1;
case 'E':
case 'w':
return nr_counters > 1 ? 1 : 0;
- case 'F':
- case 's':
- case 'S':
- return symbol_conf.vmlinux_name ? 1 : 0;
default:
break;
}
@@ -910,8 +908,12 @@ static int symbol_filter(struct map *map, struct symbol *sym)
syme = symbol__priv(sym);
syme->map = map;
syme->src = NULL;
- if (!sym_filter_entry && sym_filter && !strcmp(name, sym_filter))
- sym_filter_entry = syme;
+
+ if (!sym_filter_entry && sym_filter && !strcmp(name, sym_filter)) {
+ /* schedule initial sym_filter_entry setup */
+ sym_filter_entry_sched = syme;
+ sym_filter = NULL;
+ }
for (i = 0; skip_symbols[i]; i++) {
if (!strcmp(skip_symbols[i], name)) {
@@ -976,6 +978,13 @@ static void event__process_sample(const event_t *self,
return;
}
+ /* let's see, whether we need to install initial sym_filter_entry */
+ if (sym_filter_entry_sched) {
+ sym_filter_entry = sym_filter_entry_sched;
+ sym_filter_entry_sched = NULL;
+ parse_source(sym_filter_entry);
+ }
+
syme = symbol__priv(al.sym);
if (!syme->skip) {
syme->count[counter]++;
@@ -1270,7 +1279,7 @@ static const struct option options[] = {
OPT_BOOLEAN('i', "inherit", &inherit,
"child tasks inherit counters"),
OPT_STRING('s', "sym-annotate", &sym_filter, "symbol name",
- "symbol to annotate - requires -k option"),
+ "symbol to annotate"),
OPT_BOOLEAN('z', "zero", &zero,
"zero history across updates"),
OPT_INTEGER('F', "freq", &freq,
@@ -1306,16 +1315,14 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
symbol_conf.priv_size = (sizeof(struct sym_entry) +
(nr_counters + 1) * sizeof(unsigned long));
- if (symbol_conf.vmlinux_name == NULL)
- symbol_conf.try_vmlinux_path = true;
+
+ symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
if (symbol__init() < 0)
return -1;
if (delay_secs < 1)
delay_secs = 1;
- parse_source(sym_filter_entry);
-
/*
* User specified count overrides default frequency.
*/
--
1.6.2.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 8/9] perf annotate: fix it for non-prelinked *.so
2010-02-03 18:52 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Arnaldo Carvalho de Melo
@ 2010-02-04 6:31 ` Mike Galbraith
2010-02-04 9:54 ` [tip:perf/core] perf annotate: Fix perf top module symbol annotation tip-bot for Mike Galbraith
2010-02-04 19:34 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Kirill Smelkov
2010-02-04 9:54 ` [tip:perf/core] perf annotate: Fix " tip-bot for Kirill Smelkov
1 sibling, 2 replies; 18+ messages in thread
From: Mike Galbraith @ 2010-02-04 6:31 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, linux-kernel, Kirill Smelkov, Arnaldo Carvalho de Melo
On Wed, 2010-02-03 at 16:52 -0200, Arnaldo Carvalho de Melo wrote:
> const char *path = NULL;
> @@ -397,7 +397,8 @@ static void annotate_sym(struct hist_entry *he)
> dso, dso->long_name, sym, sym->name);
>
> sprintf(command, "objdump --start-address=0x%016Lx --stop-address=0x%016Lx -dS %s|grep -v %s",
> - map->unmap_ip(map, sym->start), map->unmap_ip(map, sym->end),
> + map__rip_2objdump(map, sym->start),
> + map__rip_2objdump(map, sym->end),
> filename, filename);
>
Monkey see monkey do.
perf tools: fix perf top module symbol annotation.
Signed-off-by: Mike Galbraith <efault@gmx.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <new-submission>
---
tools/perf/builtin-top.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6/tools/perf/builtin-top.c
===================================================================
--- linux-2.6.orig/tools/perf/builtin-top.c
+++ linux-2.6/tools/perf/builtin-top.c
@@ -204,8 +204,8 @@ static void parse_source(struct sym_entr
sprintf(command,
"objdump --start-address=0x%016Lx "
"--stop-address=0x%016Lx -dS %s",
- map->unmap_ip(map, sym->start),
- map->unmap_ip(map, sym->end), path);
+ map__rip_2objdump(map, sym->start),
+ map__rip_2objdump(map, sym->end), path);
file = popen(command, "r");
if (!file)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
` (7 preceding siblings ...)
2010-02-03 18:52 ` [PATCH 9/9] perf top: teach it to autolocate vmlinux Arnaldo Carvalho de Melo
@ 2010-02-04 9:26 ` Ingo Molnar
2010-02-04 13:04 ` Arnaldo Carvalho de Melo
8 siblings, 1 reply; 18+ messages in thread
From: Ingo Molnar @ 2010-02-04 9:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Arnaldo Carvalho de Melo, Fr??d??ric Weisbecker,
Mike Galbraith, Peter Zijlstra, Paul Mackerras
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 1784 bytes --]
these changes reintroduce the 'perf top exits' bug:
$ perf top -v
map_groups__set_modules_path_dir: cannot open /lib/modules/2.6.33-rc6-tip-00586-g398dde3-dirty/kernel dir
It's the non-existence of modules that causes a disorderly exit. I sent you
my config with the earlier bugreport - i think if you tried that config you'd
see a similar failure. I've fixed it via the commit below for now.
Ingo
------------>
>From 2161db969313cb94ffd9377a525fb75c3fee9eeb Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 4 Feb 2010 10:22:01 +0100
Subject: [PATCH] perf tools: Fix session init on non-modular kernels
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
perf top and perf record refuses to initialize on non-modular kernels:
refuse to initialize:
$ perf top -v
map_groups__set_modules_path_dir: cannot open /lib/modules/2.6.33-rc6-tip-00586-g398dde3-dirty/
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1265223128-11786-1-git-send-email-acme@infradead.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/util/symbol.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a60ba2b..6882e9f 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1937,7 +1937,7 @@ int map_groups__create_kernel_maps(struct map_groups *self,
return -1;
if (symbol_conf.use_modules && map_groups__create_modules(self) < 0)
- return -1;
+ return 0;
/*
* Now that we have all the maps created, just set the ->end of them:
*/
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf annotate: Fix it for non-prelinked *.so
2010-02-03 18:52 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Arnaldo Carvalho de Melo
2010-02-04 6:31 ` Mike Galbraith
@ 2010-02-04 9:54 ` tip-bot for Kirill Smelkov
1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Kirill Smelkov @ 2010-02-04 9:54 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, acme, hpa, mingo, kirr, efault, tglx, mingo
Commit-ID: 7a2b6209863626cf8362e5ff4653491558f91e67
Gitweb: http://git.kernel.org/tip/7a2b6209863626cf8362e5ff4653491558f91e67
Author: Kirill Smelkov <kirr@landau.phys.spbu.ru>
AuthorDate: Wed, 3 Feb 2010 16:52:07 -0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 4 Feb 2010 09:33:27 +0100
perf annotate: Fix it for non-prelinked *.so
The problem was we were incorrectly calculating objdump
addresses for sym->start and sym->end, look:
For simple ET_DYN type DSO (*.so) with one function, objdump -dS
output is something like this:
000004ac <my_strlen>:
int my_strlen(const char *s)
4ac: 55 push %ebp
4ad: 89 e5 mov %esp,%ebp
4af: 83 ec 10 sub $0x10,%esp
{
i.e. we have relative-to-dso-mapping IPs (=RIP) there.
For ET_EXEC type and probably for prelinked libs as well (sorry
can't test - I don't use prelink) objdump outputs absolute IPs,
e.g.
08048604 <zz_strlen>:
extern "C"
int zz_strlen(const char *s)
8048604: 55 push %ebp
8048605: 89 e5 mov %esp,%ebp
8048607: 83 ec 10 sub $0x10,%esp
{
So, if sym->start is always relative to dso mapping(*), we'll
have to unmap it for ET_EXEC like cases, and leave as is for
ET_DYN cases.
(*) and it is - we've explicitely made it relative. Look for
adjust_symbols handling in dso__load_sym()
Previously we were always unmapping sym->start and for ET_DYN
dsos resulting addresses were wrong, and so objdump output was
empty.
The end result was that perf annotate output for symbols from
non-prelinked *.so had always 0.00% percents only, which is
wrong.
To fix it, let's introduce a helper for converting rip to
objdump address, and also let's document what map_ip() and
unmap_ip() do -- I had to study sources for several hours to
understand it.
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
LKML-Reference: <1265223128-11786-8-git-send-email-acme@infradead.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/builtin-annotate.c | 5 +++--
tools/perf/util/map.c | 12 ++++++++++++
tools/perf/util/map.h | 9 +++++++++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4fc3899..28ea4e0 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -189,7 +189,7 @@ static int parse_line(FILE *file, struct hist_entry *he, u64 len)
line_ip = -1;
}
- start = he->map->unmap_ip(he->map, sym->start);
+ start = map__rip_2objdump(he->map, sym->start);
if (line_ip != -1) {
const char *path = NULL;
@@ -397,7 +397,8 @@ static void annotate_sym(struct hist_entry *he)
dso, dso->long_name, sym, sym->name);
sprintf(command, "objdump --start-address=0x%016Lx --stop-address=0x%016Lx -dS %s|grep -v %s",
- map->unmap_ip(map, sym->start), map->unmap_ip(map, sym->end),
+ map__rip_2objdump(map, sym->start),
+ map__rip_2objdump(map, sym->end),
filename, filename);
if (verbose >= 3)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index f6626cc..af5805f 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -210,3 +210,15 @@ size_t map__fprintf(struct map *self, FILE *fp)
return fprintf(fp, " %Lx-%Lx %Lx %s\n",
self->start, self->end, self->pgoff, self->dso->name);
}
+
+/*
+ * objdump wants/reports absolute IPs for ET_EXEC, and RIPs for ET_DYN.
+ * map->dso->adjust_symbols==1 for ET_EXEC-like cases.
+ */
+u64 map__rip_2objdump(struct map *map, u64 rip)
+{
+ u64 addr = map->dso->adjust_symbols ?
+ map->unmap_ip(map, rip) : /* RIP -> IP */
+ rip;
+ return addr;
+}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index de04839..9cee9c7 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -26,8 +26,12 @@ struct map {
u64 end;
enum map_type type;
u64 pgoff;
+
+ /* ip -> dso rip */
u64 (*map_ip)(struct map *, u64);
+ /* dso rip -> ip */
u64 (*unmap_ip)(struct map *, u64);
+
struct dso *dso;
};
@@ -56,6 +60,11 @@ static inline u64 identity__map_ip(struct map *map __used, u64 ip)
return ip;
}
+
+/* rip -> addr suitable for passing to `objdump --start-address=` */
+u64 map__rip_2objdump(struct map *map, u64 rip);
+
+
struct symbol;
struct mmap_event;
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf top: Teach it to autolocate vmlinux
2010-02-03 18:52 ` [PATCH 9/9] perf top: teach it to autolocate vmlinux Arnaldo Carvalho de Melo
@ 2010-02-04 9:54 ` tip-bot for Kirill Smelkov
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Kirill Smelkov @ 2010-02-04 9:54 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, acme, hpa, mingo, kirr, efault, tglx, mingo
Commit-ID: 6cff0e8dbaa4d5d822a814e5028683d7e71c3291
Gitweb: http://git.kernel.org/tip/6cff0e8dbaa4d5d822a814e5028683d7e71c3291
Author: Kirill Smelkov <kirr@landau.phys.spbu.ru>
AuthorDate: Wed, 3 Feb 2010 16:52:08 -0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 4 Feb 2010 09:33:28 +0100
perf top: Teach it to autolocate vmlinux
By relying on logic in dso__load_kernel_sym(), we can
automatically load vmlinux.
The only thing which needs to be adjusted, is how --sym-annotate
option is handled - now we can't rely on vmlinux been loaded
until full successful pass of dso__load_vmlinux(), but that's
not the case if we'll do sym_filter_entry setup in
symbol_filter().
So move this step right after event__process_sample() where we
know the whole dso__load_kernel_sym() pass is done.
By the way, though conceptually similar `perf top` still can't
annotate userspace - see next patches with fixes.
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
LKML-Reference: <1265223128-11786-9-git-send-email-acme@infradead.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/Documentation/perf-top.txt | 2 +-
tools/perf/builtin-top.c | 39 +++++++++++++++++++-------------
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index 4a7d558..785b9fc 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -74,7 +74,7 @@ OPTIONS
-s <symbol>::
--sym-annotate=<symbol>::
- Annotate this symbol. Requires -k option.
+ Annotate this symbol.
-v::
--verbose::
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 1fc018e..83c09c8 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -94,6 +94,7 @@ struct source_line {
static char *sym_filter = NULL;
struct sym_entry *sym_filter_entry = NULL;
+struct sym_entry *sym_filter_entry_sched = NULL;
static int sym_pcnt_filter = 5;
static int sym_counter = 0;
static int display_weighted = -1;
@@ -695,11 +696,9 @@ static void print_mapped_keys(void)
fprintf(stdout, "\t[f] profile display filter (count). \t(%d)\n", count_filter);
- if (symbol_conf.vmlinux_name) {
- fprintf(stdout, "\t[F] annotate display filter (percent). \t(%d%%)\n", sym_pcnt_filter);
- fprintf(stdout, "\t[s] annotate symbol. \t(%s)\n", name?: "NULL");
- fprintf(stdout, "\t[S] stop annotation.\n");
- }
+ fprintf(stdout, "\t[F] annotate display filter (percent). \t(%d%%)\n", sym_pcnt_filter);
+ fprintf(stdout, "\t[s] annotate symbol. \t(%s)\n", name?: "NULL");
+ fprintf(stdout, "\t[S] stop annotation.\n");
if (nr_counters > 1)
fprintf(stdout, "\t[w] toggle display weighted/count[E]r. \t(%d)\n", display_weighted ? 1 : 0);
@@ -725,14 +724,13 @@ static int key_mapped(int c)
case 'Q':
case 'K':
case 'U':
+ case 'F':
+ case 's':
+ case 'S':
return 1;
case 'E':
case 'w':
return nr_counters > 1 ? 1 : 0;
- case 'F':
- case 's':
- case 'S':
- return symbol_conf.vmlinux_name ? 1 : 0;
default:
break;
}
@@ -910,8 +908,12 @@ static int symbol_filter(struct map *map, struct symbol *sym)
syme = symbol__priv(sym);
syme->map = map;
syme->src = NULL;
- if (!sym_filter_entry && sym_filter && !strcmp(name, sym_filter))
- sym_filter_entry = syme;
+
+ if (!sym_filter_entry && sym_filter && !strcmp(name, sym_filter)) {
+ /* schedule initial sym_filter_entry setup */
+ sym_filter_entry_sched = syme;
+ sym_filter = NULL;
+ }
for (i = 0; skip_symbols[i]; i++) {
if (!strcmp(skip_symbols[i], name)) {
@@ -976,6 +978,13 @@ static void event__process_sample(const event_t *self,
return;
}
+ /* let's see, whether we need to install initial sym_filter_entry */
+ if (sym_filter_entry_sched) {
+ sym_filter_entry = sym_filter_entry_sched;
+ sym_filter_entry_sched = NULL;
+ parse_source(sym_filter_entry);
+ }
+
syme = symbol__priv(al.sym);
if (!syme->skip) {
syme->count[counter]++;
@@ -1270,7 +1279,7 @@ static const struct option options[] = {
OPT_BOOLEAN('i', "inherit", &inherit,
"child tasks inherit counters"),
OPT_STRING('s', "sym-annotate", &sym_filter, "symbol name",
- "symbol to annotate - requires -k option"),
+ "symbol to annotate"),
OPT_BOOLEAN('z', "zero", &zero,
"zero history across updates"),
OPT_INTEGER('F', "freq", &freq,
@@ -1306,16 +1315,14 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
symbol_conf.priv_size = (sizeof(struct sym_entry) +
(nr_counters + 1) * sizeof(unsigned long));
- if (symbol_conf.vmlinux_name == NULL)
- symbol_conf.try_vmlinux_path = true;
+
+ symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
if (symbol__init() < 0)
return -1;
if (delay_secs < 1)
delay_secs = 1;
- parse_source(sym_filter_entry);
-
/*
* User specified count overrides default frequency.
*/
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf annotate: Fix perf top module symbol annotation
2010-02-04 6:31 ` Mike Galbraith
@ 2010-02-04 9:54 ` tip-bot for Mike Galbraith
2010-02-04 19:34 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Kirill Smelkov
1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Mike Galbraith @ 2010-02-04 9:54 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, acme, hpa, mingo, a.p.zijlstra, kirr, efault,
fweisbec, acme, tglx, mingo
Commit-ID: 57d818895f9d294ab9080e5a662675fdee943ff1
Gitweb: http://git.kernel.org/tip/57d818895f9d294ab9080e5a662675fdee943ff1
Author: Mike Galbraith <efault@gmx.de>
AuthorDate: Thu, 4 Feb 2010 07:31:46 +0100
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 4 Feb 2010 09:33:28 +0100
perf annotate: Fix perf top module symbol annotation
Signed-off-by: Mike Galbraith <efault@gmx.de>
Cc: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <1265265106.6364.5.camel@marge.simson.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/builtin-top.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 83c09c8..e4156bc 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -204,8 +204,8 @@ static void parse_source(struct sym_entry *syme)
sprintf(command,
"objdump --start-address=0x%016Lx "
"--stop-address=0x%016Lx -dS %s",
- map->unmap_ip(map, sym->start),
- map->unmap_ip(map, sym->end), path);
+ map__rip_2objdump(map, sym->start),
+ map__rip_2objdump(map, sym->end), path);
file = popen(command, "r");
if (!file)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer
2010-02-04 9:26 ` [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Ingo Molnar
@ 2010-02-04 13:04 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-04 13:04 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Frédéric Weisbecker, Mike Galbraith,
Peter Zijlstra, Paul Mackerras
Em Thu, Feb 04, 2010 at 10:26:13AM +0100, Ingo Molnar escreveu:
> these changes reintroduce the 'perf top exits' bug:
>
> $ perf top -v
> map_groups__set_modules_path_dir: cannot open /lib/modules/2.6.33-rc6-tip-00586-g398dde3-dirty/kernel dir
>
> It's the non-existence of modules that causes a disorderly exit. I sent you
> my config with the earlier bugreport - i think if you tried that config you'd
> see a similar failure. I've fixed it via the commit below for now.
I tried with your config, problem was that it has:
CONFIG_MODULES=y
And lots of things marked as modules and I forgot to forgot to do a
'make modules_install' :-\
Anyway, its fixed now and I have yet another entry for 'perf test' :-)
Thanks!
- Arnaldo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 8/9] perf annotate: fix it for non-prelinked *.so
2010-02-04 6:31 ` Mike Galbraith
2010-02-04 9:54 ` [tip:perf/core] perf annotate: Fix perf top module symbol annotation tip-bot for Mike Galbraith
@ 2010-02-04 19:34 ` Kirill Smelkov
2010-02-04 19:48 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 18+ messages in thread
From: Kirill Smelkov @ 2010-02-04 19:34 UTC (permalink / raw)
To: Mike Galbraith
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel,
Arnaldo Carvalho de Melo
On Thu, Feb 04, 2010 at 07:31:46AM +0100, Mike Galbraith wrote:
> On Wed, 2010-02-03 at 16:52 -0200, Arnaldo Carvalho de Melo wrote:
> > const char *path = NULL;
> > @@ -397,7 +397,8 @@ static void annotate_sym(struct hist_entry *he)
> > dso, dso->long_name, sym, sym->name);
> >
> > sprintf(command, "objdump --start-address=0x%016Lx --stop-address=0x%016Lx -dS %s|grep -v %s",
> > - map->unmap_ip(map, sym->start), map->unmap_ip(map, sym->end),
> > + map__rip_2objdump(map, sym->start),
> > + map__rip_2objdump(map, sym->end),
> > filename, filename);
> >
>
> Monkey see monkey do.
>
> perf tools: fix perf top module symbol annotation.
>
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> LKML-Reference: <new-submission>
>
> ---
> tools/perf/builtin-top.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/tools/perf/builtin-top.c
> ===================================================================
> --- linux-2.6.orig/tools/perf/builtin-top.c
> +++ linux-2.6/tools/perf/builtin-top.c
> @@ -204,8 +204,8 @@ static void parse_source(struct sym_entr
> sprintf(command,
> "objdump --start-address=0x%016Lx "
> "--stop-address=0x%016Lx -dS %s",
> - map->unmap_ip(map, sym->start),
> - map->unmap_ip(map, sym->end), path);
> + map__rip_2objdump(map, sym->start),
> + map__rip_2objdump(map, sym->end), path);
If I recall correctly, that's not enough.
The problem is top code is also wrong at mapping objdump addresses to
absolute ip. That is another part of builtin-top.c which does
map->unmap_ip(), and I've already suggested a fix back at holidays:
http://marc.info/?l=linux-kernel&m=126295508002536&w=2
For the reference, here it is again:
( P.S. and Arnaldo, I'm sorry I still had no luck with spare time.
Overloaded me ... )
---- 8< ----
From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Date: Fri, 8 Jan 2010 15:23:08 +0300
Subject: [PATCH 5/6] perf top: fix annotate for userspace
First, for programs and prelinked libraries, annotate code was fooled by
objdump output IPs (src->eip in the code) being wrongly converted to
absolute IPs. In such case there were no conversion needed, but in
src->eip = strtoull(src->line, NULL, 16);
src->eip = map->unmap_ip(map, src->eip); // = eip + map->start - map->pgoff
we were reading absolute address from objdump (e.g. 8048604) and then
almost doubling it, because eip & map->start are approximately close for
small programs.
Needless to say, that later, in record_precise_ip() there was no
matching with real runtime IPs.
And second, like with `perf annotate` the problem with non-prelinked
*.so was that we were doing rip -> objdump address conversion wrong.
Also, because unlike `perf annotate`, `perf top` code does annotation based on
absolute IPs for performance reasons(*), new helper for mapping objdump
addresse to IP is introduced.
(*) we get samples info in absolute IPs, and since we do lots of
hit-testing on absolute IPs at runtime in record_precise_ip(), it's
better to convert objdump addresses to IPs once and do no conversion
at runtime.
I also had to fix how objdump output is parsed (with hardcoded 8/16
characters format, which was inappropriate for ET_DYN dsos with small
addresses like '4ac')
Also note, that not all objdump output lines has associtated IPs, e.g.
look at source lines here:
000004ac <my_strlen>:
extern "C"
int my_strlen(const char *s)
4ac: 55 push %ebp
4ad: 89 e5 mov %esp,%ebp
4af: 83 ec 10 sub $0x10,%esp
{
int len = 0;
4b2: c7 45 fc 00 00 00 00 movl $0x0,-0x4(%ebp)
4b9: eb 08 jmp 4c3 <my_strlen+0x17>
while (*s) {
++len;
4bb: 83 45 fc 01 addl $0x1,-0x4(%ebp)
++s;
4bf: 83 45 08 01 addl $0x1,0x8(%ebp)
So we mark them with eip=0, and ignore such lines in annotate lookup
code.
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
---
tools/perf/builtin-top.c | 24 ++++++++++++++----------
tools/perf/util/map.c | 8 ++++++++
tools/perf/util/map.h | 3 ++-
3 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4067e4d..6aa9b02 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -176,6 +176,7 @@ static void parse_source(struct sym_entry *syme)
FILE *file;
char command[PATH_MAX*2];
const char *path;
+ char *tmp;
u64 len;
if (!syme)
@@ -204,8 +205,8 @@ static void parse_source(struct sym_entry *syme)
sprintf(command,
"objdump --start-address=0x%016Lx "
"--stop-address=0x%016Lx -dS %s",
- map->unmap_ip(map, sym->start),
- map->unmap_ip(map, sym->end), path);
+ map__rip_2objdump(map, sym->start),
+ map__rip_2objdump(map, sym->end), path);
file = popen(command, "r");
if (!file)
@@ -235,14 +236,13 @@ static void parse_source(struct sym_entry *syme)
*source->lines_tail = src;
source->lines_tail = &src->next;
- if (strlen(src->line)>8 && src->line[8] == ':') {
- src->eip = strtoull(src->line, NULL, 16);
- src->eip = map->unmap_ip(map, src->eip);
- }
- if (strlen(src->line)>8 && src->line[16] == ':') {
- src->eip = strtoull(src->line, NULL, 16);
- src->eip = map->unmap_ip(map, src->eip);
- }
+
+ src->eip = strtoull(src->line, &tmp, 16);
+ if (*tmp == ':')
+ src->eip = map__objdump_2ip(map, src->eip);
+ else
+ /* this line has no ip info (e.g. source line) */
+ src->eip = 0;
}
pclose(file);
out_assign:
@@ -277,6 +277,10 @@ static void record_precise_ip(struct sym_entry *syme, int counter, u64 ip)
goto out_unlock;
for (line = syme->src->lines; line; line = line->next) {
+ /* skip lines without IP info */
+ if (line->eip == 0)
+ continue;
+
if (line->eip == ip) {
line->count[counter]++;
break;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index d6da969..37bfcc5 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -215,3 +215,11 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
rip;
return addr;
}
+
+u64 map__objdump_2ip(struct map *map, u64 addr)
+{
+ u64 ip = map->dso->adjust_symbols ?
+ addr :
+ map->unmap_ip(map, addr); /* RIP -> IP */
+ return ip;
+}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 39fa478..2bcd9d4 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -49,8 +49,9 @@ static inline u64 identity__map_ip(struct map *map __used, u64 ip)
}
-/* rip -> addr suitable for passing to `objdump --start-address=` */
+/* rip/ip <-> addr suitable for passing to `objdump --start-address=` */
u64 map__rip_2objdump(struct map *map, u64 rip);
+u64 map__objdump_2ip(struct map *map, u64 addr);
struct symbol;
--
1.6.6.79.gd514e.dirty
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 8/9] perf annotate: fix it for non-prelinked *.so
2010-02-04 19:34 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Kirill Smelkov
@ 2010-02-04 19:48 ` Arnaldo Carvalho de Melo
2010-02-05 6:54 ` Mike Galbraith
0 siblings, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-02-04 19:48 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: Mike Galbraith, Ingo Molnar, linux-kernel
Em Thu, Feb 04, 2010 at 10:34:04PM +0300, Kirill Smelkov escreveu:
> On Thu, Feb 04, 2010 at 07:31:46AM +0100, Mike Galbraith wrote:
> > Monkey see monkey do.
> >
> > perf tools: fix perf top module symbol annotation.
> > "objdump --start-address=0x%016Lx "
> > "--stop-address=0x%016Lx -dS %s",
> > - map->unmap_ip(map, sym->start),
> > - map->unmap_ip(map, sym->end), path);
> > + map__rip_2objdump(map, sym->start),
> > + map__rip_2objdump(map, sym->end), path);
>
> If I recall correctly, that's not enough.
>
> The problem is top code is also wrong at mapping objdump addresses to
> absolute ip. That is another part of builtin-top.c which does
> map->unmap_ip(), and I've already suggested a fix back at holidays:
>
> http://marc.info/?l=linux-kernel&m=126295508002536&w=2
Yeah, I was looking at Mike's report about 'perf annotate' not working
with modules, debugged, wrote a patch that looed at the ELF header obj
type as the key to apply or not the unmap_ip operation and when it was
working I thought I saw that patch somewhere, looked at yours and
applied it instead.
Then I was testing your subsequent patches but had to call it a day,
sent what I had that at least fixed 'annotate' and Mike took it from
there.
Looking at this now.
- Arnaldo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 8/9] perf annotate: fix it for non-prelinked *.so
2010-02-04 19:48 ` Arnaldo Carvalho de Melo
@ 2010-02-05 6:54 ` Mike Galbraith
0 siblings, 0 replies; 18+ messages in thread
From: Mike Galbraith @ 2010-02-05 6:54 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Kirill Smelkov, Ingo Molnar, linux-kernel
On Thu, 2010-02-04 at 17:48 -0200, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 04, 2010 at 10:34:04PM +0300, Kirill Smelkov escreveu:
> > On Thu, Feb 04, 2010 at 07:31:46AM +0100, Mike Galbraith wrote:
> > > Monkey see monkey do.
> > >
> > > perf tools: fix perf top module symbol annotation.
> > > "objdump --start-address=0x%016Lx "
> > > "--stop-address=0x%016Lx -dS %s",
> > > - map->unmap_ip(map, sym->start),
> > > - map->unmap_ip(map, sym->end), path);
> > > + map__rip_2objdump(map, sym->start),
> > > + map__rip_2objdump(map, sym->end), path);
> >
> > If I recall correctly, that's not enough.
> >
> > The problem is top code is also wrong at mapping objdump addresses to
> > absolute ip. That is another part of builtin-top.c which does
> > map->unmap_ip(), and I've already suggested a fix back at holidays:
> >
> > http://marc.info/?l=linux-kernel&m=126295508002536&w=2
>
> Yeah, I was looking at Mike's report about 'perf annotate' not working
> with modules, debugged, wrote a patch that looed at the ELF header obj
> type as the key to apply or not the unmap_ip operation and when it was
> working I thought I saw that patch somewhere, looked at yours and
> applied it instead.
>
> Then I was testing your subsequent patches but had to call it a day,
> sent what I had that at least fixed 'annotate' and Mike took it from
> there.
>
> Looking at this now.
Hm, seems to work fine.
(piddle)
Aha, the rest is needed for userland annotation to work.
-Mike
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2010-02-05 6:54 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-03 18:52 [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 2/9] perf symbols: Fixup vsyscall maps Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 3/9] perf symbols: Ditch vdso global variable Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 4/9] perf probe: Don't use a perf_session instance just to resolve symbols Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 5/9] perf build-id: Move the routine to find DSOs with hits to the lib Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 6/9] perf record: Stop intercepting events, use postprocessing to get build-ids Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 7/9] perf tools: Adjust some verbosity levels Arnaldo Carvalho de Melo
2010-02-03 18:52 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Arnaldo Carvalho de Melo
2010-02-04 6:31 ` Mike Galbraith
2010-02-04 9:54 ` [tip:perf/core] perf annotate: Fix perf top module symbol annotation tip-bot for Mike Galbraith
2010-02-04 19:34 ` [PATCH 8/9] perf annotate: fix it for non-prelinked *.so Kirill Smelkov
2010-02-04 19:48 ` Arnaldo Carvalho de Melo
2010-02-05 6:54 ` Mike Galbraith
2010-02-04 9:54 ` [tip:perf/core] perf annotate: Fix " tip-bot for Kirill Smelkov
2010-02-03 18:52 ` [PATCH 9/9] perf top: teach it to autolocate vmlinux Arnaldo Carvalho de Melo
2010-02-04 9:54 ` [tip:perf/core] perf top: Teach " tip-bot for Kirill Smelkov
2010-02-04 9:26 ` [PATCH 1/9] perf symbols: Remove perf_session usage in symbols layer Ingo Molnar
2010-02-04 13:04 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.