Linux Perf Users
 help / color / mirror / Atom feed
From: Shuai Xue <xueshuai@linux.alibaba.com>
To: stable@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, acme@kernel.org,
	gregkh@linuxfoundation.org
Cc: adrian.hunter@intel.com, alexander.shishkin@linux.intel.com,
	irogers@google.com, mark.rutland@arm.com, namhyung@kernel.org,
	peterz@infradead.org, acme@redhat.com, kprateek.nayak@amd.com,
	ravi.bangoria@amd.com, sandipan.das@amd.com,
	anshuman.khandual@arm.com, german.gomez@arm.com,
	james.clark@arm.com, terrelln@fb.com, seanjc@google.com,
	changbin.du@huawei.com, liuwenyu7@huawei.com,
	yangjihong1@huawei.com, mhiramat@kernel.org, ojeda@kernel.org,
	song@kernel.org, leo.yan@linaro.org, kjain@linux.ibm.com,
	ak@linux.intel.com, kan.liang@linux.intel.com,
	atrajeev@linux.vnet.ibm.com, siyanteng@loongson.cn,
	liam.howlett@oracle.com, pbonzini@redhat.com, jolsa@kernel.org
Subject: [PATCH 5.10.y 1/2] Revert "perf hist: Add missing puts to hist__account_cycles"
Date: Mon,  4 Nov 2024 19:27:35 +0800	[thread overview]
Message-ID: <20241104112736.28554-2-xueshuai@linux.alibaba.com> (raw)
In-Reply-To: <20241104112736.28554-1-xueshuai@linux.alibaba.com>

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 <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Sandipan Das <sandipan.das@amd.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: German Gomez <german.gomez@arm.com>
Cc: James Clark <james.clark@arm.com>
Cc: Nick Terrell <terrelln@fb.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: liuwenyu <liuwenyu7@huawei.com>
Cc: Yang Jihong <yangjihong1@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Song Liu <song@kernel.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Yanteng Si <siyanteng@loongson.cn>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org # 5.10.228
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 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 c78d8813811c..8a793e4c9400 100644
--- 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);
 		}
 	}
-- 
2.39.3


  reply	other threads:[~2024-11-04 11:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-04 11:27 [PATCH 5.10.y 0/2] Fixed perf abort when taken branch stack sampling enabled Shuai Xue
2024-11-04 11:27 ` Shuai Xue [this message]
2024-11-04 11:27 ` [PATCH 5.10.y 2/2] perf session: Add missing evlist__delete when deleting a session Shuai Xue
2024-11-10  5:12 ` [PATCH 5.10.y 0/2] Fixed perf abort when taken branch stack sampling enabled Greg KH
2024-11-10 10:26   ` Shuai Xue

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241104112736.28554-2-xueshuai@linux.alibaba.com \
    --to=xueshuai@linux.alibaba.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=anshuman.khandual@arm.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=changbin.du@huawei.com \
    --cc=german.gomez@arm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=irogers@google.com \
    --cc=james.clark@arm.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kjain@linux.ibm.com \
    --cc=kprateek.nayak@amd.com \
    --cc=leo.yan@linaro.org \
    --cc=liam.howlett@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=liuwenyu7@huawei.com \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@amd.com \
    --cc=sandipan.das@amd.com \
    --cc=seanjc@google.com \
    --cc=siyanteng@loongson.cn \
    --cc=song@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=terrelln@fb.com \
    --cc=yangjihong1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox