[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65cb75e0-4c0b-9384-1f6b-77a0053d8109@intel.com>
Date: Wed, 25 Jan 2023 09:29:31 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: Krister Johansen <kjlx@...pleofstupid.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Michael Petlan <mpetlan@...hat.com>,
David Reaver <me@...idreaver.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf/util: Symbol lookup can fail if multiple segmets
match stext
On 25/01/23 00:35, Krister Johansen wrote:
> This problem was encountered on an arm64 system with a lot of memory.
> Without kernel debug symbols installed, and with both kcore and kallsyms
> available, perf managed to get confused and returned "unknown" for all
> of the kernel symbols that it tried to look up.
>
> On this system, stext fell within the vmalloc segment. The kcore symbol
> matching code tries to find the first segment that contains stext and
> uses that to replace the segment generated from just the kallsyms
> information. In this case, however, there were two: a very large
> vmalloc segment, and the text segment. This caused perf to get confused
> because multiple overlapping segments were inserted into the RB tree
> that holds the discovered segments. However, that alone wasn't
> sufficient to cause the problem. Even when we could find the segment,
> the offsets were adjusted in such a way that the newly generated symbols
> didn't line up with the instruction addresses in the trace. The most
> obvious solution would be to consult which segment type is text from
> kcore, but this information is not exposed to users.
>
> Instead, select the smallest matching segment that contains stext
> instead of the first matching segment. This allows us to match the text
> segment instead of vmalloc, if one is contained within the other.
>
> Signed-off-by: Krister Johansen <kjlx@...pleofstupid.com>
> ---
> tools/perf/util/symbol.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index a3a165ae933a..14ac4189eaff 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1368,10 +1368,16 @@ 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;
We'd usually put a blank line here
> list_for_each_entry(new_map, &md.maps, node) {
> - if (stext >= new_map->start && stext < new_map->end) {
> + u64 new_size = new_map->end - new_map->start;
> +
> + if (!(stext >= new_map->start && stext < new_map->end))
> + continue;
> +
Really needs a comment, and please be specific e.g.
ARM64 vmalloc segment overlaps the kernel text segment, so
choosing the smaller segment will get the kernel text.
> + if (!replacement_map || new_size < replacement_size) {
> replacement_map = new_map;
> - break;
> + replacement_size = new_size;
> }
> }
> }
Powered by blists - more mailing lists