From: James Clark <james.clark@arm.com>
To: linux-perf-users@vger.kernel.org, atrajeev@linux.vnet.ibm.com,
irogers@google.com
Cc: James Clark <james.clark@arm.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
"Liang, Kan" <kan.liang@linux.intel.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 3/4] perf symbols: Update kcore map before merging in remaining symbols
Date: Tue, 7 May 2024 15:12:07 +0100 [thread overview]
Message-ID: <20240507141210.195939-4-james.clark@arm.com> (raw)
In-Reply-To: <20240507141210.195939-1-james.clark@arm.com>
When loading kcore, the main vmlinux map is updated in the same loop
that merges the remaining maps. If a map that overlaps is merged in
before kcore, the list can become unsortable when the main map addresses
are updated. This will later trigger the check_invariants() assert:
$ perf record
$ perf report
util/maps.c:96: check_invariants: Assertion `map__end(prev) <=
map__start(map) || map__start(prev) == map__start(map)' failed.
Aborted
Fix it by moving the main map update prior to the loop so that
maps__merge_in() can split it if necessary.
Fixes: 659ad3492b91 ("perf maps: Switch from rbtree to lazily sorted array for addresses")
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/util/symbol.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2d95f22d713d..e98dfe766da3 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1289,7 +1289,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 *replacement_map = NULL;
+ struct map *map_ref, *replacement_map = NULL;
struct machine *machine;
bool is_64_bit;
int err, fd;
@@ -1367,6 +1367,24 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
if (!replacement_map)
replacement_map = list_entry(md.maps.next, struct map_list_node, node)->map;
+ /*
+ * Update addresses of vmlinux map. Re-insert it to ensure maps are
+ * correctly ordered. Do this before using maps__merge_in() for the
+ * remaining maps so vmlinux gets split if necessary.
+ */
+ map_ref = map__get(map);
+ maps__remove(kmaps, map_ref);
+
+ map__set_start(map_ref, map__start(replacement_map));
+ map__set_end(map_ref, map__end(replacement_map));
+ map__set_pgoff(map_ref, map__pgoff(replacement_map));
+ map__set_mapping_type(map_ref, map__mapping_type(replacement_map));
+
+ err = maps__insert(kmaps, map_ref);
+ map__put(map_ref);
+ if (err)
+ goto out_err;
+
/* Add new maps */
while (!list_empty(&md.maps)) {
struct map_list_node *new_node = list_entry(md.maps.next, struct map_list_node, node);
@@ -1374,24 +1392,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
list_del_init(&new_node->node);
- if (RC_CHK_EQUAL(new_map, replacement_map)) {
- struct map *map_ref;
-
- /* Ensure maps are correctly ordered */
- map_ref = map__get(map);
- maps__remove(kmaps, map_ref);
-
- map__set_start(map_ref, map__start(new_map));
- map__set_end(map_ref, map__end(new_map));
- map__set_pgoff(map_ref, map__pgoff(new_map));
- map__set_mapping_type(map_ref, map__mapping_type(new_map));
-
- err = maps__insert(kmaps, map_ref);
- map__put(map_ref);
- map__put(new_map);
- if (err)
- goto out_err;
- } else {
+ /* skip if replacement_map, already inserted above */
+ if (!RC_CHK_EQUAL(new_map, replacement_map)) {
/*
* Merge kcore map into existing maps,
* and ensure that current maps (eBPF)
--
2.34.1
next prev parent reply other threads:[~2024-05-07 14:13 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 14:12 [PATCH 0/4] perf maps/symbols: Various assert fixes James Clark
2024-05-07 14:12 ` [PATCH 1/4] perf symbols: Remove map from list before updating addresses James Clark
2024-05-07 14:12 ` [PATCH 2/4] perf maps: Re-use __maps__free_maps_by_name() James Clark
2024-05-08 4:06 ` Namhyung Kim
2024-05-08 22:06 ` Ian Rogers
2024-05-07 14:12 ` James Clark [this message]
2024-05-08 4:10 ` [PATCH 3/4] perf symbols: Update kcore map before merging in remaining symbols Namhyung Kim
2024-05-08 9:14 ` James Clark
2024-05-08 14:19 ` Leo Yan
2024-05-07 14:12 ` [PATCH 4/4] perf symbols: Fix ownership of string in dso__load_vmlinux() James Clark
2024-05-08 22:14 ` Ian Rogers
2024-05-07 15:11 ` [PATCH 0/4] perf maps/symbols: Various assert fixes Arnaldo Carvalho de Melo
2024-05-07 15:12 ` Arnaldo Carvalho de Melo
2024-05-08 7:52 ` James Clark
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=20240507141210.195939-4-james.clark@arm.com \
--to=james.clark@arm.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=atrajeev@linux.vnet.ibm.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/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
all inboxes | Powered by JetHome®