From e0db3dc98d4dadf2a1c316514f7408a9f78b38bc Mon Sep 17 00:00:00 2001 From: Shuai Xue Date: Mon, 4 Nov 2024 19:27:35 +0800 Subject: [PATCH] Revert "perf hist: Add missing puts to hist__account_cycles" Revert "perf hist: Add missing puts to hist__account_cycles" This reverts commit a83fc293acd5c5050a4828eced4a71d2b2fffdd3. On x86 platform, kernel v5.10.228, perf-report command aborts due to "free(): invalid pointer" when perf-record command is run with taken branch stack sampling enabled. This regression can be reproduced with the following steps: - sudo perf record -b - sudo perf report The root cause is that bi[i].to.ms.maps does not always point to thread->maps, which is a buffer dynamically allocated by maps_new(). Instead, it may point to &machine->kmaps, while kmaps is not a pointer but a variable. The original upstream commit c1149037f65b ("perf hist: Add missing puts to hist__account_cycles") worked well because machine->kmaps had been refactored to a pointer by the previous commit 1a97cee604dc ("perf maps: Use a pointer for kmaps"). To this end, just revert commit a83fc293acd5c5050a4828eced4a71d2b2fffdd3. It is worth noting that the memory leak issue, which the reverted patch intended to fix, has been solved by commit cf96b8e45a9b ("perf session: Add missing evlist__delete when deleting a session"). The root cause is that the evlist is not being deleted on exit in perf-report, perf-script, and perf-data. Consequently, the reference count of the thread increased by thread__get() in hist_entry__init() is not decremented in hist_entry__delete(). As a result, thread->maps is not properly freed. Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Ian Rogers Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Arnaldo Carvalho de Melo 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 Cc: stable@vger.kernel.org # 5.10.228 Signed-off-by: Shuai Xue Signed-off-by: Greg Kroah-Hartman --- tools/perf/util/hist.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index c78d88138..8a793e4c9 100755 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -2624,6 +2624,8 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, /* If we have branch cycles always annotate them. */ if (bs && bs->nr && entries[0].flags.cycles) { + int i; + bi = sample__resolve_bstack(sample, al); if (bi) { struct addr_map_symbol *prev = NULL; @@ -2638,7 +2640,7 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, * Note that perf stores branches reversed from * program order! */ - for (int i = bs->nr - 1; i >= 0; i--) { + for (i = bs->nr - 1; i >= 0; i--) { addr_map_symbol__account_cycles(&bi[i].from, nonany_branch_mode ? NULL : prev, bi[i].flags.cycles); @@ -2647,12 +2649,6 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, if (total_cycles) *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); - } free(bi); } }