From 1eee78aea9252fabcd333805d5d9fa42a1bf9427 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 22 May 2015 12:58:53 -0300 Subject: [PATCH 1/7] perf tools: Introduce struct maps That for now has the maps rbtree and the list for the dead maps, that may be still referenced from some hist_entry, etc. This paves the way for protecting the rbtree with a lock, then refcount the maps and finally remove the removed_maps list, as it'll not ne anymore needed. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-fl0fa6142pj8khj97fow3uw0@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/vmlinux-kallsyms.c | 2 +- tools/perf/util/event.c | 2 +- tools/perf/util/map.c | 64 +++++++++++++++++------------ tools/perf/util/map.h | 16 +++++--- tools/perf/util/probe-event.c | 2 +- tools/perf/util/symbol.c | 4 +- 6 files changed, 52 insertions(+), 38 deletions(-) diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c index 94ac6924df65e..b34c5fc829ae2 100644 --- a/tools/perf/tests/vmlinux-kallsyms.c +++ b/tools/perf/tests/vmlinux-kallsyms.c @@ -26,7 +26,7 @@ int test__vmlinux_matches_kallsyms(void) struct map *kallsyms_map, *vmlinux_map, *map; struct machine kallsyms, vmlinux; enum map_type type = MAP__FUNCTION; - struct rb_root *maps = &vmlinux.kmaps.maps[type]; + struct maps *maps = &vmlinux.kmaps.maps[type]; u64 mem_start, mem_end; /* diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index 9d3bba1754235..c1925968a8afe 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -331,7 +331,7 @@ int perf_event__synthesize_modules(struct perf_tool *tool, int rc = 0; struct map *pos; struct map_groups *kmaps = &machine->kmaps; - struct rb_root *maps = &kmaps->maps[MAP__FUNCTION]; + struct maps *maps = &kmaps->maps[MAP__FUNCTION]; union perf_event *event = zalloc((sizeof(event->mmap) + machine->id_hdr_size)); if (event == NULL) { diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index 898ab92a98dd2..adf012c4d6504 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -418,48 +418,58 @@ u64 map__objdump_2mem(struct map *map, u64 ip) return ip + map->reloc; } +static void maps__init(struct maps *maps) +{ + maps->entries = RB_ROOT; + INIT_LIST_HEAD(&maps->removed_maps); +} + void map_groups__init(struct map_groups *mg, struct machine *machine) { int i; for (i = 0; i < MAP__NR_TYPES; ++i) { - mg->maps[i] = RB_ROOT; - INIT_LIST_HEAD(&mg->removed_maps[i]); + maps__init(&mg->maps[i]); } mg->machine = machine; atomic_set(&mg->refcnt, 1); } -static void maps__delete(struct rb_root *maps) +static void maps__purge(struct maps *maps) { - struct rb_node *next = rb_first(maps); + struct rb_root *root = &maps->entries; + struct rb_node *next = rb_first(root); while (next) { struct map *pos = rb_entry(next, struct map, rb_node); next = rb_next(&pos->rb_node); - rb_erase(&pos->rb_node, maps); + rb_erase(&pos->rb_node, root); map__delete(pos); } } -static void maps__delete_removed(struct list_head *maps) +static void maps__purge_removed_maps(struct maps *maps) { struct map *pos, *n; - list_for_each_entry_safe(pos, n, maps, node) { + list_for_each_entry_safe(pos, n, &maps->removed_maps, node) { list_del(&pos->node); map__delete(pos); } } +static void maps__exit(struct maps *maps) +{ + maps__purge(maps); + maps__purge_removed_maps(maps); +} + void map_groups__exit(struct map_groups *mg) { int i; - for (i = 0; i < MAP__NR_TYPES; ++i) { - maps__delete(&mg->maps[i]); - maps__delete_removed(&mg->removed_maps[i]); - } + for (i = 0; i < MAP__NR_TYPES; ++i) + maps__exit(&mg->maps[i]); } bool map_groups__empty(struct map_groups *mg) @@ -469,7 +479,7 @@ bool map_groups__empty(struct map_groups *mg) for (i = 0; i < MAP__NR_TYPES; ++i) { if (maps__first(&mg->maps[i])) return false; - if (!list_empty(&mg->removed_maps[i])) + if (!list_empty(&mg->maps[i].removed_maps)) return false; } @@ -523,7 +533,7 @@ struct symbol *map_groups__find_symbol_by_name(struct map_groups *mg, { struct rb_node *nd; - for (nd = rb_first(&mg->maps[type]); nd; nd = rb_next(nd)) { + for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) { struct map *pos = rb_entry(nd, struct map, rb_node); struct symbol *sym = map__find_symbol_by_name(pos, name, filter); @@ -560,7 +570,7 @@ size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type, size_t printed = fprintf(fp, "%s:\n", map_type__name[type]); struct rb_node *nd; - for (nd = rb_first(&mg->maps[type]); nd; nd = rb_next(nd)) { + for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) { struct map *pos = rb_entry(nd, struct map, rb_node); printed += fprintf(fp, "Map:"); printed += map__fprintf(pos, fp); @@ -587,7 +597,7 @@ static size_t __map_groups__fprintf_removed_maps(struct map_groups *mg, struct map *pos; size_t printed = 0; - list_for_each_entry(pos, &mg->removed_maps[type], node) { + list_for_each_entry(pos, &mg->maps[type].removed_maps, node) { printed += fprintf(fp, "Map:"); printed += map__fprintf(pos, fp); if (verbose > 1) { @@ -617,7 +627,7 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp) int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE *fp) { - struct rb_root *root = &mg->maps[map->type]; + struct rb_root *root = &mg->maps[map->type].entries; struct rb_node *next = rb_first(root); int err = 0; @@ -671,7 +681,7 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, * If we have references, just move them to a separate list. */ if (pos->referenced) - list_add_tail(&pos->node, &mg->removed_maps[map->type]); + list_add_tail(&pos->node, &mg->maps[map->type].removed_maps); else map__delete(pos); @@ -689,7 +699,7 @@ int map_groups__clone(struct map_groups *mg, struct map_groups *parent, enum map_type type) { struct map *map; - struct rb_root *maps = &parent->maps[type]; + struct maps *maps = &parent->maps[type]; for (map = maps__first(maps); map; map = map__next(map)) { struct map *new = map__clone(map); @@ -700,9 +710,9 @@ int map_groups__clone(struct map_groups *mg, return 0; } -void maps__insert(struct rb_root *maps, struct map *map) +void maps__insert(struct maps *maps, struct map *map) { - struct rb_node **p = &maps->rb_node; + struct rb_node **p = &maps->entries.rb_node; struct rb_node *parent = NULL; const u64 ip = map->start; struct map *m; @@ -717,17 +727,17 @@ void maps__insert(struct rb_root *maps, struct map *map) } rb_link_node(&map->rb_node, parent, p); - rb_insert_color(&map->rb_node, maps); + rb_insert_color(&map->rb_node, &maps->entries); } -void maps__remove(struct rb_root *maps, struct map *map) +void maps__remove(struct maps *maps, struct map *map) { - rb_erase(&map->rb_node, maps); + rb_erase(&map->rb_node, &maps->entries); } -struct map *maps__find(struct rb_root *maps, u64 ip) +struct map *maps__find(struct maps *maps, u64 ip) { - struct rb_node **p = &maps->rb_node; + struct rb_node **p = &maps->entries.rb_node; struct rb_node *parent = NULL; struct map *m; @@ -745,9 +755,9 @@ struct map *maps__find(struct rb_root *maps, u64 ip) return NULL; } -struct map *maps__first(struct rb_root *maps) +struct map *maps__first(struct maps *maps) { - struct rb_node *first = rb_first(maps); + struct rb_node *first = rb_first(&maps->entries); if (first) return rb_entry(first, struct map, rb_node); diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index f2b27566d9864..e3702fd468c53 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -58,9 +58,13 @@ struct kmap { struct map_groups *kmaps; }; +struct maps { + struct rb_root entries; + struct list_head removed_maps; +}; + struct map_groups { - struct rb_root maps[MAP__NR_TYPES]; - struct list_head removed_maps[MAP__NR_TYPES]; + struct maps maps[MAP__NR_TYPES]; struct machine *machine; atomic_t refcnt; }; @@ -162,10 +166,10 @@ void map__reloc_vmlinux(struct map *map); size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type, FILE *fp); -void maps__insert(struct rb_root *maps, struct map *map); -void maps__remove(struct rb_root *maps, struct map *map); -struct map *maps__find(struct rb_root *maps, u64 addr); -struct map *maps__first(struct rb_root *maps); +void maps__insert(struct maps *maps, struct map *map); +void maps__remove(struct maps *maps, struct map *map); +struct map *maps__find(struct maps *maps, u64 addr); +struct map *maps__first(struct maps *maps); struct map *map__next(struct map *map); void map_groups__init(struct map_groups *mg, struct machine *machine); void map_groups__exit(struct map_groups *mg); diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 97da98481d892..32471d0839d12 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -163,7 +163,7 @@ static u64 kernel_get_symbol_address_by_name(const char *name, bool reloc) static struct map *kernel_get_module_map(const char *module) { struct map_groups *grp = &host_machine->kmaps; - struct rb_root *maps = &grp->maps[MAP__FUNCTION]; + struct maps *maps = &grp->maps[MAP__FUNCTION]; struct map *pos; /* A file path -- this is an offline module */ diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index b9e3eb5818840..c8a3e79c5da21 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -202,7 +202,7 @@ void symbols__fixup_end(struct rb_root *symbols) void __map_groups__fixup_end(struct map_groups *mg, enum map_type type) { - struct rb_root *maps = &mg->maps[type]; + struct maps *maps = &mg->maps[type]; struct map *next, *curr; curr = maps__first(maps); @@ -1520,7 +1520,7 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter) struct map *map_groups__find_by_name(struct map_groups *mg, enum map_type type, const char *name) { - struct rb_root *maps = &mg->maps[type]; + struct maps *maps = &mg->maps[type]; struct map *map; for (map = maps__first(maps); map; map = map__next(map)) { From 6a2ffcddad22ead7ce75c5773e87895b91e7cca7 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 22 May 2015 13:45:24 -0300 Subject: [PATCH 2/7] perf tools: Protect accesses the map rbtrees with a rw lock To allow concurrent access, next step: refcount struct map instances, so that we can ditch maps->removed_maps and stop leaking threads, maps, then struct DSO needs the same treatment. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-o45w2w5dzrza38nzqxnqzhyf@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/map.c | 122 +++++++++++++++++++++++++++++---------- tools/perf/util/map.h | 2 + tools/perf/util/symbol.c | 17 +++++- 3 files changed, 108 insertions(+), 33 deletions(-) diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index adf012c4d6504..0905b07072da5 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -16,6 +16,8 @@ #include "machine.h" #include +static void __maps__insert(struct maps *maps, struct map *map); + const char *map_type__name[MAP__NR_TYPES] = { [MAP__FUNCTION] = "Functions", [MAP__VARIABLE] = "Variables", @@ -421,6 +423,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip) static void maps__init(struct maps *maps) { maps->entries = RB_ROOT; + pthread_rwlock_init(&maps->lock, NULL); INIT_LIST_HEAD(&maps->removed_maps); } @@ -434,7 +437,7 @@ void map_groups__init(struct map_groups *mg, struct machine *machine) atomic_set(&mg->refcnt, 1); } -static void maps__purge(struct maps *maps) +static void __maps__purge(struct maps *maps) { struct rb_root *root = &maps->entries; struct rb_node *next = rb_first(root); @@ -448,7 +451,7 @@ static void maps__purge(struct maps *maps) } } -static void maps__purge_removed_maps(struct maps *maps) +static void __maps__purge_removed_maps(struct maps *maps) { struct map *pos, *n; @@ -460,8 +463,10 @@ static void maps__purge_removed_maps(struct maps *maps) static void maps__exit(struct maps *maps) { - maps__purge(maps); - maps__purge_removed_maps(maps); + pthread_rwlock_wrlock(&maps->lock); + __maps__purge(maps); + __maps__purge_removed_maps(maps); + pthread_rwlock_unlock(&maps->lock); } void map_groups__exit(struct map_groups *mg) @@ -531,20 +536,28 @@ struct symbol *map_groups__find_symbol_by_name(struct map_groups *mg, struct map **mapp, symbol_filter_t filter) { + struct maps *maps = &mg->maps[type]; + struct symbol *sym; struct rb_node *nd; - for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) { + pthread_rwlock_rdlock(&maps->lock); + + for (nd = rb_first(&maps->entries); nd; nd = rb_next(nd)) { struct map *pos = rb_entry(nd, struct map, rb_node); - struct symbol *sym = map__find_symbol_by_name(pos, name, filter); + + sym = map__find_symbol_by_name(pos, name, filter); if (sym == NULL) continue; if (mapp != NULL) *mapp = pos; - return sym; + goto out; } - return NULL; + sym = NULL; +out: + pthread_rwlock_unlock(&maps->lock); + return sym; } int map_groups__find_ams(struct addr_map_symbol *ams, symbol_filter_t filter) @@ -564,25 +577,35 @@ int map_groups__find_ams(struct addr_map_symbol *ams, symbol_filter_t filter) return ams->sym ? 0 : -1; } -size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type, - FILE *fp) +static size_t maps__fprintf(struct maps *maps, FILE *fp) { - size_t printed = fprintf(fp, "%s:\n", map_type__name[type]); + size_t printed = 0; struct rb_node *nd; - for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) { + pthread_rwlock_rdlock(&maps->lock); + + for (nd = rb_first(&maps->entries); nd; nd = rb_next(nd)) { struct map *pos = rb_entry(nd, struct map, rb_node); printed += fprintf(fp, "Map:"); printed += map__fprintf(pos, fp); if (verbose > 2) { - printed += dso__fprintf(pos->dso, type, fp); + printed += dso__fprintf(pos->dso, pos->type, fp); printed += fprintf(fp, "--\n"); } } + pthread_rwlock_unlock(&maps->lock); + return printed; } +size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type, + FILE *fp) +{ + size_t printed = fprintf(fp, "%s:\n", map_type__name[type]); + return printed += maps__fprintf(&mg->maps[type], fp); +} + static size_t map_groups__fprintf_maps(struct map_groups *mg, FILE *fp) { size_t printed = 0, i; @@ -624,13 +647,17 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp) return printed + map_groups__fprintf_removed_maps(mg, fp); } -int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, - FILE *fp) +static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp) { - struct rb_root *root = &mg->maps[map->type].entries; - struct rb_node *next = rb_first(root); + struct rb_root *root; + struct rb_node *next; int err = 0; + pthread_rwlock_wrlock(&maps->lock); + + root = &maps->entries; + next = rb_first(root); + while (next) { struct map *pos = rb_entry(next, struct map, rb_node); next = rb_next(&pos->rb_node); @@ -658,7 +685,7 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, } before->end = map->start; - map_groups__insert(mg, before); + __maps__insert(maps, before); if (verbose >= 2) map__fprintf(before, fp); } @@ -672,7 +699,7 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, } after->start = map->end; - map_groups__insert(mg, after); + __maps__insert(maps, after); if (verbose >= 2) map__fprintf(after, fp); } @@ -681,15 +708,24 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, * If we have references, just move them to a separate list. */ if (pos->referenced) - list_add_tail(&pos->node, &mg->maps[map->type].removed_maps); + list_add_tail(&pos->node, &maps->removed_maps); else map__delete(pos); if (err) - return err; + goto out; } - return 0; + err = 0; +out: + pthread_rwlock_unlock(&maps->lock); + return err; +} + +int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, + FILE *fp) +{ + return maps__fixup_overlappings(&mg->maps[map->type], map, fp); } /* @@ -698,19 +734,26 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, int map_groups__clone(struct map_groups *mg, struct map_groups *parent, enum map_type type) { + int err = -ENOMEM; struct map *map; struct maps *maps = &parent->maps[type]; + pthread_rwlock_rdlock(&maps->lock); + for (map = maps__first(maps); map; map = map__next(map)) { struct map *new = map__clone(map); if (new == NULL) - return -ENOMEM; + goto out_unlock; map_groups__insert(mg, new); } - return 0; + + err = 0; +out_unlock: + pthread_rwlock_unlock(&maps->lock); + return err; } -void maps__insert(struct maps *maps, struct map *map) +static void __maps__insert(struct maps *maps, struct map *map) { struct rb_node **p = &maps->entries.rb_node; struct rb_node *parent = NULL; @@ -730,17 +773,33 @@ void maps__insert(struct maps *maps, struct map *map) rb_insert_color(&map->rb_node, &maps->entries); } -void maps__remove(struct maps *maps, struct map *map) +void maps__insert(struct maps *maps, struct map *map) +{ + pthread_rwlock_wrlock(&maps->lock); + __maps__insert(maps, map); + pthread_rwlock_unlock(&maps->lock); +} + +static void __maps__remove(struct maps *maps, struct map *map) { rb_erase(&map->rb_node, &maps->entries); } +void maps__remove(struct maps *maps, struct map *map) +{ + pthread_rwlock_wrlock(&maps->lock); + __maps__remove(maps, map); + pthread_rwlock_unlock(&maps->lock); +} + struct map *maps__find(struct maps *maps, u64 ip) { - struct rb_node **p = &maps->entries.rb_node; - struct rb_node *parent = NULL; + struct rb_node **p, *parent = NULL; struct map *m; + pthread_rwlock_rdlock(&maps->lock); + + p = &maps->entries.rb_node; while (*p != NULL) { parent = *p; m = rb_entry(parent, struct map, rb_node); @@ -749,10 +808,13 @@ struct map *maps__find(struct maps *maps, u64 ip) else if (ip >= m->end) p = &(*p)->rb_right; else - return m; + goto out; } - return NULL; + m = NULL; +out: + pthread_rwlock_unlock(&maps->lock); + return m; } struct map *maps__first(struct maps *maps) diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index e3702fd468c53..6796f2785649a 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,7 @@ struct kmap { struct maps { struct rb_root entries; + pthread_rwlock_t lock; struct list_head removed_maps; }; diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index c8a3e79c5da21..8aae8b6b1ceed 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -205,9 +205,11 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type) struct maps *maps = &mg->maps[type]; struct map *next, *curr; + pthread_rwlock_wrlock(&maps->lock); + curr = maps__first(maps); if (curr == NULL) - return; + goto out_unlock; for (next = map__next(curr); next; next = map__next(curr)) { curr->end = next->start; @@ -219,6 +221,9 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type) * last map final address. */ curr->end = ~0ULL; + +out_unlock: + pthread_rwlock_unlock(&maps->lock); } struct symbol *symbol__new(u64 start, u64 len, u8 binding, const char *name) @@ -1523,12 +1528,18 @@ struct map *map_groups__find_by_name(struct map_groups *mg, struct maps *maps = &mg->maps[type]; struct map *map; + pthread_rwlock_rdlock(&maps->lock); + for (map = maps__first(maps); map; map = map__next(map)) { if (map->dso && strcmp(map->dso->short_name, name) == 0) - return map; + goto out_unlock; } - return NULL; + map = NULL; + +out_unlock: + pthread_rwlock_unlock(&maps->lock); + return map; } int dso__load_vmlinux(struct dso *dso, struct map *map, From facf3f0621b2e11957af1aae9085730ea78ccf85 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 25 May 2015 15:30:09 -0300 Subject: [PATCH 3/7] perf tools: Check if a map is still in use when deleting it I.e. match RB_CLEAR_NODE() with RB_EMPTY_NODE(), to check that it isn't in a rb tree at the time of its deletion. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-vumvhird765id11zbx00d2r8@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 4 ++++ tools/perf/util/map.c | 9 +++++---- tools/perf/util/symbol.c | 8 ++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index b57a027fb200d..c434e1264087f 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -59,6 +59,10 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel, (al->sym == NULL || strcmp(ann->sym_hist_filter, al->sym->name) != 0)) { /* We're only interested in a symbol named sym_hist_filter */ + /* + * FIXME: why isn't this done in the symbol_filter when loading + * the DSO? + */ if (al->sym != NULL) { rb_erase(&al->sym->rb_node, &al->map->dso->symbols[al->map->type]); diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index 0905b07072da5..4d3a92d5dff3e 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -225,6 +225,7 @@ struct map *map__new2(u64 start, struct dso *dso, enum map_type type) void map__delete(struct map *map) { + BUG_ON(!RB_EMPTY_NODE(&map->rb_node)); free(map); } @@ -446,7 +447,7 @@ static void __maps__purge(struct maps *maps) struct map *pos = rb_entry(next, struct map, rb_node); next = rb_next(&pos->rb_node); - rb_erase(&pos->rb_node, root); + rb_erase_init(&pos->rb_node, root); map__delete(pos); } } @@ -456,7 +457,7 @@ static void __maps__purge_removed_maps(struct maps *maps) struct map *pos, *n; list_for_each_entry_safe(pos, n, &maps->removed_maps, node) { - list_del(&pos->node); + list_del_init(&pos->node); map__delete(pos); } } @@ -671,7 +672,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp map__fprintf(pos, fp); } - rb_erase(&pos->rb_node, root); + rb_erase_init(&pos->rb_node, root); /* * Now check if we need to create new maps for areas not * overlapped by the new map: @@ -782,7 +783,7 @@ void maps__insert(struct maps *maps, struct map *map) static void __maps__remove(struct maps *maps, struct map *map) { - rb_erase(&map->rb_node, &maps->entries); + rb_erase_init(&map->rb_node, &maps->entries); } void maps__remove(struct maps *maps, struct map *map) diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 8aae8b6b1ceed..743a9b360e3dc 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -659,14 +659,14 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map, curr_map = map_groups__find(kmaps, map->type, pos->start); if (!curr_map || (filter && filter(curr_map, pos))) { - rb_erase(&pos->rb_node, root); + rb_erase_init(&pos->rb_node, root); symbol__delete(pos); } else { pos->start -= curr_map->start - curr_map->pgoff; if (pos->end) pos->end -= curr_map->start - curr_map->pgoff; if (curr_map != map) { - rb_erase(&pos->rb_node, root); + rb_erase_init(&pos->rb_node, root); symbols__insert( &curr_map->dso->symbols[curr_map->type], pos); @@ -1173,7 +1173,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, /* Add new maps */ while (!list_empty(&md.maps)) { new_map = list_entry(md.maps.next, struct map, node); - list_del(&new_map->node); + list_del_init(&new_map->node); if (new_map == replacement_map) { map->start = new_map->start; map->end = new_map->end; @@ -1211,7 +1211,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, out_err: while (!list_empty(&md.maps)) { map = list_entry(md.maps.next, struct map, node); - list_del(&map->node); + list_del_init(&map->node); map__delete(map); } close(fd); From 84c2cafa288939e11d21c7830e32b2aee21b723e Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 25 May 2015 16:59:56 -0300 Subject: [PATCH 4/7] perf tools: Reference count struct map We have pointers to struct map instances in several places, like in the hist_entry instances, so we need a way to know when we can destroy them, otherwise we may either keep leaking them or end up referencing deleted instances. Start fixing it by reference counting them. This patch puts the reference count for struct map in place, replacing direct map__delete() calls with map__put() ones and then grabbing a reference count when adding it to the maps struct where maps for a struct thread are kept. Next we'll grab reference counts when setting pointers to struct map instances, in places like in the hist_entry code. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-wi19xczk0t2a41r1i2chuio5@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 3 ++- tools/perf/util/map.c | 21 +++++++++++++++------ tools/perf/util/map.h | 11 +++++++++++ tools/perf/util/probe-event.c | 6 +++--- tools/perf/util/symbol-elf.c | 2 ++ tools/perf/util/symbol.c | 7 +++++-- 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 6bf845758ae38..0c0e61cce577a 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -759,7 +759,6 @@ void machine__destroy_kernel_maps(struct machine *machine) kmap->ref_reloc_sym = NULL; } - map__delete(machine->vmlinux_maps[type]); machine->vmlinux_maps[type] = NULL; } } @@ -1247,6 +1246,7 @@ int machine__process_mmap2_event(struct machine *machine, thread__insert_map(thread, map); thread__put(thread); + map__put(map); return 0; out_problem_map: @@ -1297,6 +1297,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event thread__insert_map(thread, map); thread__put(thread); + map__put(map); return 0; out_problem_map: diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index 4d3a92d5dff3e..af572322586d2 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -139,6 +139,7 @@ void map__init(struct map *map, enum map_type type, map->groups = NULL; map->referenced = false; map->erange_warned = false; + atomic_set(&map->refcnt, 1); } struct map *map__new(struct machine *machine, u64 start, u64 len, @@ -229,6 +230,12 @@ void map__delete(struct map *map) free(map); } +void map__put(struct map *map) +{ + if (map && atomic_dec_and_test(&map->refcnt)) + map__delete(map); +} + void map__fixup_start(struct map *map) { struct rb_root *symbols = &map->dso->symbols[map->type]; @@ -448,7 +455,7 @@ static void __maps__purge(struct maps *maps) next = rb_next(&pos->rb_node); rb_erase_init(&pos->rb_node, root); - map__delete(pos); + map__put(pos); } } @@ -458,7 +465,7 @@ static void __maps__purge_removed_maps(struct maps *maps) list_for_each_entry_safe(pos, n, &maps->removed_maps, node) { list_del_init(&pos->node); - map__delete(pos); + map__put(pos); } } @@ -682,7 +689,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp if (before == NULL) { err = -ENOMEM; - goto move_map; + goto put_map; } before->end = map->start; @@ -696,7 +703,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp if (after == NULL) { err = -ENOMEM; - goto move_map; + goto put_map; } after->start = map->end; @@ -704,14 +711,14 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp if (verbose >= 2) map__fprintf(after, fp); } -move_map: +put_map: /* * If we have references, just move them to a separate list. */ if (pos->referenced) list_add_tail(&pos->node, &maps->removed_maps); else - map__delete(pos); + map__put(pos); if (err) goto out; @@ -772,6 +779,7 @@ static void __maps__insert(struct maps *maps, struct map *map) rb_link_node(&map->rb_node, parent, p); rb_insert_color(&map->rb_node, &maps->entries); + map__get(map); } void maps__insert(struct maps *maps, struct map *map) @@ -784,6 +792,7 @@ void maps__insert(struct maps *maps, struct map *map) static void __maps__remove(struct maps *maps, struct map *map) { rb_erase_init(&map->rb_node, &maps->entries); + map__put(map); } void maps__remove(struct maps *maps, struct map *map) diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index 6796f2785649a..b8df09d94aca5 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -52,6 +52,7 @@ struct map { struct dso *dso; struct map_groups *groups; + atomic_t refcnt; }; struct kmap { @@ -150,6 +151,16 @@ struct map *map__new(struct machine *machine, u64 start, u64 len, struct map *map__new2(u64 start, struct dso *dso, enum map_type type); void map__delete(struct map *map); struct map *map__clone(struct map *map); + +static inline struct map *map__get(struct map *map) +{ + if (map) + atomic_inc(&map->refcnt); + return map; +} + +void map__put(struct map *map); + int map__overlap(struct map *l, struct map *r); size_t map__fprintf(struct map *map, FILE *fp); size_t map__fprintf_dsoname(struct map *map, FILE *fp); diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 32471d0839d12..b0b8a8080009c 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -195,7 +195,7 @@ static void put_target_map(struct map *map, bool user) { if (map && user) { /* Only the user map needs to be released */ - map__delete(map); + map__put(map); } } @@ -1791,7 +1791,7 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp, out: if (map && !is_kprobe) { - map__delete(map); + map__put(map); } return ret; @@ -2884,7 +2884,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter, dso__fprintf_symbols_by_name(map->dso, map->type, stdout); end: if (user) { - map__delete(map); + map__put(map); } exit_symbol_maps(); diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 9d526a5312b1d..fa10116a12abb 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -972,8 +972,10 @@ int dso__load_sym(struct dso *dso, struct map *map, map->unmap_ip = map__unmap_ip; /* Ensure maps are correctly ordered */ if (kmaps) { + map__get(map); map_groups__remove(kmaps, map); map_groups__insert(kmaps, map); + map__put(map); } } diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 743a9b360e3dc..a3e80d6ad70ae 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1180,13 +1180,16 @@ static int dso__load_kcore(struct dso *dso, struct map *map, map->pgoff = new_map->pgoff; map->map_ip = new_map->map_ip; map->unmap_ip = new_map->unmap_ip; - map__delete(new_map); /* Ensure maps are correctly ordered */ + map__get(map); map_groups__remove(kmaps, map); map_groups__insert(kmaps, map); + map__put(map); } else { map_groups__insert(kmaps, new_map); } + + map__put(new_map); } /* @@ -1212,7 +1215,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, while (!list_empty(&md.maps)) { map = list_entry(md.maps.next, struct map, node); list_del_init(&map->node); - map__delete(map); + map__put(map); } close(fd); return -EINVAL; From 18ffdfe8e98f861a39590ef2374ad51fc963567e Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 25 May 2015 22:51:54 +0200 Subject: [PATCH 5/7] perf tools: Add hint for 'Too many events are opened.' error message Enhancing the 'Too many events are opened.' error message with hint to use use 'ulimit -n ' command. Before: $ perf record -e 'sched:*,syscalls:*' ls Error: Too many events are opened. Try again after reducing the number of events. Now: $ perf record -e 'sched:*,syscalls:*' ls Error: Too many events are opened. Probably the maximum number of open file descriptors has been reached. Hint: Try again after reducing the number of events. Hint: Try increasing the limit with 'ulimit -n ' Signed-off-by: Jiri Olsa Cc: David Ahern Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1432587114-14924-1-git-send-email-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evsel.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index c886b9f7a48d0..a3e36fc634dc4 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -2149,7 +2149,9 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target, case EMFILE: return scnprintf(msg, size, "%s", "Too many events are opened.\n" - "Try again after reducing the number of events."); + "Probably the maximum number of open file descriptors has been reached.\n" + "Hint: Try again after reducing the number of events.\n" + "Hint: Try increasing the limit with 'ulimit -n '"); case ENODEV: if (target->cpu_list) return scnprintf(msg, size, "%s", From 83be34a7a913bdf9f21f524333c63d9c48a28ef4 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Wed, 27 May 2015 10:51:46 -0700 Subject: [PATCH 6/7] perf annotation: Add symbol__get_annotation Add a new utility function to get an function annotation out of existing code. Signed-off-by: Andi Kleen Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1432749114-904-4-git-send-email-andi@firstfloor.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 7f5bdfc9bc87d..bf80430099098 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -506,6 +506,17 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, return 0; } +static struct annotation *symbol__get_annotation(struct symbol *sym) +{ + struct annotation *notes = symbol__annotation(sym); + + if (notes->src == NULL) { + if (symbol__alloc_hist(sym) < 0) + return NULL; + } + return notes; +} + static int symbol__inc_addr_samples(struct symbol *sym, struct map *map, int evidx, u64 addr) { @@ -513,13 +524,9 @@ static int symbol__inc_addr_samples(struct symbol *sym, struct map *map, if (sym == NULL) return 0; - - notes = symbol__annotation(sym); - if (notes->src == NULL) { - if (symbol__alloc_hist(sym) < 0) - return -ENOMEM; - } - + notes = symbol__get_annotation(sym); + if (notes == NULL) + return -ENOMEM; return __symbol__inc_addr_samples(sym, map, notes, evidx, addr); } From f00898f4e20b286877b8d6d96d6e404661fd7985 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Wed, 27 May 2015 10:51:51 -0700 Subject: [PATCH 7/7] perf tools: Move branch option parsing to own file .. to allow sharing between builtin-record and builtin-top later. No code changes, just moved code. Signed-off-by: Andi Kleen Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1432749114-904-9-git-send-email-andi@firstfloor.org [ Rename too generic branch.[ch] name to parse-branch-options.[ch] ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-record.c | 89 +----------------------- tools/perf/util/Build | 1 + tools/perf/util/parse-branch-options.c | 93 ++++++++++++++++++++++++++ tools/perf/util/parse-branch-options.h | 5 ++ 4 files changed, 100 insertions(+), 88 deletions(-) create mode 100644 tools/perf/util/parse-branch-options.c create mode 100644 tools/perf/util/parse-branch-options.h diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 5dfe913956177..91aa2a3dcf19e 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -28,6 +28,7 @@ #include "util/thread_map.h" #include "util/data.h" #include "util/auxtrace.h" +#include "util/parse-branch-options.h" #include #include @@ -751,94 +752,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) return status; } -#define BRANCH_OPT(n, m) \ - { .name = n, .mode = (m) } - -#define BRANCH_END { .name = NULL } - -struct branch_mode { - const char *name; - int mode; -}; - -static const struct branch_mode branch_modes[] = { - BRANCH_OPT("u", PERF_SAMPLE_BRANCH_USER), - BRANCH_OPT("k", PERF_SAMPLE_BRANCH_KERNEL), - BRANCH_OPT("hv", PERF_SAMPLE_BRANCH_HV), - BRANCH_OPT("any", PERF_SAMPLE_BRANCH_ANY), - BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL), - BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN), - BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL), - BRANCH_OPT("abort_tx", PERF_SAMPLE_BRANCH_ABORT_TX), - BRANCH_OPT("in_tx", PERF_SAMPLE_BRANCH_IN_TX), - BRANCH_OPT("no_tx", PERF_SAMPLE_BRANCH_NO_TX), - BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_COND), - BRANCH_END -}; - -static int -parse_branch_stack(const struct option *opt, const char *str, int unset) -{ -#define ONLY_PLM \ - (PERF_SAMPLE_BRANCH_USER |\ - PERF_SAMPLE_BRANCH_KERNEL |\ - PERF_SAMPLE_BRANCH_HV) - - uint64_t *mode = (uint64_t *)opt->value; - const struct branch_mode *br; - char *s, *os = NULL, *p; - int ret = -1; - - if (unset) - return 0; - - /* - * cannot set it twice, -b + --branch-filter for instance - */ - if (*mode) - return -1; - - /* str may be NULL in case no arg is passed to -b */ - if (str) { - /* because str is read-only */ - s = os = strdup(str); - if (!s) - return -1; - - for (;;) { - p = strchr(s, ','); - if (p) - *p = '\0'; - - for (br = branch_modes; br->name; br++) { - if (!strcasecmp(s, br->name)) - break; - } - if (!br->name) { - ui__warning("unknown branch filter %s," - " check man page\n", s); - goto error; - } - - *mode |= br->mode; - - if (!p) - break; - - s = p + 1; - } - } - ret = 0; - - /* default to any branch */ - if ((*mode & ~ONLY_PLM) == 0) { - *mode = PERF_SAMPLE_BRANCH_ANY; - } -error: - free(os); - return ret; -} - static void callchain_debug(void) { static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF", "LBR" }; diff --git a/tools/perf/util/Build b/tools/perf/util/Build index 6966d0743bf75..e4b676de2f643 100644 --- a/tools/perf/util/Build +++ b/tools/perf/util/Build @@ -75,6 +75,7 @@ libperf-$(CONFIG_X86) += tsc.o libperf-y += cloexec.o libperf-y += thread-stack.o libperf-$(CONFIG_AUXTRACE) += auxtrace.o +libperf-y += parse-branch-options.o libperf-$(CONFIG_LIBELF) += symbol-elf.o libperf-$(CONFIG_LIBELF) += probe-event.o diff --git a/tools/perf/util/parse-branch-options.c b/tools/perf/util/parse-branch-options.c new file mode 100644 index 0000000000000..9d999436658fd --- /dev/null +++ b/tools/perf/util/parse-branch-options.c @@ -0,0 +1,93 @@ +#include "perf.h" +#include "util/util.h" +#include "util/debug.h" +#include "util/parse-options.h" +#include "util/parse-branch-options.h" + +#define BRANCH_OPT(n, m) \ + { .name = n, .mode = (m) } + +#define BRANCH_END { .name = NULL } + +struct branch_mode { + const char *name; + int mode; +}; + +static const struct branch_mode branch_modes[] = { + BRANCH_OPT("u", PERF_SAMPLE_BRANCH_USER), + BRANCH_OPT("k", PERF_SAMPLE_BRANCH_KERNEL), + BRANCH_OPT("hv", PERF_SAMPLE_BRANCH_HV), + BRANCH_OPT("any", PERF_SAMPLE_BRANCH_ANY), + BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL), + BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN), + BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL), + BRANCH_OPT("abort_tx", PERF_SAMPLE_BRANCH_ABORT_TX), + BRANCH_OPT("in_tx", PERF_SAMPLE_BRANCH_IN_TX), + BRANCH_OPT("no_tx", PERF_SAMPLE_BRANCH_NO_TX), + BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_COND), + BRANCH_END +}; + +int +parse_branch_stack(const struct option *opt, const char *str, int unset) +{ +#define ONLY_PLM \ + (PERF_SAMPLE_BRANCH_USER |\ + PERF_SAMPLE_BRANCH_KERNEL |\ + PERF_SAMPLE_BRANCH_HV) + + uint64_t *mode = (uint64_t *)opt->value; + const struct branch_mode *br; + char *s, *os = NULL, *p; + int ret = -1; + + if (unset) + return 0; + + /* + * cannot set it twice, -b + --branch-filter for instance + */ + if (*mode) + return -1; + + /* str may be NULL in case no arg is passed to -b */ + if (str) { + /* because str is read-only */ + s = os = strdup(str); + if (!s) + return -1; + + for (;;) { + p = strchr(s, ','); + if (p) + *p = '\0'; + + for (br = branch_modes; br->name; br++) { + if (!strcasecmp(s, br->name)) + break; + } + if (!br->name) { + ui__warning("unknown branch filter %s," + " check man page\n", s); + goto error; + } + + *mode |= br->mode; + + if (!p) + break; + + s = p + 1; + } + } + ret = 0; + + /* default to any branch */ + if ((*mode & ~ONLY_PLM) == 0) { + *mode = PERF_SAMPLE_BRANCH_ANY; + } +error: + free(os); + return ret; +} diff --git a/tools/perf/util/parse-branch-options.h b/tools/perf/util/parse-branch-options.h new file mode 100644 index 0000000000000..b9d9470c2e82d --- /dev/null +++ b/tools/perf/util/parse-branch-options.h @@ -0,0 +1,5 @@ +#ifndef _PERF_PARSE_BRANCH_OPTIONS_H +#define _PERF_PARSE_BRANCH_OPTIONS_H 1 +struct option; +int parse_branch_stack(const struct option *opt, const char *str, int unset); +#endif /* _PERF_PARSE_BRANCH_OPTIONS_H */