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

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

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

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

[Impact]

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

[Fix]

* edeb0c90df3581b821a764052d185df985f8b8dc edeb0c9 "perf tools: Stop
  fallbacking to kallsyms for vdso symbols lookup"
* The patch can be cleany applied to Cosmic but needed a simple
  backport for Bionic for context adjustment.

[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 low since this
  happens only while using the perf top tool
* and it is known that the commit (above) fixes the problem
* and the fix is upstream since 4.19

Arnaldo Carvalho de Melo (1):
  perf tools: Stop fallbacking to kallsyms for vdso symbols lookup

 tools/perf/util/event.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 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/1] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup

Kleber Souza
From: Arnaldo Carvalho de Melo <[hidden email]>

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

David reports that:

<quote>
Perf has this hack where it uses the kernel symbol map as a backup when
a symbol can't be found in the user's symbol table(s).

This causes problems because the tests driving this code path use
machine__kernel_ip(), and that is completely meaningless on Sparc.  On
sparc64 the kernel and user live in physically separate virtual address
spaces, rather than a shared one.  And the kernel lives at a virtual
address that overlaps common userspace addresses.  So this test passes
almost all the time when a user symbol lookup fails.

The consequence of this is that, if the unfound user virtual address in
the sample doesn't match up to a kernel symbol either, we trigger things
like this code in builtin-top.c:

        if (al.sym == NULL && al.map != NULL) {
                const char *msg = "Kernel samples will not be resolved.\n";
                /*
                 * As we do lazy loading of symtabs we only will know if the
                 * specified vmlinux file is invalid when we actually have a
                 * hit in kernel space and then try to load it. So if we get
                 * here and there are _no_ symbols in the DSO backing the
                 * kernel map, bail out.
                 *
                 * We may never get here, for instance, if we use -K/
                 * --hide-kernel-symbols, even if the user specifies an
                 * invalid --vmlinux ;-)
                 */
                if (!machine->kptr_restrict_warned && !top->vmlinux_warned &&
                    __map__is_kernel(al.map) && map__has_symbols(al.map)) {
                        if (symbol_conf.vmlinux_name) {
                                char serr[256];
                                dso__strerror_load(al.map->dso, serr, sizeof(serr));
                                ui__warning("The %s file can't be used: %s\n%s",
                                            symbol_conf.vmlinux_name, serr, msg);
                        } else {
                                ui__warning("A vmlinux file was not found.\n%s",
                                            msg);
                        }

                        if (use_browser <= 0)
                                sleep(5);
                        top->vmlinux_warned = true;
                }
        }

When I fire up a compilation on sparc, this triggers immediately.

I'm trying to figure out what the "backup to kernel map" code is
accomplishing.

I see some language in the current code and in the changes that have
happened in this area talking about vdso.  Does that really happen?

The vdso is mapped into userspace virtual addresses, not kernel ones.

More history.  This didn't cause problems on sparc some time ago,
because the kernel IP check used to be "ip < 0" :-) Sparc kernel
addresses are not negative.  But now with machine__kernel_ip(), which
works using the symbol table determined kernel address range, it does
trigger.

What it all boils down to is that on architectures like sparc,
machine__kernel_ip() should always return false in this scenerio, and
therefore this kind of logic:

                if (cpumode == PERF_RECORD_MISC_USER && machine &&
                    mg != &machine->kmaps &&
                    machine__kernel_ip(machine, al->addr)) {

is basically invalid.  PERF_RECORD_MISC_USER implies no kernel address
can possibly match for the sample/event in question (no matter how
hard you try!) :-)
</>

So, I thought something had changed and in the past we would somehow
find that address in the kallsyms, but I couldn't find anything to back
that up, the patch introducing this is over a decade old, lots of things
changed, so I was just thinking I was missing something.

I tried a gtod busy loop to generate vdso activity and added a 'perf
probe' at that branch, on x86_64 to see if it ever gets hit:

Made thread__find_map() noinline, as 'perf probe' in lines of inline
functions seems to not be working, only at function start. (Masami?)

  # perf probe -x ~/bin/perf -L thread__find_map:57
  <thread__find_map@/home/acme/git/perf/tools/perf/util/event.c:57>
     57                 if (cpumode == PERF_RECORD_MISC_USER && machine &&
     58                     mg != &machine->kmaps &&
     59                     machine__kernel_ip(machine, al->addr)) {
     60                         mg = &machine->kmaps;
     61                         load_map = true;
     62                         goto try_again;
                        }
                } else {
                        /*
                         * Kernel maps might be changed when loading
                         * symbols so loading
                         * must be done prior to using kernel maps.
                         */
     69                 if (load_map)
     70                         map__load(al->map);
     71                 al->addr = al->map->map_ip(al->map, al->addr);

  # perf probe -x ~/bin/perf thread__find_map:60
  Added new event:
    probe_perf:thread__find_map (on thread__find_map:60 in /home/acme/bin/perf)

  You can now use it in all perf tools, such as:

        perf record -e probe_perf:thread__find_map -aR sleep 1

  #

  Then used this to see if, system wide, those probe points were being hit:

  # perf trace -e *perf:thread*/max-stack=8/
  ^C[root@jouet ~]#

  No hits when running 'perf top' and:

  # cat gtod.c
  #include <sys/time.h>

  int main(void)
  {
        struct timeval tv;

        while (1)
                gettimeofday(&tv, 0);

        return 0;
  }
  [root@jouet c]# ./gtod
  ^C

  Pressed 'P' in 'perf top' and the [vdso] samples are there:

  62.84%  [vdso]                    [.] __vdso_gettimeofday
   8.13%  gtod                      [.] main
   7.51%  [vdso]                    [.] 0x0000000000000914
   5.78%  [vdso]                    [.] 0x0000000000000917
   5.43%  gtod                      [.] _init
   2.71%  [vdso]                    [.] 0x000000000000092d
   0.35%  [kernel]                  [k] native_io_delay
   0.33%  libc-2.26.so              [.] __memmove_avx_unaligned_erms
   0.20%  [vdso]                    [.] 0x000000000000091d
   0.17%  [i2c_i801]                [k] i801_access
   0.06%  firefox                   [.] free
   0.06%  libglib-2.0.so.0.5400.3   [.] g_source_iter_next
   0.05%  [vdso]                    [.] 0x0000000000000919
   0.05%  libpthread-2.26.so        [.] __pthread_mutex_lock
   0.05%  libpixman-1.so.0.34.0     [.] 0x000000000006d3a7
   0.04%  [kernel]                  [k] entry_SYSCALL_64_trampoline
   0.04%  libxul.so                 [.] style::dom_apis::query_selector_slow
   0.04%  [kernel]                  [k] module_get_kallsym
   0.04%  firefox                   [.] malloc
   0.04%  [vdso]                    [.] 0x0000000000000910

  I added a 'perf probe' to thread__find_map:69, and that surely got tons
  of hits, i.e. for every map found, just to make sure the 'perf probe'
  command was really working.

  In the process I noticed a bug, we're only have records for '[vdso]' for
  pre-existing commands, i.e. ones that are running when we start 'perf top',
  when we will generate the PERF_RECORD_MMAP by looking at /perf/PID/maps.

  I.e. like this, for preexisting processes with a vdso map, again,
  tracing for all the system, only pre-existing processes get a [vdso] map
  (when having one):

  [root@jouet ~]# perf probe -x ~/bin/perf __machine__addnew_vdso
  Added new event:
  probe_perf:__machine__addnew_vdso (on __machine__addnew_vdso in /home/acme/bin/perf)

  You can now use it in all perf tools, such as:

        perf record -e probe_perf:__machine__addnew_vdso -aR sleep 1

  [root@jouet ~]# perf trace -e probe_perf:__machine__addnew_vdso/max-stack=8/
     0.000 probe_perf:__machine__addnew_vdso:(568eb3)
                                       __machine__addnew_vdso (/home/acme/bin/perf)
                                       map__new (/home/acme/bin/perf)
                                       machine__process_mmap2_event (/home/acme/bin/perf)
                                       machine__process_event (/home/acme/bin/perf)
                                       perf_event__process (/home/acme/bin/perf)
                                       perf_tool__process_synth_event (/home/acme/bin/perf)
                                       perf_event__synthesize_mmap_events (/home/acme/bin/perf)
                                       __event__synthesize_thread (/home/acme/bin/perf)

The kernel is generating a PERF_RECORD_MMAP for vDSOs, but somehow
'perf top' is not getting those records while 'perf record' is:

  # perf record ~acme/c/gtod
  ^C[ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.076 MB perf.data (1499 samples) ]

  # perf report -D | grep PERF_RECORD_MMAP2
  71293612401913 0x11b48 [0x70]: PERF_RECORD_MMAP2 25484/25484: [0x400000(0x1000) @ 0 fd:02 1137 541179306]: r-xp /home/acme/c/gtod
  71293612419012 0x11be0 [0x70]: PERF_RECORD_MMAP2 25484/25484: [0x7fa4a2783000(0x227000) @ 0 fd:00 3146370 854107250]: r-xp /usr/lib64/ld-2.26.so
  71293612432110 0x11c50 [0x60]: PERF_RECORD_MMAP2 25484/25484: [0x7ffcdb53a000(0x2000) @ 0 00:00 0 0]: r-xp [vdso]
  71293612509944 0x11cb0 [0x70]: PERF_RECORD_MMAP2 25484/25484: [0x7fa4a23cd000(0x3b6000) @ 0 fd:00 3149723 262067164]: r-xp /usr/lib64/libc-2.26.so
  #
  # perf script | grep vdso | head
      gtod 25484 71293.612768: 2485554 cycles:ppp:  7ffcdb53a914 [unknown] ([vdso])
      gtod 25484 71293.613576: 2149343 cycles:ppp:  7ffcdb53a917 [unknown] ([vdso])
      gtod 25484 71293.614274: 1814652 cycles:ppp:  7ffcdb53aca8 __vdso_gettimeofday+0x98 ([vdso])
      gtod 25484 71293.614862: 1669070 cycles:ppp:  7ffcdb53acc5 __vdso_gettimeofday+0xb5 ([vdso])
      gtod 25484 71293.615404: 1451589 cycles:ppp:  7ffcdb53acc5 __vdso_gettimeofday+0xb5 ([vdso])
      gtod 25484 71293.615999: 1269941 cycles:ppp:  7ffcdb53ace6 __vdso_gettimeofday+0xd6 ([vdso])
      gtod 25484 71293.616405: 1177946 cycles:ppp:  7ffcdb53a914 [unknown] ([vdso])
      gtod 25484 71293.616775: 1121290 cycles:ppp:  7ffcdb53ac47 __vdso_gettimeofday+0x37 ([vdso])
      gtod 25484 71293.617150: 1037721 cycles:ppp:  7ffcdb53ace6 __vdso_gettimeofday+0xd6 ([vdso])
      gtod 25484 71293.617478:  994526 cycles:ppp:  7ffcdb53ace6 __vdso_gettimeofday+0xd6 ([vdso])
  #

The patch is the obvious one and with it we also continue to resolve
vdso symbols for pre-existing processes in 'perf top' and for all
processes in 'perf record' + 'perf report/script'.

Suggested-by: David Miller <[hidden email]>
Acked-by: David Miller <[hidden email]>
Cc: Adrian Hunter <[hidden email]>
Cc: David Ahern <[hidden email]>
Cc: Jiri Olsa <[hidden email]>
Cc: Namhyung Kim <[hidden email]>
Cc: Wang Nan <[hidden email]>
Link: https://lkml.kernel.org/n/tip-cs7skq9pp0kjypiju6o7trse@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(backported from commit edeb0c90df3581b821a764052d185df985f8b8dc)
[ klebers: context adjustments, function thread__find_map() was called
  thread__find_addr_map() on 4.15. ]
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 tools/perf/util/event.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 97a8ef9980db..8d040f95998b 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1528,26 +1528,9 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
 
  return;
  }
-try_again:
+
  al->map = map_groups__find(mg, type, al->addr);
- if (al->map == NULL) {
- /*
- * If this is outside of all known maps, and is a negative
- * address, try to look it up in the kernel dso, as it might be
- * a vsyscall or vdso (which executes in user-mode).
- *
- * XXX This is nasty, we should have a symbol list in the
- * "[vdso]" dso, but for now lets use the old trick of looking
- * in the whole kernel symbol list.
- */
- if (cpumode == PERF_RECORD_MISC_USER && machine &&
-    mg != &machine->kmaps &&
-    machine__kernel_ip(machine, al->addr)) {
- mg = &machine->kmaps;
- load_map = true;
- goto try_again;
- }
- } else {
+ if (al->map != NULL) {
  /*
  * Kernel maps might be changed when loading symbols so loading
  * must be done prior to using kernel maps.
--
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][Cosmic][PATCH 1/1] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup

Kleber Souza
In reply to this post by Kleber Souza
From: Arnaldo Carvalho de Melo <[hidden email]>

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

David reports that:

<quote>
Perf has this hack where it uses the kernel symbol map as a backup when
a symbol can't be found in the user's symbol table(s).

This causes problems because the tests driving this code path use
machine__kernel_ip(), and that is completely meaningless on Sparc.  On
sparc64 the kernel and user live in physically separate virtual address
spaces, rather than a shared one.  And the kernel lives at a virtual
address that overlaps common userspace addresses.  So this test passes
almost all the time when a user symbol lookup fails.

The consequence of this is that, if the unfound user virtual address in
the sample doesn't match up to a kernel symbol either, we trigger things
like this code in builtin-top.c:

        if (al.sym == NULL && al.map != NULL) {
                const char *msg = "Kernel samples will not be resolved.\n";
                /*
                 * As we do lazy loading of symtabs we only will know if the
                 * specified vmlinux file is invalid when we actually have a
                 * hit in kernel space and then try to load it. So if we get
                 * here and there are _no_ symbols in the DSO backing the
                 * kernel map, bail out.
                 *
                 * We may never get here, for instance, if we use -K/
                 * --hide-kernel-symbols, even if the user specifies an
                 * invalid --vmlinux ;-)
                 */
                if (!machine->kptr_restrict_warned && !top->vmlinux_warned &&
                    __map__is_kernel(al.map) && map__has_symbols(al.map)) {
                        if (symbol_conf.vmlinux_name) {
                                char serr[256];
                                dso__strerror_load(al.map->dso, serr, sizeof(serr));
                                ui__warning("The %s file can't be used: %s\n%s",
                                            symbol_conf.vmlinux_name, serr, msg);
                        } else {
                                ui__warning("A vmlinux file was not found.\n%s",
                                            msg);
                        }

                        if (use_browser <= 0)
                                sleep(5);
                        top->vmlinux_warned = true;
                }
        }

When I fire up a compilation on sparc, this triggers immediately.

I'm trying to figure out what the "backup to kernel map" code is
accomplishing.

I see some language in the current code and in the changes that have
happened in this area talking about vdso.  Does that really happen?

The vdso is mapped into userspace virtual addresses, not kernel ones.

More history.  This didn't cause problems on sparc some time ago,
because the kernel IP check used to be "ip < 0" :-) Sparc kernel
addresses are not negative.  But now with machine__kernel_ip(), which
works using the symbol table determined kernel address range, it does
trigger.

What it all boils down to is that on architectures like sparc,
machine__kernel_ip() should always return false in this scenerio, and
therefore this kind of logic:

                if (cpumode == PERF_RECORD_MISC_USER && machine &&
                    mg != &machine->kmaps &&
                    machine__kernel_ip(machine, al->addr)) {

is basically invalid.  PERF_RECORD_MISC_USER implies no kernel address
can possibly match for the sample/event in question (no matter how
hard you try!) :-)
</>

So, I thought something had changed and in the past we would somehow
find that address in the kallsyms, but I couldn't find anything to back
that up, the patch introducing this is over a decade old, lots of things
changed, so I was just thinking I was missing something.

I tried a gtod busy loop to generate vdso activity and added a 'perf
probe' at that branch, on x86_64 to see if it ever gets hit:

Made thread__find_map() noinline, as 'perf probe' in lines of inline
functions seems to not be working, only at function start. (Masami?)

  # perf probe -x ~/bin/perf -L thread__find_map:57
  <thread__find_map@/home/acme/git/perf/tools/perf/util/event.c:57>
     57                 if (cpumode == PERF_RECORD_MISC_USER && machine &&
     58                     mg != &machine->kmaps &&
     59                     machine__kernel_ip(machine, al->addr)) {
     60                         mg = &machine->kmaps;
     61                         load_map = true;
     62                         goto try_again;
                        }
                } else {
                        /*
                         * Kernel maps might be changed when loading
                         * symbols so loading
                         * must be done prior to using kernel maps.
                         */
     69                 if (load_map)
     70                         map__load(al->map);
     71                 al->addr = al->map->map_ip(al->map, al->addr);

  # perf probe -x ~/bin/perf thread__find_map:60
  Added new event:
    probe_perf:thread__find_map (on thread__find_map:60 in /home/acme/bin/perf)

  You can now use it in all perf tools, such as:

        perf record -e probe_perf:thread__find_map -aR sleep 1

  #

  Then used this to see if, system wide, those probe points were being hit:

  # perf trace -e *perf:thread*/max-stack=8/
  ^C[root@jouet ~]#

  No hits when running 'perf top' and:

  # cat gtod.c
  #include <sys/time.h>

  int main(void)
  {
        struct timeval tv;

        while (1)
                gettimeofday(&tv, 0);

        return 0;
  }
  [root@jouet c]# ./gtod
  ^C

  Pressed 'P' in 'perf top' and the [vdso] samples are there:

  62.84%  [vdso]                    [.] __vdso_gettimeofday
   8.13%  gtod                      [.] main
   7.51%  [vdso]                    [.] 0x0000000000000914
   5.78%  [vdso]                    [.] 0x0000000000000917
   5.43%  gtod                      [.] _init
   2.71%  [vdso]                    [.] 0x000000000000092d
   0.35%  [kernel]                  [k] native_io_delay
   0.33%  libc-2.26.so              [.] __memmove_avx_unaligned_erms
   0.20%  [vdso]                    [.] 0x000000000000091d
   0.17%  [i2c_i801]                [k] i801_access
   0.06%  firefox                   [.] free
   0.06%  libglib-2.0.so.0.5400.3   [.] g_source_iter_next
   0.05%  [vdso]                    [.] 0x0000000000000919
   0.05%  libpthread-2.26.so        [.] __pthread_mutex_lock
   0.05%  libpixman-1.so.0.34.0     [.] 0x000000000006d3a7
   0.04%  [kernel]                  [k] entry_SYSCALL_64_trampoline
   0.04%  libxul.so                 [.] style::dom_apis::query_selector_slow
   0.04%  [kernel]                  [k] module_get_kallsym
   0.04%  firefox                   [.] malloc
   0.04%  [vdso]                    [.] 0x0000000000000910

  I added a 'perf probe' to thread__find_map:69, and that surely got tons
  of hits, i.e. for every map found, just to make sure the 'perf probe'
  command was really working.

  In the process I noticed a bug, we're only have records for '[vdso]' for
  pre-existing commands, i.e. ones that are running when we start 'perf top',
  when we will generate the PERF_RECORD_MMAP by looking at /perf/PID/maps.

  I.e. like this, for preexisting processes with a vdso map, again,
  tracing for all the system, only pre-existing processes get a [vdso] map
  (when having one):

  [root@jouet ~]# perf probe -x ~/bin/perf __machine__addnew_vdso
  Added new event:
  probe_perf:__machine__addnew_vdso (on __machine__addnew_vdso in /home/acme/bin/perf)

  You can now use it in all perf tools, such as:

        perf record -e probe_perf:__machine__addnew_vdso -aR sleep 1

  [root@jouet ~]# perf trace -e probe_perf:__machine__addnew_vdso/max-stack=8/
     0.000 probe_perf:__machine__addnew_vdso:(568eb3)
                                       __machine__addnew_vdso (/home/acme/bin/perf)
                                       map__new (/home/acme/bin/perf)
                                       machine__process_mmap2_event (/home/acme/bin/perf)
                                       machine__process_event (/home/acme/bin/perf)
                                       perf_event__process (/home/acme/bin/perf)
                                       perf_tool__process_synth_event (/home/acme/bin/perf)
                                       perf_event__synthesize_mmap_events (/home/acme/bin/perf)
                                       __event__synthesize_thread (/home/acme/bin/perf)

The kernel is generating a PERF_RECORD_MMAP for vDSOs, but somehow
'perf top' is not getting those records while 'perf record' is:

  # perf record ~acme/c/gtod
  ^C[ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.076 MB perf.data (1499 samples) ]

  # perf report -D | grep PERF_RECORD_MMAP2
  71293612401913 0x11b48 [0x70]: PERF_RECORD_MMAP2 25484/25484: [0x400000(0x1000) @ 0 fd:02 1137 541179306]: r-xp /home/acme/c/gtod
  71293612419012 0x11be0 [0x70]: PERF_RECORD_MMAP2 25484/25484: [0x7fa4a2783000(0x227000) @ 0 fd:00 3146370 854107250]: r-xp /usr/lib64/ld-2.26.so
  71293612432110 0x11c50 [0x60]: PERF_RECORD_MMAP2 25484/25484: [0x7ffcdb53a000(0x2000) @ 0 00:00 0 0]: r-xp [vdso]
  71293612509944 0x11cb0 [0x70]: PERF_RECORD_MMAP2 25484/25484: [0x7fa4a23cd000(0x3b6000) @ 0 fd:00 3149723 262067164]: r-xp /usr/lib64/libc-2.26.so
  #
  # perf script | grep vdso | head
      gtod 25484 71293.612768: 2485554 cycles:ppp:  7ffcdb53a914 [unknown] ([vdso])
      gtod 25484 71293.613576: 2149343 cycles:ppp:  7ffcdb53a917 [unknown] ([vdso])
      gtod 25484 71293.614274: 1814652 cycles:ppp:  7ffcdb53aca8 __vdso_gettimeofday+0x98 ([vdso])
      gtod 25484 71293.614862: 1669070 cycles:ppp:  7ffcdb53acc5 __vdso_gettimeofday+0xb5 ([vdso])
      gtod 25484 71293.615404: 1451589 cycles:ppp:  7ffcdb53acc5 __vdso_gettimeofday+0xb5 ([vdso])
      gtod 25484 71293.615999: 1269941 cycles:ppp:  7ffcdb53ace6 __vdso_gettimeofday+0xd6 ([vdso])
      gtod 25484 71293.616405: 1177946 cycles:ppp:  7ffcdb53a914 [unknown] ([vdso])
      gtod 25484 71293.616775: 1121290 cycles:ppp:  7ffcdb53ac47 __vdso_gettimeofday+0x37 ([vdso])
      gtod 25484 71293.617150: 1037721 cycles:ppp:  7ffcdb53ace6 __vdso_gettimeofday+0xd6 ([vdso])
      gtod 25484 71293.617478:  994526 cycles:ppp:  7ffcdb53ace6 __vdso_gettimeofday+0xd6 ([vdso])
  #

The patch is the obvious one and with it we also continue to resolve
vdso symbols for pre-existing processes in 'perf top' and for all
processes in 'perf record' + 'perf report/script'.

Suggested-by: David Miller <[hidden email]>
Acked-by: David Miller <[hidden email]>
Cc: Adrian Hunter <[hidden email]>
Cc: David Ahern <[hidden email]>
Cc: Jiri Olsa <[hidden email]>
Cc: Namhyung Kim <[hidden email]>
Cc: Wang Nan <[hidden email]>
Link: https://lkml.kernel.org/n/tip-cs7skq9pp0kjypiju6o7trse@...
Signed-off-by: Arnaldo Carvalho de Melo <[hidden email]>
(cherry picked from commit edeb0c90df3581b821a764052d185df985f8b8dc)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 tools/perf/util/event.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 6f3db78efe39..9087fe3ee5de 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1554,26 +1554,9 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
 
  return NULL;
  }
-try_again:
+
  al->map = map_groups__find(mg, al->addr);
- if (al->map == NULL) {
- /*
- * If this is outside of all known maps, and is a negative
- * address, try to look it up in the kernel dso, as it might be
- * a vsyscall or vdso (which executes in user-mode).
- *
- * XXX This is nasty, we should have a symbol list in the
- * "[vdso]" dso, but for now lets use the old trick of looking
- * in the whole kernel symbol list.
- */
- if (cpumode == PERF_RECORD_MISC_USER && machine &&
-    mg != &machine->kmaps &&
-    machine__kernel_ip(machine, al->addr)) {
- mg = &machine->kmaps;
- load_map = true;
- goto try_again;
- }
- } else {
+ if (al->map != NULL) {
  /*
  * Kernel maps might be changed when loading symbols so loading
  * must be done prior to using kernel maps.
--
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/Cmnt: [SRU][Bionic][Cosmic][PATCH 0/1] Fix for perf top problem on s390x (LP: #1828166)

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

> BugLink: https://bugs.launchpad.net/1828166
>
> [Impact]
>
> * The perf top tool hangs and shows error messages, like 'Not enough
>   memory for annotating'
>
> [Fix]
>
> * edeb0c90df3581b821a764052d185df985f8b8dc edeb0c9 "perf tools: Stop
>   fallbacking to kallsyms for vdso symbols lookup"
> * The patch can be cleany applied to Cosmic but needed a simple
>   backport for Bionic for context adjustment.
>
> [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 low since this
>   happens only while using the perf top tool
> * and it is known that the commit (above) fixes the problem
> * and the fix is upstream since 4.19
>
> Arnaldo Carvalho de Melo (1):
>   perf tools: Stop fallbacking to kallsyms for vdso symbols lookup
>
>  tools/perf/util/event.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
>
The BugLink will need to be adjusted slightly when this is applied to
have the '/bugs/' component:
https://bugs.launchpad.net/bugs/1828166

Otherwise it seems like a mature fix and the accompanying documentation
is thorough.

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

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

pEpkey.asc (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Marcelo Henrique Cerri
In reply to this post by Kleber Souza
With the fixed buglink...

Acked-by: Marcelo Henrique Cerri <[hidden email]>

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

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Kleber Souza
In reply to this post by Kleber Souza
On 5/9/19 2:44 PM, Kleber Sacilotto de Souza wrote:

> BugLink: https://bugs.launchpad.net/1828166
>
> [Impact]
>
> * The perf top tool hangs and shows error messages, like 'Not enough
>   memory for annotating'
>
> [Fix]
>
> * edeb0c90df3581b821a764052d185df985f8b8dc edeb0c9 "perf tools: Stop
>   fallbacking to kallsyms for vdso symbols lookup"
> * The patch can be cleany applied to Cosmic but needed a simple
>   backport for Bionic for context adjustment.
>
> [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 low since this
>   happens only while using the perf top tool
> * and it is known that the commit (above) fixes the problem
> * and the fix is upstream since 4.19
>
> Arnaldo Carvalho de Melo (1):
>   perf tools: Stop fallbacking to kallsyms for vdso symbols lookup
>
>  tools/perf/util/event.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
>

This patch doesn't really fix the issue reported, so I'm NAK'ing
it and will not apply the patch until we are sure of the fix.


Thanks,
Kleber

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