Re: [PATCH 2/2] perf/top: Fix s390 gap between kernel end and module start

From: Arnaldo Carvalho de Melo
Date: Thu Aug 08 2019 - 09:49:50 EST


Em Wed, Jul 24, 2019 at 02:27:03PM +0200, Thomas Richter escreveu:
> During execution of command 'perf top' the error message:
>
> Not enough memory for annotating '__irf_end' symbol!)
>
> is emitted from this call sequence:
> __cmd_top
> perf_top__mmap_read
> perf_top__mmap_read_idx
> perf_event__process_sample
> hist_entry_iter__add
> hist_iter__top_callback
> perf_top__record_precise_ip
> hist_entry__inc_addr_samples
> symbol__inc_addr_samples
> symbol__get_annotation
> symbol__alloc_hist

Thanks, applied.

- Arnaldo

> In this function the size of symbol __irf_end is calculated. The size
> of a symbol is the difference between its start and end address.
> When the symbol was read the first time, its start and end was set to:
> symbol__new: __irf_end 0xe954d0-0xe954d0
> which is correct and maps with /proc/kallsyms:
>
> root@s8360046:~/linux-4.15.0/tools/perf# fgrep _irf_end /proc/kallsyms
> 0000000000e954d0 t __irf_end
> root@s8360046:~/linux-4.15.0/tools/perf#
>
> In function symbol__alloc_hist() the end of symbol __irf_end is
>
> symbol__alloc_hist sym:__irf_end start:0xe954d0 end:0x3ff80045a8
>
> which is identical with the first module entry in /proc/kallsyms
>
> This results in a symbol size of __irf_req for histogram analyses of
> 70334140059072 bytes and a malloc() for this requested size fails.
>
> The root cause of this is function
> __dso__load_kallsyms()
> +-> symbols__fixup_end()
>
> Function symbols__fixup_end() enlarges the last symbol in the
> kallsyms map
> # fgrep __irf_end /proc/kallsyms
> 0000000000e954d0 t __irf_end
> #
>
> to the start address of the first module:
> # cat /proc/kallsyms | sort | egrep ' [tT] '
> ....
> 0000000000e952d0 T __security_initcall_end
> 0000000000e954d0 T __initramfs_size
> 0000000000e954d0 t __irf_end
> 000003ff800045a8 T fc_get_event_number [scsi_transport_fc]
> 000003ff800045d0 t store_fc_vport_disable [scsi_transport_fc]
> 000003ff800046a8 T scsi_is_fc_rport [scsi_transport_fc]
> 000003ff800046d0 t fc_target_setup [scsi_transport_fc]
>
> On s390 the kernel is located around memory address 0x200, 0x10000
> or 0x100000, depending on linux version. Modules however start some-
> where around 0x3ff xxxx xxxx.
>
> This is different than x86 and produces a large gap for which
> histogram allocation fails.
>
> Fix this by detecting the kernel's last symbol and do no
> adjustment for it. Introduce a weak function and handle s390 specifics.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Reported-by: Klaus Theurich <klaus.theurich@xxxxxxxxxx>
> Signed-off-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>
> Acked-by: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
> ---
> tools/perf/arch/s390/util/machine.c | 17 +++++++++++++++++
> tools/perf/util/symbol.c | 7 ++++++-
> tools/perf/util/symbol.h | 1 +
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/s390/util/machine.c b/tools/perf/arch/s390/util/machine.c
> index de26b1441a48..c8c86a0c9b79 100644
> --- a/tools/perf/arch/s390/util/machine.c
> +++ b/tools/perf/arch/s390/util/machine.c
> @@ -6,6 +6,7 @@
> #include "machine.h"
> #include "api/fs/fs.h"
> #include "debug.h"
> +#include "symbol.h"
>
> int arch__fix_module_text_start(u64 *start, u64 *size, const char *name)
> {
> @@ -33,3 +34,19 @@ int arch__fix_module_text_start(u64 *start, u64 *size, const char *name)
>
> return 0;
> }
> +
> +/* On s390 kernel text segment start is located at very low memory addresses,
> + * for example 0x10000. Modules are located at very high memory addresses,
> + * for example 0x3ff xxxx xxxx. The gap between end of kernel text segment
> + * and beginning of first module's text segment is very big.
> + * Therefore do not fill this gap and do not assign it to the kernel dso map.
> + */
> +void arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
> +{
> + if (strchr(p->name, '[') == NULL && strchr(c->name, '['))
> + /* Last kernel symbol mapped to end of page */
> + p->end = roundup(p->end, page_size);
> + else
> + p->end = c->start;
> + pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
> +}
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 5cbad55cd99d..3b49eb4e3ed9 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -91,6 +91,11 @@ static int prefix_underscores_count(const char *str)
> return tail - str;
> }
>
> +void __weak arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
> +{
> + p->end = c->start;
> +}
> +
> const char * __weak arch__normalize_symbol_name(const char *name)
> {
> return name;
> @@ -217,7 +222,7 @@ void symbols__fixup_end(struct rb_root_cached *symbols)
> curr = rb_entry(nd, struct symbol, rb_node);
>
> if (prev->end == prev->start && prev->end != curr->start)
> - prev->end = curr->start;
> + arch__symbols__fixup_end(prev, curr);
> }
>
> /* Last entry */
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 9a8fe012910a..f30ab608ea54 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -277,6 +277,7 @@ const char *arch__normalize_symbol_name(const char *name);
> #define SYMBOL_A 0
> #define SYMBOL_B 1
>
> +void arch__symbols__fixup_end(struct symbol *p, struct symbol *c);
> int arch__compare_symbol_names(const char *namea, const char *nameb);
> int arch__compare_symbol_names_n(const char *namea, const char *nameb,
> unsigned int n);
> --
> 2.21.0

--

- Arnaldo