From 56e144fe98260a0f8a17326993ceb576ef859ed5 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 24 Oct 2023 15:23:14 -0700 Subject: perf mem_info: Add and use map_symbol__exit and addr_map_symbol__exit Fix leak where mem_info__put wouldn't release the maps/map as used by perf mem. Add exit functions and use elsewhere that the maps and map are released. Signed-off-by: Ian Rogers Cc: K Prateek Nayak Cc: Ravi Bangoria Cc: Sandipan Das Cc: Anshuman Khandual Cc: German Gomez Cc: James Clark Cc: Nick Terrell Cc: Sean Christopherson Cc: Changbin Du Cc: liuwenyu Cc: Yang Jihong Cc: Masami Hiramatsu Cc: Miguel Ojeda Cc: Song Liu Cc: Leo Yan Cc: Kajol Jain Cc: Andi Kleen Cc: Kan Liang Cc: Athira Rajeev Cc: Yanteng Si Cc: Liam Howlett Cc: Paolo Bonzini Link: https://lore.kernel.org/r/20231024222353.3024098-12-irogers@google.com Signed-off-by: Namhyung Kim --- tools/perf/util/Build | 1 + tools/perf/util/callchain.c | 27 +++++++++------------------ tools/perf/util/hist.c | 28 ++++++++++++---------------- tools/perf/util/machine.c | 6 ++---- tools/perf/util/map_symbol.c | 15 +++++++++++++++ tools/perf/util/map_symbol.h | 4 ++++ tools/perf/util/symbol.c | 5 ++++- 7 files changed, 47 insertions(+), 39 deletions(-) create mode 100644 tools/perf/util/map_symbol.c (limited to 'tools/perf') diff --git a/tools/perf/util/Build b/tools/perf/util/Build index 0ea5a9d368d4..96058f949ec9 100644 --- a/tools/perf/util/Build +++ b/tools/perf/util/Build @@ -49,6 +49,7 @@ perf-y += dso.o perf-y += dsos.o perf-y += symbol.o perf-y += symbol_fprintf.o +perf-y += map_symbol.o perf-y += color.o perf-y += color_config.o perf-y += metricgroup.o diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index 5349c6a21849..99cce43ba152 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -659,8 +659,7 @@ add_child(struct callchain_node *parent, list_for_each_entry_safe(call, tmp, &new->val, list) { list_del_init(&call->list); - map__zput(call->ms.map); - maps__zput(call->ms.maps); + map_symbol__exit(&call->ms); zfree(&call->brtype_stat); free(call); } @@ -1040,10 +1039,8 @@ merge_chain_branch(struct callchain_cursor *cursor, }; callchain_cursor_append(cursor, list->ip, &ms, false, NULL, 0, 0, 0, list->srcline); list_del_init(&list->list); - map__zput(ms.map); - maps__zput(ms.maps); - map__zput(list->ms.map); - maps__zput(list->ms.maps); + map_symbol__exit(&ms); + map_symbol__exit(&list->ms); zfree(&list->brtype_stat); free(list); } @@ -1096,8 +1093,7 @@ int callchain_cursor_append(struct callchain_cursor *cursor, } node->ip = ip; - maps__zput(node->ms.maps); - map__zput(node->ms.map); + map_symbol__exit(&node->ms); node->ms = *ms; node->ms.maps = maps__get(ms->maps); node->ms.map = map__get(ms->map); @@ -1496,16 +1492,14 @@ static void free_callchain_node(struct callchain_node *node) list_for_each_entry_safe(list, tmp, &node->parent_val, list) { list_del_init(&list->list); - map__zput(list->ms.map); - maps__zput(list->ms.maps); + map_symbol__exit(&list->ms); zfree(&list->brtype_stat); free(list); } list_for_each_entry_safe(list, tmp, &node->val, list) { list_del_init(&list->list); - map__zput(list->ms.map); - maps__zput(list->ms.maps); + map_symbol__exit(&list->ms); zfree(&list->brtype_stat); free(list); } @@ -1591,8 +1585,7 @@ int callchain_node__make_parent_list(struct callchain_node *node) out: list_for_each_entry_safe(chain, new, &head, list) { list_del_init(&chain->list); - map__zput(chain->ms.map); - maps__zput(chain->ms.maps); + map_symbol__exit(&chain->ms); zfree(&chain->brtype_stat); free(chain); } @@ -1676,10 +1669,8 @@ void callchain_cursor_reset(struct callchain_cursor *cursor) cursor->nr = 0; cursor->last = &cursor->first; - for (node = cursor->first; node != NULL; node = node->next) { - map__zput(node->ms.map); - maps__zput(node->ms.maps); - } + for (node = cursor->first; node != NULL; node = node->next) + map_symbol__exit(&node->ms); } void callchain_param_setup(u64 sample_type, const char *arch) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 0aa7e231172a..0888b7163b7c 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -515,17 +515,16 @@ err_rawdata: err_infos: if (he->branch_info) { - map__put(he->branch_info->from.ms.map); - map__put(he->branch_info->to.ms.map); + map_symbol__exit(&he->branch_info->from.ms); + map_symbol__exit(&he->branch_info->to.ms); zfree(&he->branch_info); } if (he->mem_info) { - map__put(he->mem_info->iaddr.ms.map); - map__put(he->mem_info->daddr.ms.map); + map_symbol__exit(&he->mem_info->iaddr.ms); + map_symbol__exit(&he->mem_info->daddr.ms); } err: - maps__zput(he->ms.maps); - map__zput(he->ms.map); + map_symbol__exit(&he->ms); zfree(&he->stat_acc); return -ENOMEM; } @@ -1317,20 +1316,19 @@ void hist_entry__delete(struct hist_entry *he) struct hist_entry_ops *ops = he->ops; thread__zput(he->thread); - maps__zput(he->ms.maps); - map__zput(he->ms.map); + map_symbol__exit(&he->ms); if (he->branch_info) { - map__zput(he->branch_info->from.ms.map); - map__zput(he->branch_info->to.ms.map); + map_symbol__exit(&he->branch_info->from.ms); + map_symbol__exit(&he->branch_info->to.ms); zfree_srcline(&he->branch_info->srcline_from); zfree_srcline(&he->branch_info->srcline_to); zfree(&he->branch_info); } if (he->mem_info) { - map__zput(he->mem_info->iaddr.ms.map); - map__zput(he->mem_info->daddr.ms.map); + map_symbol__exit(&he->mem_info->iaddr.ms); + map_symbol__exit(&he->mem_info->daddr.ms); mem_info__zput(he->mem_info); } @@ -2700,10 +2698,8 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, *total_cycles += bi[i].flags.cycles; } for (unsigned int i = 0; i < bs->nr; i++) { - map__put(bi[i].to.ms.map); - maps__put(bi[i].to.ms.maps); - map__put(bi[i].from.ms.map); - maps__put(bi[i].from.ms.maps); + map_symbol__exit(&bi[i].to.ms); + map_symbol__exit(&bi[i].from.ms); } free(bi); } diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 31fdbb0f27af..90c750150b19 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -2389,8 +2389,7 @@ static int add_callchain_ip(struct thread *thread, iter_cycles, branch_from, srcline); out: addr_location__exit(&al); - maps__put(ms.maps); - map__put(ms.map); + map_symbol__exit(&ms); return err; } @@ -3116,8 +3115,7 @@ static int append_inlines(struct callchain_cursor *cursor, struct map_symbol *ms if (ret != 0) return ret; } - map__put(ilist_ms.map); - maps__put(ilist_ms.maps); + map_symbol__exit(&ilist_ms); return ret; } diff --git a/tools/perf/util/map_symbol.c b/tools/perf/util/map_symbol.c new file mode 100644 index 000000000000..bef5079f2403 --- /dev/null +++ b/tools/perf/util/map_symbol.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "map_symbol.h" +#include "maps.h" +#include "map.h" + +void map_symbol__exit(struct map_symbol *ms) +{ + maps__zput(ms->maps); + map__zput(ms->map); +} + +void addr_map_symbol__exit(struct addr_map_symbol *ams) +{ + map_symbol__exit(&ams->ms); +} diff --git a/tools/perf/util/map_symbol.h b/tools/perf/util/map_symbol.h index e08817b0c30f..72d5ed938ed6 100644 --- a/tools/perf/util/map_symbol.h +++ b/tools/perf/util/map_symbol.h @@ -22,4 +22,8 @@ struct addr_map_symbol { u64 phys_addr; u64 data_page_size; }; + +void map_symbol__exit(struct map_symbol *ms); +void addr_map_symbol__exit(struct addr_map_symbol *ams); + #endif // __PERF_MAP_SYMBOL diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 822f4dcebfe6..82cc74b9358e 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -2791,8 +2791,11 @@ struct mem_info *mem_info__get(struct mem_info *mi) void mem_info__put(struct mem_info *mi) { - if (mi && refcount_dec_and_test(&mi->refcnt)) + if (mi && refcount_dec_and_test(&mi->refcnt)) { + addr_map_symbol__exit(&mi->iaddr); + addr_map_symbol__exit(&mi->daddr); free(mi); + } } struct mem_info *mem_info__new(void) -- cgit v1.2.3