From 83720209961f5cfed17214bc78978a02e3414a41 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Mon, 20 Mar 2023 14:22:32 -0700 Subject: perf map: Move map list node into symbol Using a perf map as a list node is only done in symbol. Move the list_node struct into symbol as a single pointer to the map. This makes reference count behavior more obvious and easy to check. Committer notes: Some changes to reduce the number of lines touched by keeping, for instance, the 'new_map' variable and setting it to new_node->map, so that we keep more of the project history in place and keep as much as possible the value of the 'git blame' tool. Also use map__zput() when putting a struct members, so that when we free the container struct we can get use-after-free errors as NULL pointer derefs sometimes. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Alexey Bayduraev Cc: Andi Kleen Cc: Andrew Morton Cc: Andy Shevchenko Cc: Darren Hart Cc: Davidlohr Bueso Cc: Dmitriy Vyukov Cc: Eric Dumazet Cc: German Gomez Cc: Hao Luo Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Leo Yan Cc: Madhavan Srinivasan Cc: Mark Rutland Cc: Masami Hiramatsu Cc: Miaoqian Lin Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Shunsuke Nakamura Cc: Song Liu Cc: Stephane Eranian Cc: Stephen Brennan Cc: Steven Rostedt (VMware) Cc: Thomas Gleixner Cc: Thomas Richter Cc: Yury Norov Link: https://lore.kernel.org/r/20230320212248.1175731-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/map.h | 5 +--- tools/perf/util/symbol.c | 73 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 54 insertions(+), 24 deletions(-) (limited to 'tools/perf/util') diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index 3dcfe06db6b3..2879cae05ee0 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -16,10 +16,7 @@ struct maps; struct machine; struct map { - union { - struct rb_node rb_node; - struct list_head node; - }; + struct rb_node rb_node; u64 start; u64 end; bool erange_warned:1; diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index a458aa8b87bb..8719074b7f21 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -48,6 +48,11 @@ static bool symbol__is_idle(const char *name); int vmlinux_path__nr_entries; char **vmlinux_path; +struct map_list_node { + struct list_head node; + struct map *map; +}; + struct symbol_conf symbol_conf = { .nanosecs = false, .use_modules = true, @@ -85,6 +90,11 @@ static enum dso_binary_type binary_type_symtab[] = { #define DSO_BINARY_TYPE__SYMTAB_CNT ARRAY_SIZE(binary_type_symtab) +static struct map_list_node *map_list_node__new(void) +{ + return malloc(sizeof(struct map_list_node)); +} + static bool symbol_type__filter(char symbol_type) { symbol_type = toupper(symbol_type); @@ -1219,16 +1229,21 @@ struct kcore_mapfn_data { static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data) { struct kcore_mapfn_data *md = data; - struct map *map; + struct map_list_node *list_node = map_list_node__new(); - map = map__new2(start, md->dso); - if (map == NULL) + if (!list_node) return -ENOMEM; - map->end = map->start + len; - map->pgoff = pgoff; + list_node->map = map__new2(start, md->dso); + if (!list_node->map) { + free(list_node); + return -ENOMEM; + } - list_add(&map->node, &md->maps); + list_node->map->end = list_node->map->start + len; + list_node->map->pgoff = pgoff; + + list_add(&list_node->node, &md->maps); return 0; } @@ -1264,12 +1279,18 @@ int maps__merge_in(struct maps *kmaps, struct map *new_map) * |new.............| -> |new..| |new..| * |old....| -> |old....| */ - struct map *m = map__clone(new_map); + struct map_list_node *m = map_list_node__new(); if (!m) return -ENOMEM; - m->end = old_map->start; + m->map = map__clone(new_map); + if (!m->map) { + free(m); + return -ENOMEM; + } + + m->map->end = old_map->start; list_add_tail(&m->node, &merged); new_map->pgoff += old_map->end - new_map->start; new_map->start = old_map->end; @@ -1299,10 +1320,13 @@ int maps__merge_in(struct maps *kmaps, struct map *new_map) } while (!list_empty(&merged)) { - old_map = list_entry(merged.next, struct map, node); - list_del_init(&old_map->node); - maps__insert(kmaps, old_map); - map__put(old_map); + struct map_list_node *old_node; + + old_node = list_entry(merged.next, struct map_list_node, node); + list_del_init(&old_node->node); + maps__insert(kmaps, old_node->map); + map__put(old_node->map); + free(old_node); } if (new_map) { @@ -1317,7 +1341,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, { struct maps *kmaps = map__kmaps(map); struct kcore_mapfn_data md; - struct map *old_map, *new_map, *replacement_map = NULL, *next; + struct map *old_map, *replacement_map = NULL, *next; struct machine *machine; bool is_64_bit; int err, fd; @@ -1378,8 +1402,10 @@ static int dso__load_kcore(struct dso *dso, struct map *map, /* Find the kernel map using the '_stext' symbol */ if (!kallsyms__get_function_start(kallsyms_filename, "_stext", &stext)) { u64 replacement_size = 0; + struct map_list_node *new_node; - list_for_each_entry(new_map, &md.maps, node) { + list_for_each_entry(new_node, &md.maps, node) { + struct map *new_map = new_node->map; u64 new_size = new_map->end - new_map->start; if (!(stext >= new_map->start && stext < new_map->end)) @@ -1399,12 +1425,15 @@ static int dso__load_kcore(struct dso *dso, struct map *map, } if (!replacement_map) - replacement_map = list_entry(md.maps.next, struct map, node); + replacement_map = list_entry(md.maps.next, struct map_list_node, node)->map; /* Add new maps */ while (!list_empty(&md.maps)) { - new_map = list_entry(md.maps.next, struct map, node); - list_del_init(&new_map->node); + struct map_list_node *new_node = list_entry(md.maps.next, struct map_list_node, node); + struct map *new_map = new_node->map; + + list_del_init(&new_node->node); + if (new_map == replacement_map) { map->start = new_map->start; map->end = new_map->end; @@ -1426,6 +1455,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, if (maps__merge_in(kmaps, new_map)) goto out_err; } + free(new_node); } if (machine__is(machine, "x86_64")) { @@ -1462,9 +1492,12 @@ 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_init(&map->node); - map__put(map); + struct map_list_node *list_node; + + list_node = list_entry(md.maps.next, struct map_list_node, node); + list_del_init(&list_node->node); + map__zput(list_node->map); + free(list_node); } close(fd); return -EINVAL; -- cgit v1.2.3