From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Thomas Richter <tmricht@linux.vnet.ibm.com>,
Hendrik Brueckner <brueckner@linux.vnet.ibm.com>,
Zvonko Kosic <zvonko.kosic@de.ibm.com>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 17/19] perf record: Fix wrong size in perf_record_mmap for last kernel module
Date: Mon, 14 Aug 2017 13:27:39 -0300 [thread overview]
Message-ID: <20170814162741.6101-18-acme@kernel.org> (raw)
In-Reply-To: <20170814162741.6101-1-acme@kernel.org>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
During work on perf report for s390 I ran into the following issue:
0 0x318 [0x78]: PERF_RECORD_MMAP -1/0:
[0x3ff804d6990(0xfffffc007fb2966f) @ 0]:
x /lib/modules/4.12.0perf1+/kernel/drivers/s390/net/qeth_l2.ko
This is a PERF_RECORD_MMAP entry of the perf.data file with an invalid
module size for qeth_l2.ko (the s390 ethernet device driver).
Even a mainframe does not have 0xfffffc007fb2966f bytes of main memory.
It turned out that this wrong size is created by the perf record
command. What happens is this function call sequence from
__cmd_record():
perf_session__new():
perf_session__create_kernel_maps():
machine__create_kernel_maps():
machine__create_modules(): Creates map for all loaded kernel modules.
modules__parse(): Reads /proc/modules and extracts module name and
load address (1st and last column)
machine__create_module(): Called for every module found in /proc/modules.
Creates a new map for every module found and enters
module name and start address into the map. Since the
module end address is unknown it is set to zero.
This ends up with a kernel module map list sorted by module start
addresses. All module end addresses are zero.
Last machine__create_kernel_maps() calls function map_groups__fixup_end().
This function iterates through the maps and assigns each map entry's
end address the successor map entry start address. The last entry of the
map group has no successor, so ~0 is used as end to consume the remaining
memory.
Later __cmd_record calls function record__synthesize() which in turn calls
perf_event__synthesize_kernel_mmap() and perf_event__synthesize_modules()
to create PERF_REPORT_MMAP entries into the perf.data file.
On s390 this results in the last module qeth_l2.ko
(which has highest start address, see module table:
[root@s8360047 perf]# cat /proc/modules
qeth_l2 86016 1 - Live 0x000003ff804d6000
qeth 266240 1 qeth_l2, Live 0x000003ff80296000
ccwgroup 24576 1 qeth, Live 0x000003ff80218000
vmur 36864 0 - Live 0x000003ff80182000
qdio 143360 2 qeth_l2,qeth, Live 0x000003ff80002000
[root@s8360047 perf]# )
to be the last entry and its map has an end address of ~0.
When the PERF_RECORD_MMAP entry is created for kernel module qeth_l2.ko
its start address and length is written. The length is calculated in line:
event->mmap.len = pos->end - pos->start;
and results in 0xffffffffffffffff - 0x3ff804d6990(*) = 0xfffffc007fb2966f
(*) On s390 the module start address is actually determined by a __weak function
named arch__fix_module_text_start() in machine__create_module().
I think this improvable. We can use the module size (2nd column of /proc/modules)
to get each loaded kernel module size and calculate its end address.
Only for map entries which do not have a valid end address (end is still zero)
we can use the heuristic we have now, that is use successor start address or ~0.
Signed-off-by: Thomas-Mich Richter <tmricht@linux.vnet.ibm.com>
Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Thomas-Mich Richter <tmricht@linux.vnet.ibm.com>
Cc: Zvonko Kosic <zvonko.kosic@de.ibm.com>
LPU-Reference: 20170803134902.47207-2-tmricht@linux.vnet.ibm.com
Link: http://lkml.kernel.org/n/tip-nmoqij5b5vxx7rq2ckwu8iaj@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/machine.c | 4 +++-
tools/perf/util/symbol-elf.c | 2 +-
tools/perf/util/symbol.c | 21 ++++++++++++++-------
tools/perf/util/symbol.h | 2 +-
4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index d4df353051af..5c8eacaca4f4 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1138,7 +1138,8 @@ int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
return 0;
}
-static int machine__create_module(void *arg, const char *name, u64 start)
+static int machine__create_module(void *arg, const char *name, u64 start,
+ u64 size)
{
struct machine *machine = arg;
struct map *map;
@@ -1149,6 +1150,7 @@ static int machine__create_module(void *arg, const char *name, u64 start)
map = machine__findnew_module_map(machine, start, name);
if (map == NULL)
return -1;
+ map->end = start + size;
dso__kernel_module_get_build_id(map->dso, machine->root_dir);
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 7cf18f14e152..98deabb9b47e 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1442,7 +1442,7 @@ static int kcore_copy__parse_kallsyms(struct kcore_copy_info *kci,
static int kcore_copy__process_modules(void *arg,
const char *name __maybe_unused,
- u64 start)
+ u64 start, u64 size __maybe_unused)
{
struct kcore_copy_info *kci = arg;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 971b990557b4..5909ee4c7ade 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -233,7 +233,8 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
goto out_unlock;
for (next = map__next(curr); next; next = map__next(curr)) {
- curr->end = next->start;
+ if (!curr->end)
+ curr->end = next->start;
curr = next;
}
@@ -241,7 +242,8 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
* We still haven't the actual symbols, so guess the
* last map final address.
*/
- curr->end = ~0ULL;
+ if (!curr->end)
+ curr->end = ~0ULL;
out_unlock:
pthread_rwlock_unlock(&maps->lock);
@@ -552,7 +554,7 @@ void dso__sort_by_name(struct dso *dso, enum map_type type)
int modules__parse(const char *filename, void *arg,
int (*process_module)(void *arg, const char *name,
- u64 start))
+ u64 start, u64 size))
{
char *line = NULL;
size_t n;
@@ -565,8 +567,8 @@ int modules__parse(const char *filename, void *arg,
while (1) {
char name[PATH_MAX];
- u64 start;
- char *sep;
+ u64 start, size;
+ char *sep, *endptr;
ssize_t line_len;
line_len = getline(&line, &n, file);
@@ -598,7 +600,11 @@ int modules__parse(const char *filename, void *arg,
scnprintf(name, sizeof(name), "[%s]", line);
- err = process_module(arg, name, start);
+ size = strtoul(sep + 1, &endptr, 0);
+ if (*endptr != ' ' && *endptr != '\t')
+ continue;
+
+ err = process_module(arg, name, start, size);
if (err)
break;
}
@@ -945,7 +951,8 @@ static struct module_info *find_module(const char *name,
return NULL;
}
-static int __read_proc_modules(void *arg, const char *name, u64 start)
+static int __read_proc_modules(void *arg, const char *name, u64 start,
+ u64 size __maybe_unused)
{
struct rb_root *modules = arg;
struct module_info *mi;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f0b08810d7fa..b221671070e2 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -273,7 +273,7 @@ int filename__read_build_id(const char *filename, void *bf, size_t size);
int sysfs__read_build_id(const char *filename, void *bf, size_t size);
int modules__parse(const char *filename, void *arg,
int (*process_module)(void *arg, const char *name,
- u64 start));
+ u64 start, u64 size));
int filename__read_debuglink(const char *filename, char *debuglink,
size_t size);
--
2.13.4
next prev parent reply other threads:[~2017-08-14 16:27 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-14 16:27 [GIT PULL 00/19] perf/core improvements and fixes Arnaldo Carvalho de Melo
2017-08-14 16:27 ` Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 01/19] perf scripting python: Add ppc64le to audit uname list Arnaldo Carvalho de Melo
2017-08-14 16:27 ` Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 02/19] perf vendor events powerpc: remove suffix in mapfile Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 03/19] perf vendor events powerpc: Update POWER9 events Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 04/19] perf stat: Fix saved values rbtree lookup Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 05/19] perf tools: Add missing newline to expr parser error messages Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 06/19] perf test: Make 'list' subcommand match main 'perf test' numbering/matching Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 07/19] perf test: Add 'struct test *' to the test functions Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 08/19] perf test: Add infrastructure to run shell based tests Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 09/19] perf test: Make 'list' use same filtering code as main 'perf test' Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 10/19] perf test shell: Add 'probe_vfs_getname' shell test Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 11/19] perf test shell: Install shell tests Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 12/19] perf test shell: Move vfs_getname probe function to lib Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 13/19] perf test shell: Add test using probe:vfs_getname and verifying results Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 14/19] perf test shell: Add test using vfs_getname + 'perf trace' Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 15/19] perf util: Take elf_name as const string in dso__demangle_sym Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 16/19] perf srcline: Do not consider empty files as valid srclines Arnaldo Carvalho de Melo
2017-08-14 16:27 ` Arnaldo Carvalho de Melo [this message]
2017-08-14 16:27 ` [PATCH 18/19] perf report: Fix module symbol adjustment for s390x Arnaldo Carvalho de Melo
2017-08-14 16:27 ` [PATCH 19/19] perf test shell: Add uprobes + backtrace ping test Arnaldo Carvalho de Melo
2017-08-14 17:39 ` [GIT PULL 00/19] perf/core improvements and fixes Ingo Molnar
2017-08-14 17:39 ` Ingo Molnar
2017-08-14 17:52 ` Arnaldo Carvalho de Melo
2017-08-14 17:52 ` Arnaldo Carvalho de Melo
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=20170814162741.6101-18-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=brueckner@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=tmricht@linux.vnet.ibm.com \
--cc=zvonko.kosic@de.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.