[SRU][Bionic][Disco][PATCH 0/2] Fix for perf top problem on s390x (LP: #1828166)

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][Disco][PATCH 0/2] Fix for perf top problem on s390x (LP: #1828166)

Kleber Souza
BugLink: https://bugs.launchpad.net/bugs/1828166

[Impact]

* The perf top tool hangs and shows error messages, like 'Not enough
memory for annotating'

[Fix]

The following fixes have been developed by IBM and accepted upstream:

* b9c0a64901d5bdec6eafd38d1dc8fa0e2974fccb b9c0a64 "perf annotate: Fix
s390 gap between kernel end and module start"

* 12a6d2940b5f02b4b9f71ce098e3bb02bc24a9ea 12a6d29 "perf record: Fix
module size on s390"

Disco needs also as prereq:

* 6738028dd57df064b969d8392c943ef3b3ae705d 6738028 perf record: Fix i
s390 missing module symbol and warning for non-root users

These fixes are either cherry picked or required small context
adjustments.

[Test Case]

* start a benchmark (mem_alloc, but it doesn't really matter what)

* execute perf top in a second terminal

* the output of perf top is correct

* now stop the benchmark

* and perf top shows an error message, like "Not enough memory for
annotating '__irf_end' symbol!)"

* and perf top can't be exited anymore

[Regression Potential]

* The regression potential can be considered as medium since this
happens only while using the perf top tool and just 3 files are
changed, and one of them is arch/s390/util/machine.c. But symbol
and machine header in /tools/perf/util modified and several
loc added.

* Smoke tested 'perf top' for regressions on a amd64 VM, no issues
found.

[Additional Info]

These patches have already been committed to Eoan (LP: #1841110).

Thomas Richter (2):
  perf record: Fix module size on s390
  perf annotate: Fix s390 gap between kernel end and module start

 tools/perf/arch/s390/util/machine.c | 31 ++++++++++++++++++++++++++++-
 tools/perf/util/machine.c           |  3 ++-
 tools/perf/util/machine.h           |  2 +-
 tools/perf/util/symbol.c            |  7 ++++++-
 tools/perf/util/symbol.h            |  1 +
 5 files changed, 40 insertions(+), 4 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 1/2] perf record: Fix module size on s390

Kleber Souza
From: Thomas Richter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1828166

On s390 the modules loaded in memory have the text segment located after
the GOT and Relocation table. This can be seen with this output:

  [root@m35lp76 perf]# fgrep qeth /proc/modules
  qeth 151552 1 qeth_l2, Live 0x000003ff800b2000
  ...
  [root@m35lp76 perf]# cat /sys/module/qeth/sections/.text
  0x000003ff800b3990
  [root@m35lp76 perf]#

There is an offset of 0x1990 bytes. The size of the qeth module is
151552 bytes (0x25000 in hex).

The location of the GOT/relocation table at the beginning of a module is
unique to s390.

commit 203d8a4aa6ed ("perf s390: Fix 'start' address of module's map")
adjusts the start address of a module in the map structures, but does
not adjust the size of the modules. This leads to overlapping of module
maps as this example shows:

[root@m35lp76 perf] # ./perf report -D
     0 0 0xfb0 [0xa0]: PERF_RECORD_MMAP -1/0: [0x3ff800b3990(0x25000)
          @ 0]:  x /lib/modules/.../qeth.ko.xz
     0 0 0x1050 [0xb0]: PERF_RECORD_MMAP -1/0: [0x3ff800d85a0(0x8000)
          @ 0]:  x /lib/modules/.../ip6_tables.ko.xz

The module qeth.ko has an adjusted start address modified to b3990, but
its size is unchanged and the module ends at 0x3ff800d8990.  This end
address overlaps with the next modules start address of 0x3ff800d85a0.

When the size of the leading GOT/Relocation table stored in the
beginning of the text segment (0x1990 bytes) is subtracted from module
qeth end address, there are no overlaps anymore:

   0x3ff800d8990 - 0x1990 = 0x0x3ff800d7000

which is the same as

   0x3ff800b2000 + 0x25000 = 0x0x3ff800d7000.

To fix this issue, also adjust the modules size in function
arch__fix_module_text_start(). Add another function parameter named size
and reduce the size of the module when the text segment start address is
changed.

Output after:
     0 0 0xfb0 [0xa0]: PERF_RECORD_MMAP -1/0: [0x3ff800b3990(0x23670)
          @ 0]:  x /lib/modules/.../qeth.ko.xz
     0 0 0x1050 [0xb0]: PERF_RECORD_MMAP -1/0: [0x3ff800d85a0(0x7a60)
          @ 0]:  x /lib/modules/.../ip6_tables.ko.xz

Reported-by: Stefan Liebler <[hidden email]>
Signed-off-by: Thomas Richter <[hidden email]>
Acked-by: Heiko Carstens <[hidden email]>
Cc: Hendrik Brueckner <[hidden email]>
Cc: Vasily Gorbik <[hidden email]>
Cc: [hidden email]
Fixes: 203d8a4aa6ed ("perf s390: Fix 'start' address of module's map")
Link: http://lkml.kernel.org/r/20190724122703.3996-1-tmricht@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(backported from commit 12a6d2940b5f02b4b9f71ce098e3bb02bc24a9ea)
[ kleber: context adjustment. ]
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 tools/perf/arch/s390/util/machine.c | 14 +++++++++++++-
 tools/perf/util/machine.c           |  3 ++-
 tools/perf/util/machine.h           |  2 +-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/s390/util/machine.c b/tools/perf/arch/s390/util/machine.c
index a19690a17291..de26b1441a48 100644
--- a/tools/perf/arch/s390/util/machine.c
+++ b/tools/perf/arch/s390/util/machine.c
@@ -7,7 +7,7 @@
 #include "api/fs/fs.h"
 #include "debug.h"
 
-int arch__fix_module_text_start(u64 *start, const char *name)
+int arch__fix_module_text_start(u64 *start, u64 *size, const char *name)
 {
  u64 m_start = *start;
  char path[PATH_MAX];
@@ -17,6 +17,18 @@ int arch__fix_module_text_start(u64 *start, const char *name)
  if (sysfs__read_ull(path, (unsigned long long *)start) < 0) {
  pr_debug2("Using module %s start:%#lx\n", path, m_start);
  *start = m_start;
+ } else {
+ /* Successful read of the modules segment text start address.
+ * Calculate difference between module start address
+ * in memory and module text segment start address.
+ * For example module load address is 0x3ff8011b000
+ * (from /proc/modules) and module text segment start
+ * address is 0x3ff8011b870 (from file above).
+ *
+ * Adjust the module size and subtract the GOT table
+ * size located at the beginning of the module.
+ */
+ *size -= (*start - m_start);
  }
 
  return 0;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 2c81923648cb..b53defbd6312 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1261,6 +1261,7 @@ static int machine__set_modules_path(struct machine *machine)
  return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);
 }
 int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
+ u64 *size __maybe_unused,
  const char *name __maybe_unused)
 {
  return 0;
@@ -1272,7 +1273,7 @@ static int machine__create_module(void *arg, const char *name, u64 start,
  struct machine *machine = arg;
  struct map *map;
 
- if (arch__fix_module_text_start(&start, name) < 0)
+ if (arch__fix_module_text_start(&start, &size, name) < 0)
  return -1;
 
  map = machine__findnew_module_map(machine, start, name);
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index b53ab92ea20c..059a06d4bf9a 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -227,7 +227,7 @@ struct symbol *machine__find_kernel_function_by_name(struct machine *machine,
 
 struct map *machine__findnew_module_map(struct machine *machine, u64 start,
  const char *filename);
-int arch__fix_module_text_start(u64 *start, const char *name);
+int arch__fix_module_text_start(u64 *start, u64 *size, const char *name);
 
 int __machine__load_kallsyms(struct machine *machine, const char *filename,
      enum map_type type, bool no_kcore);
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 2/2] perf annotate: Fix s390 gap between kernel end and module start

Kleber Souza
In reply to this post by Kleber Souza
From: Thomas Richter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1828166

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

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.

Reported-by: Klaus Theurich <[hidden email]>
Signed-off-by: Thomas Richter <[hidden email]>
Acked-by: Heiko Carstens <[hidden email]>
Cc: Hendrik Brueckner <[hidden email]>
Cc: Vasily Gorbik <[hidden email]>
Cc: [hidden email]
Link: http://lkml.kernel.org/r/20190724122703.3996-2-tmricht@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(cherry picked from commit b9c0a64901d5bdec6eafd38d1dc8fa0e2974fccb)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 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 d0840882c231..d4354cf65ed5 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -94,6 +94,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;
@@ -220,7 +225,7 @@ void symbols__fixup_end(struct rb_root *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 0563f33c1eb3..9bfff445472f 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -353,6 +353,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.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Disco][PATCH 1/3] perf record: Fix s390 missing module symbol and warning for non-root users

Kleber Souza
In reply to this post by Kleber Souza
From: Thomas Richter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1828166

Command 'perf record' and 'perf report' on a system without kernel
debuginfo packages uses /proc/kallsyms and /proc/modules to find
addresses for kernel and module symbols. On x86 this works for root and
non-root users.

On s390, when invoked as non-root user, many of the following warnings
are shown and module symbols are missing:

    proc/{kallsyms,modules} inconsistency while looking for
        "[sha1_s390]" module!

Command 'perf record' creates a list of module start addresses by
parsing the output of /proc/modules and creates a PERF_RECORD_MMAP
record for the kernel and each module. The following function call
sequence is executed:

  machine__create_kernel_maps
    machine__create_module
      modules__parse
        machine__create_module --> for each line in /proc/modules
          arch__fix_module_text_start

Function arch__fix_module_text_start() is s390 specific. It opens
file /sys/module/<name>/sections/.text to extract the module's .text
section start address. On s390 the module loader prepends a header
before the first section, whereas on x86 the module's text section
address is identical the the module's load address.

However module section files are root readable only. For non-root the
read operation fails and machine__create_module() returns an error.
Command perf record does not generate any PERF_RECORD_MMAP record
for loaded modules. Later command perf report complains about missing
module maps.

To fix this function arch__fix_module_text_start() always returns
success. For root users there is no change, for non-root users
the module's load address is used as module's text start address
(the prepended header then counts as part of the text section).

This enable non-root users to use module symbols and avoid the
warning when perf report is executed.

Output before:

  [tmricht@m83lp54 perf]$ ./perf report -D | fgrep MMAP
  0 0x168 [0x50]: PERF_RECORD_MMAP ... x [kernel.kallsyms]_text

Output after:

  [tmricht@m83lp54 perf]$ ./perf report -D | fgrep MMAP
  0 0x168 [0x50]: PERF_RECORD_MMAP ... x [kernel.kallsyms]_text
  0 0x1b8 [0x98]: PERF_RECORD_MMAP ... x /lib/modules/.../autofs4.ko.xz
  0 0x250 [0xa8]: PERF_RECORD_MMAP ... x /lib/modules/.../sha_common.ko.xz
  0 0x2f8 [0x98]: PERF_RECORD_MMAP ... x /lib/modules/.../des_generic.ko.xz

Signed-off-by: Thomas Richter <[hidden email]>
Reviewed-by: Hendrik Brueckner <[hidden email]>
Cc: Heiko Carstens <[hidden email]>
Link: http://lkml.kernel.org/r/20190522144601.50763-4-tmricht@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(cherry picked from commit 6738028dd57df064b969d8392c943ef3b3ae705d)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 tools/perf/arch/s390/util/machine.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/s390/util/machine.c b/tools/perf/arch/s390/util/machine.c
index 0b2054007314..a19690a17291 100644
--- a/tools/perf/arch/s390/util/machine.c
+++ b/tools/perf/arch/s390/util/machine.c
@@ -5,16 +5,19 @@
 #include "util.h"
 #include "machine.h"
 #include "api/fs/fs.h"
+#include "debug.h"
 
 int arch__fix_module_text_start(u64 *start, const char *name)
 {
+ u64 m_start = *start;
  char path[PATH_MAX];
 
  snprintf(path, PATH_MAX, "module/%.*s/sections/.text",
  (int)strlen(name) - 2, name + 1);
-
- if (sysfs__read_ull(path, (unsigned long long *)start) < 0)
- return -1;
+ if (sysfs__read_ull(path, (unsigned long long *)start) < 0) {
+ pr_debug2("Using module %s start:%#lx\n", path, m_start);
+ *start = m_start;
+ }
 
  return 0;
 }
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Disco][PATCH 2/3] perf record: Fix module size on s390

Kleber Souza
In reply to this post by Kleber Souza
From: Thomas Richter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1828166

On s390 the modules loaded in memory have the text segment located after
the GOT and Relocation table. This can be seen with this output:

  [root@m35lp76 perf]# fgrep qeth /proc/modules
  qeth 151552 1 qeth_l2, Live 0x000003ff800b2000
  ...
  [root@m35lp76 perf]# cat /sys/module/qeth/sections/.text
  0x000003ff800b3990
  [root@m35lp76 perf]#

There is an offset of 0x1990 bytes. The size of the qeth module is
151552 bytes (0x25000 in hex).

The location of the GOT/relocation table at the beginning of a module is
unique to s390.

commit 203d8a4aa6ed ("perf s390: Fix 'start' address of module's map")
adjusts the start address of a module in the map structures, but does
not adjust the size of the modules. This leads to overlapping of module
maps as this example shows:

[root@m35lp76 perf] # ./perf report -D
     0 0 0xfb0 [0xa0]: PERF_RECORD_MMAP -1/0: [0x3ff800b3990(0x25000)
          @ 0]:  x /lib/modules/.../qeth.ko.xz
     0 0 0x1050 [0xb0]: PERF_RECORD_MMAP -1/0: [0x3ff800d85a0(0x8000)
          @ 0]:  x /lib/modules/.../ip6_tables.ko.xz

The module qeth.ko has an adjusted start address modified to b3990, but
its size is unchanged and the module ends at 0x3ff800d8990.  This end
address overlaps with the next modules start address of 0x3ff800d85a0.

When the size of the leading GOT/Relocation table stored in the
beginning of the text segment (0x1990 bytes) is subtracted from module
qeth end address, there are no overlaps anymore:

   0x3ff800d8990 - 0x1990 = 0x0x3ff800d7000

which is the same as

   0x3ff800b2000 + 0x25000 = 0x0x3ff800d7000.

To fix this issue, also adjust the modules size in function
arch__fix_module_text_start(). Add another function parameter named size
and reduce the size of the module when the text segment start address is
changed.

Output after:
     0 0 0xfb0 [0xa0]: PERF_RECORD_MMAP -1/0: [0x3ff800b3990(0x23670)
          @ 0]:  x /lib/modules/.../qeth.ko.xz
     0 0 0x1050 [0xb0]: PERF_RECORD_MMAP -1/0: [0x3ff800d85a0(0x7a60)
          @ 0]:  x /lib/modules/.../ip6_tables.ko.xz

Reported-by: Stefan Liebler <[hidden email]>
Signed-off-by: Thomas Richter <[hidden email]>
Acked-by: Heiko Carstens <[hidden email]>
Cc: Hendrik Brueckner <[hidden email]>
Cc: Vasily Gorbik <[hidden email]>
Cc: [hidden email]
Fixes: 203d8a4aa6ed ("perf s390: Fix 'start' address of module's map")
Link: http://lkml.kernel.org/r/20190724122703.3996-1-tmricht@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(cherry picked from commit 12a6d2940b5f02b4b9f71ce098e3bb02bc24a9ea)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 tools/perf/arch/s390/util/machine.c | 14 +++++++++++++-
 tools/perf/util/machine.c           |  3 ++-
 tools/perf/util/machine.h           |  2 +-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/s390/util/machine.c b/tools/perf/arch/s390/util/machine.c
index a19690a17291..de26b1441a48 100644
--- a/tools/perf/arch/s390/util/machine.c
+++ b/tools/perf/arch/s390/util/machine.c
@@ -7,7 +7,7 @@
 #include "api/fs/fs.h"
 #include "debug.h"
 
-int arch__fix_module_text_start(u64 *start, const char *name)
+int arch__fix_module_text_start(u64 *start, u64 *size, const char *name)
 {
  u64 m_start = *start;
  char path[PATH_MAX];
@@ -17,6 +17,18 @@ int arch__fix_module_text_start(u64 *start, const char *name)
  if (sysfs__read_ull(path, (unsigned long long *)start) < 0) {
  pr_debug2("Using module %s start:%#lx\n", path, m_start);
  *start = m_start;
+ } else {
+ /* Successful read of the modules segment text start address.
+ * Calculate difference between module start address
+ * in memory and module text segment start address.
+ * For example module load address is 0x3ff8011b000
+ * (from /proc/modules) and module text segment start
+ * address is 0x3ff8011b870 (from file above).
+ *
+ * Adjust the module size and subtract the GOT table
+ * size located at the beginning of the module.
+ */
+ *size -= (*start - m_start);
  }
 
  return 0;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 596db1daee35..d3a5890b5047 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1295,6 +1295,7 @@ static int machine__set_modules_path(struct machine *machine)
  return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);
 }
 int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
+ u64 *size __maybe_unused,
  const char *name __maybe_unused)
 {
  return 0;
@@ -1306,7 +1307,7 @@ static int machine__create_module(void *arg, const char *name, u64 start,
  struct machine *machine = arg;
  struct map *map;
 
- if (arch__fix_module_text_start(&start, name) < 0)
+ if (arch__fix_module_text_start(&start, &size, name) < 0)
  return -1;
 
  map = machine__findnew_module_map(machine, start, name);
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index a5d1da60f751..85881aad1b75 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -219,7 +219,7 @@ struct symbol *machine__find_kernel_symbol_by_name(struct machine *machine,
 
 struct map *machine__findnew_module_map(struct machine *machine, u64 start,
  const char *filename);
-int arch__fix_module_text_start(u64 *start, const char *name);
+int arch__fix_module_text_start(u64 *start, u64 *size, const char *name);
 
 int machine__load_kallsyms(struct machine *machine, const char *filename);
 
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Disco][PATCH 3/3] perf annotate: Fix s390 gap between kernel end and module start

Kleber Souza
In reply to this post by Kleber Souza
From: Thomas Richter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1828166

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

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.

Reported-by: Klaus Theurich <[hidden email]>
Signed-off-by: Thomas Richter <[hidden email]>
Acked-by: Heiko Carstens <[hidden email]>
Cc: Hendrik Brueckner <[hidden email]>
Cc: Vasily Gorbik <[hidden email]>
Cc: [hidden email]
Link: http://lkml.kernel.org/r/20190724122703.3996-2-tmricht@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(cherry picked from commit b9c0a64901d5bdec6eafd38d1dc8fa0e2974fccb)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 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 ca5f2e4796ea..5c62f8f0332c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -86,6 +86,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;
@@ -212,7 +217,7 @@ void symbols__fixup_end(struct rb_root *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 14d9d438e7e2..b5822618dc13 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -351,6 +351,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.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][Disco][PATCH 0/2] Fix for perf top problem on s390x (LP: #1828166)

Connor Kuehl
In reply to this post by Kleber Souza
On 8/23/19 5:07 AM, Kleber Sacilotto de Souza wrote:

> BugLink: https://bugs.launchpad.net/bugs/1828166
>
> [Impact]
>
> * The perf top tool hangs and shows error messages, like 'Not enough
> memory for annotating'
>
> [Fix]
>
> The following fixes have been developed by IBM and accepted upstream:
>
> * b9c0a64901d5bdec6eafd38d1dc8fa0e2974fccb b9c0a64 "perf annotate: Fix
> s390 gap between kernel end and module start"
>
> * 12a6d2940b5f02b4b9f71ce098e3bb02bc24a9ea 12a6d29 "perf record: Fix
> module size on s390"
>
> Disco needs also as prereq:
>
> * 6738028dd57df064b969d8392c943ef3b3ae705d 6738028 perf record: Fix i
> s390 missing module symbol and warning for non-root users
>
> These fixes are either cherry picked or required small context
> adjustments.
>
> [Test Case]
>
> * start a benchmark (mem_alloc, but it doesn't really matter what)
>
> * execute perf top in a second terminal
>
> * the output of perf top is correct
>
> * now stop the benchmark
>
> * and perf top shows an error message, like "Not enough memory for
> annotating '__irf_end' symbol!)"
>
> * and perf top can't be exited anymore
>
> [Regression Potential]
>
> * The regression potential can be considered as medium since this
> happens only while using the perf top tool and just 3 files are
> changed, and one of them is arch/s390/util/machine.c. But symbol
> and machine header in /tools/perf/util modified and several
> loc added.
>
> * Smoke tested 'perf top' for regressions on a amd64 VM, no issues
> found.
>
> [Additional Info]
>
> These patches have already been committed to Eoan (LP: #1841110).
>
> Thomas Richter (2):
>   perf record: Fix module size on s390
>   perf annotate: Fix s390 gap between kernel end and module start
>
>  tools/perf/arch/s390/util/machine.c | 31 ++++++++++++++++++++++++++++-
>  tools/perf/util/machine.c           |  3 ++-
>  tools/perf/util/machine.h           |  2 +-
>  tools/perf/util/symbol.c            |  7 ++++++-
>  tools/perf/util/symbol.h            |  1 +
>  5 files changed, 40 insertions(+), 4 deletions(-)
>

+ positive test results from reporter and smoke test.

Acked-by: Connor Kuehl <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][Disco][PATCH 0/2] Fix for perf top problem on s390x (LP: #1828166)

Khaled Elmously
In reply to this post by Kleber Souza
On 2019-08-23 14:07:15 , Kleber Souza wrote:

> BugLink: https://bugs.launchpad.net/bugs/1828166
>
> [Impact]
>
> * The perf top tool hangs and shows error messages, like 'Not enough
> memory for annotating'
>
> [Fix]
>
> The following fixes have been developed by IBM and accepted upstream:
>
> * b9c0a64901d5bdec6eafd38d1dc8fa0e2974fccb b9c0a64 "perf annotate: Fix
> s390 gap between kernel end and module start"
>
> * 12a6d2940b5f02b4b9f71ce098e3bb02bc24a9ea 12a6d29 "perf record: Fix
> module size on s390"
>
> Disco needs also as prereq:
>
> * 6738028dd57df064b969d8392c943ef3b3ae705d 6738028 perf record: Fix i
> s390 missing module symbol and warning for non-root users
>
> These fixes are either cherry picked or required small context
> adjustments.
>
> [Test Case]
>
> * start a benchmark (mem_alloc, but it doesn't really matter what)
>
> * execute perf top in a second terminal
>
> * the output of perf top is correct
>
> * now stop the benchmark
>
> * and perf top shows an error message, like "Not enough memory for
> annotating '__irf_end' symbol!)"
>
> * and perf top can't be exited anymore
>
> [Regression Potential]
>
> * The regression potential can be considered as medium since this
> happens only while using the perf top tool and just 3 files are
> changed, and one of them is arch/s390/util/machine.c. But symbol
> and machine header in /tools/perf/util modified and several
> loc added.
>
> * Smoke tested 'perf top' for regressions on a amd64 VM, no issues
> found.
>
> [Additional Info]
>
> These patches have already been committed to Eoan (LP: #1841110).
>
> Thomas Richter (2):
>   perf record: Fix module size on s390
>   perf annotate: Fix s390 gap between kernel end and module start
>
>  tools/perf/arch/s390/util/machine.c | 31 ++++++++++++++++++++++++++++-
>  tools/perf/util/machine.c           |  3 ++-
>  tools/perf/util/machine.h           |  2 +-
>  tools/perf/util/symbol.c            |  7 ++++++-
>  tools/perf/util/symbol.h            |  1 +
>  5 files changed, 40 insertions(+), 4 deletions(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED/cmt: [SRU][Bionic][Disco][PATCH 0/2] Fix for perf top problem on s390x (LP: #1828166)

Khaled Elmously
In reply to this post by Kleber Souza
I just spent like 40 minutes reviewing this.. :(

All of these patches were already applied in B and D from upstream stable updates.


On 2019-08-23 14:07:15 , Kleber Souza wrote:

> BugLink: https://bugs.launchpad.net/bugs/1828166
>
> [Impact]
>
> * The perf top tool hangs and shows error messages, like 'Not enough
> memory for annotating'
>
> [Fix]
>
> The following fixes have been developed by IBM and accepted upstream:
>
> * b9c0a64901d5bdec6eafd38d1dc8fa0e2974fccb b9c0a64 "perf annotate: Fix
> s390 gap between kernel end and module start"
>
> * 12a6d2940b5f02b4b9f71ce098e3bb02bc24a9ea 12a6d29 "perf record: Fix
> module size on s390"
>
> Disco needs also as prereq:
>
> * 6738028dd57df064b969d8392c943ef3b3ae705d 6738028 perf record: Fix i
> s390 missing module symbol and warning for non-root users
>
> These fixes are either cherry picked or required small context
> adjustments.
>
> [Test Case]
>
> * start a benchmark (mem_alloc, but it doesn't really matter what)
>
> * execute perf top in a second terminal
>
> * the output of perf top is correct
>
> * now stop the benchmark
>
> * and perf top shows an error message, like "Not enough memory for
> annotating '__irf_end' symbol!)"
>
> * and perf top can't be exited anymore
>
> [Regression Potential]
>
> * The regression potential can be considered as medium since this
> happens only while using the perf top tool and just 3 files are
> changed, and one of them is arch/s390/util/machine.c. But symbol
> and machine header in /tools/perf/util modified and several
> loc added.
>
> * Smoke tested 'perf top' for regressions on a amd64 VM, no issues
> found.
>
> [Additional Info]
>
> These patches have already been committed to Eoan (LP: #1841110).
>
> Thomas Richter (2):
>   perf record: Fix module size on s390
>   perf annotate: Fix s390 gap between kernel end and module start
>
>  tools/perf/arch/s390/util/machine.c | 31 ++++++++++++++++++++++++++++-
>  tools/perf/util/machine.c           |  3 ++-
>  tools/perf/util/machine.h           |  2 +-
>  tools/perf/util/symbol.c            |  7 ++++++-
>  tools/perf/util/symbol.h            |  1 +
>  5 files changed, 40 insertions(+), 4 deletions(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team