[PATCH] perf tools: Fix build-id matching on vmlinux

[PATCH] perf tools: Fix build-id matching on vmlinux

[Namhyung Kim]

There's a problem on finding correct kernel symbols when perf report
runs on a different kernel.  Although a part of the problem was solved
by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
version when finding vmlinux"), there's a remaining problem still.

When perf records samples, it synthesizes the kernel map using
machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
You can easily see it using 'perf report -D' command.

After finishing record, it goes through the recorded events to find
maps/dsos actually used.  And then record build-id info of them.

During this process, it needs to load symbols in a dso and it'd call
dso__load_vmlinux() since the default value of the symbol_conf.try_
vmlinux_path is true.  However it changes dso->long_name to a real
path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
if one is running on a custom kernel.

It resulted in that perf report reads the build-id of the vmlinux, but
cannot use it since it only knows about the [kernel.kallsyms] map.  It
then falls back to possible vmlinux paths by using the recorded kernel
version (in case of a recent version) or a running kernel silently
(which might break the result).  I think it's worth going to the
stable tree.

I can think of a couple of ways to fix it.  In this patch, I changed
to use the name of "[kernel.kallsyms]" for the kernel build-id event
instead of not trying vmlinux paths.  This way we can provide maximum
info (like annotation) with minimum change IMHO.

Before:

  $ perf record -a usleep 1

  $ perf buildid-list
  00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
  78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
  4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
  1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so

  $ perf buildid-list -H
  0000000000000000000000000000000000000000 [kernel.kallsyms]
  78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
  4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
  1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
  0000000000000000000000000000000000000000 /tmp/perf-2523.map

After:

  $ perf record -a usleep 1

  $ perf buildid-list
  00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
  78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
  4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
  1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so

  $ perf buildid-list -H
  00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
  78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
  4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
  1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
  0000000000000000000000000000000000000000 /tmp/perf-2523.map

Cc: stable@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/header.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 158c787ce0c4..5c4093dee467 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -263,6 +263,9 @@ static int __dsos__write_buildid_table(struct list_head *head,
 			machine__mmap_name(machine, nm, sizeof(nm));
 			name = nm;
 			name_len = strlen(nm) + 1;
+		} else if (dso__is_vmlinux(pos)) {
+			name = pos->name;
+			name_len = strlen(pos->name) + 1;
 		} else {
 			name = pos->long_name;
 			name_len = pos->long_name_len + 1;
-- 
2.0.0

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Namhyung Kim]

Ping!


On Tue, 26 Aug 2014 15:38:39 +0900, Namhyung Kim wrote:
> There's a problem on finding correct kernel symbols when perf report
> runs on a different kernel.  Although a part of the problem was solved
> by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
> version when finding vmlinux"), there's a remaining problem still.
>
> When perf records samples, it synthesizes the kernel map using
> machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
> You can easily see it using 'perf report -D' command.
>
> After finishing record, it goes through the recorded events to find
> maps/dsos actually used.  And then record build-id info of them.
>
> During this process, it needs to load symbols in a dso and it'd call
> dso__load_vmlinux() since the default value of the symbol_conf.try_
> vmlinux_path is true.  However it changes dso->long_name to a real
> path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
> if one is running on a custom kernel.
>
> It resulted in that perf report reads the build-id of the vmlinux, but
> cannot use it since it only knows about the [kernel.kallsyms] map.  It
> then falls back to possible vmlinux paths by using the recorded kernel
> version (in case of a recent version) or a running kernel silently
> (which might break the result).  I think it's worth going to the
> stable tree.
>
> I can think of a couple of ways to fix it.  In this patch, I changed
> to use the name of "[kernel.kallsyms]" for the kernel build-id event
> instead of not trying vmlinux paths.  This way we can provide maximum
> info (like annotation) with minimum change IMHO.
>
> Before:
>
>   $ perf record -a usleep 1
>
>   $ perf buildid-list
>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>
>   $ perf buildid-list -H
>   0000000000000000000000000000000000000000 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>
> After:
>
>   $ perf record -a usleep 1
>
>   $ perf buildid-list
>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>
>   $ perf buildid-list -H
>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/header.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 158c787ce0c4..5c4093dee467 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -263,6 +263,9 @@ static int __dsos__write_buildid_table(struct list_head *head,
>  			machine__mmap_name(machine, nm, sizeof(nm));
>  			name = nm;
>  			name_len = strlen(nm) + 1;
> +		} else if (dso__is_vmlinux(pos)) {
> +			name = pos->name;
> +			name_len = strlen(pos->name) + 1;
>  		} else {
>  			name = pos->long_name;
>  			name_len = pos->long_name_len + 1;

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Arnaldo Carvalho de Melo]

Em Tue, Aug 26, 2014 at 03:38:39PM +0900, Namhyung Kim escreveu:
> There's a problem on finding correct kernel symbols when perf report
> runs on a different kernel.  Although a part of the problem was solved
> by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
> version when finding vmlinux"), there's a remaining problem still.
> 
> When perf records samples, it synthesizes the kernel map using
> machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
> You can easily see it using 'perf report -D' command.
> 
> After finishing record, it goes through the recorded events to find
> maps/dsos actually used.  And then record build-id info of them.
> 
> During this process, it needs to load symbols in a dso and it'd call
> dso__load_vmlinux() since the default value of the symbol_conf.try_
> vmlinux_path is true.  However it changes dso->long_name to a real
> path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
> if one is running on a custom kernel.
> 
> It resulted in that perf report reads the build-id of the vmlinux, but
> cannot use it since it only knows about the [kernel.kallsyms] map.  It
> then falls back to possible vmlinux paths by using the recorded kernel
> version (in case of a recent version) or a running kernel silently
> (which might break the result).  I think it's worth going to the
> stable tree.
> 
> I can think of a couple of ways to fix it.  In this patch, I changed
> to use the name of "[kernel.kallsyms]" for the kernel build-id event
> instead of not trying vmlinux paths.  This way we can provide maximum
> info (like annotation) with minimum change IMHO.
> 
> Before:
> 
>   $ perf record -a usleep 1
> 
>   $ perf buildid-list
>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
> 
>   $ perf buildid-list -H
>   0000000000000000000000000000000000000000 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
> 
> After:
> 
>   $ perf record -a usleep 1
> 
>   $ perf buildid-list
>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]

We are losing information, namely the pathname for the kernel used, that
may be useful in analysis.

Why not make sure that if there is a build-id in the perf.data header,
then we completely refusing anything that doesn't match the build-id?
I.e. the name is irrelevant for this purpose, the contents, as keyed by
the build-id, is what matters.

- Arnaldo

>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
> 
>   $ perf buildid-list -H
>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/header.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 158c787ce0c4..5c4093dee467 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -263,6 +263,9 @@ static int __dsos__write_buildid_table(struct list_head *head,
>  			machine__mmap_name(machine, nm, sizeof(nm));
>  			name = nm;
>  			name_len = strlen(nm) + 1;
> +		} else if (dso__is_vmlinux(pos)) {
> +			name = pos->name;
> +			name_len = strlen(pos->name) + 1;
>  		} else {
>  			name = pos->long_name;
>  			name_len = pos->long_name_len + 1;
> -- 
> 2.0.0

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Stephane Eranian]

On Tue, Aug 26, 2014 at 8:38 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> There's a problem on finding correct kernel symbols when perf report
> runs on a different kernel.  Although a part of the problem was solved
> by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
> version when finding vmlinux"), there's a remaining problem still.
>
> When perf records samples, it synthesizes the kernel map using
> machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
> You can easily see it using 'perf report -D' command.
>
> After finishing record, it goes through the recorded events to find
> maps/dsos actually used.  And then record build-id info of them.
>
> During this process, it needs to load symbols in a dso and it'd call
> dso__load_vmlinux() since the default value of the symbol_conf.try_
> vmlinux_path is true.  However it changes dso->long_name to a real
> path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
> if one is running on a custom kernel.
>
> It resulted in that perf report reads the build-id of the vmlinux, but
> cannot use it since it only knows about the [kernel.kallsyms] map.  It
> then falls back to possible vmlinux paths by using the recorded kernel
> version (in case of a recent version) or a running kernel silently
> (which might break the result).  I think it's worth going to the
> stable tree.
>
> I can think of a couple of ways to fix it.  In this patch, I changed
> to use the name of "[kernel.kallsyms]" for the kernel build-id event
> instead of not trying vmlinux paths.  This way we can provide maximum
> info (like annotation) with minimum change IMHO.
>
> Before:
>
>   $ perf record -a usleep 1
>
>   $ perf buildid-list
>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>
>   $ perf buildid-list -H
>   0000000000000000000000000000000000000000 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>
There is something I don't understand in your example above.
The -H option shows only DSO with samples. So why do you get the
buildid without -H
and you get no buildid with -H? In other words, I don't connect the
dots between what
-H does on the buildid change for the kernel. Looks like you have the
buildid in the
perf.data file.



>
> After:
>
>   $ perf record -a usleep 1
>
>   $ perf buildid-list
>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>
>   $ perf buildid-list -H
>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/header.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 158c787ce0c4..5c4093dee467 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -263,6 +263,9 @@ static int __dsos__write_buildid_table(struct list_head *head,
>                         machine__mmap_name(machine, nm, sizeof(nm));
>                         name = nm;
>                         name_len = strlen(nm) + 1;
> +               } else if (dso__is_vmlinux(pos)) {
> +                       name = pos->name;
> +                       name_len = strlen(pos->name) + 1;
>                 } else {
>                         name = pos->long_name;
>                         name_len = pos->long_name_len + 1;
> --
> 2.0.0
>

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Namhyung Kim]

Hi Arnaldo,

On Thu, 4 Sep 2014 11:18:45 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 26, 2014 at 03:38:39PM +0900, Namhyung Kim escreveu:
>> There's a problem on finding correct kernel symbols when perf report
>> runs on a different kernel.  Although a part of the problem was solved
>> by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
>> version when finding vmlinux"), there's a remaining problem still.
>> 
>> When perf records samples, it synthesizes the kernel map using
>> machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
>> You can easily see it using 'perf report -D' command.
>> 
>> After finishing record, it goes through the recorded events to find
>> maps/dsos actually used.  And then record build-id info of them.
>> 
>> During this process, it needs to load symbols in a dso and it'd call
>> dso__load_vmlinux() since the default value of the symbol_conf.try_
>> vmlinux_path is true.  However it changes dso->long_name to a real
>> path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
>> if one is running on a custom kernel.
>> 
>> It resulted in that perf report reads the build-id of the vmlinux, but
>> cannot use it since it only knows about the [kernel.kallsyms] map.  It
>> then falls back to possible vmlinux paths by using the recorded kernel
>> version (in case of a recent version) or a running kernel silently
>> (which might break the result).  I think it's worth going to the
>> stable tree.
>> 
>> I can think of a couple of ways to fix it.  In this patch, I changed
>> to use the name of "[kernel.kallsyms]" for the kernel build-id event
>> instead of not trying vmlinux paths.  This way we can provide maximum
>> info (like annotation) with minimum change IMHO.
>> 
>> Before:
>> 
>>   $ perf record -a usleep 1
>> 
>>   $ perf buildid-list
>>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>> 
>>   $ perf buildid-list -H
>>   0000000000000000000000000000000000000000 [kernel.kallsyms]
>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>> 
>> After:
>> 
>>   $ perf record -a usleep 1
>> 
>>   $ perf buildid-list
>>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>
> We are losing information, namely the pathname for the kernel used, that
> may be useful in analysis.

Right.  That's a problem.


>
> Why not make sure that if there is a build-id in the perf.data header,
> then we completely refusing anything that doesn't match the build-id?
> I.e. the name is irrelevant for this purpose, the contents, as keyed by
> the build-id, is what matters.

The perf report rebuilds machine states from the event records only.  In
this case, the kernel map was recorded in the name of [kernel.kallsyms]
so it couldn't find the build-id from the table.

Thanks,
Namhyung


>
> - Arnaldo
>
>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>> 
>>   $ perf buildid-list -H
>>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]
>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>> 
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> ---
>>  tools/perf/util/header.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index 158c787ce0c4..5c4093dee467 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -263,6 +263,9 @@ static int __dsos__write_buildid_table(struct list_head *head,
>>  			machine__mmap_name(machine, nm, sizeof(nm));
>>  			name = nm;
>>  			name_len = strlen(nm) + 1;
>> +		} else if (dso__is_vmlinux(pos)) {
>> +			name = pos->name;
>> +			name_len = strlen(pos->name) + 1;
>>  		} else {
>>  			name = pos->long_name;
>>  			name_len = pos->long_name_len + 1;
>> -- 
>> 2.0.0

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Namhyung Kim]

Hi Stephane,

On Thu, 4 Sep 2014 16:37:51 +0200, Stephane Eranian wrote:
> On Tue, Aug 26, 2014 at 8:38 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>>
>> There's a problem on finding correct kernel symbols when perf report
>> runs on a different kernel.  Although a part of the problem was solved
>> by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
>> version when finding vmlinux"), there's a remaining problem still.
>>
>> When perf records samples, it synthesizes the kernel map using
>> machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
>> You can easily see it using 'perf report -D' command.
>>
>> After finishing record, it goes through the recorded events to find
>> maps/dsos actually used.  And then record build-id info of them.
>>
>> During this process, it needs to load symbols in a dso and it'd call
>> dso__load_vmlinux() since the default value of the symbol_conf.try_
>> vmlinux_path is true.  However it changes dso->long_name to a real
>> path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
>> if one is running on a custom kernel.
>>
>> It resulted in that perf report reads the build-id of the vmlinux, but
>> cannot use it since it only knows about the [kernel.kallsyms] map.  It
>> then falls back to possible vmlinux paths by using the recorded kernel
>> version (in case of a recent version) or a running kernel silently
>> (which might break the result).  I think it's worth going to the
>> stable tree.
>>
>> I can think of a couple of ways to fix it.  In this patch, I changed
>> to use the name of "[kernel.kallsyms]" for the kernel build-id event
>> instead of not trying vmlinux paths.  This way we can provide maximum
>> info (like annotation) with minimum change IMHO.
>>
>> Before:
>>
>>   $ perf record -a usleep 1
>>
>>   $ perf buildid-list
>>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>>
>>   $ perf buildid-list -H
>>   0000000000000000000000000000000000000000 [kernel.kallsyms]
>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>>
> There is something I don't understand in your example above.  The -H
> option shows only DSO with samples. So why do you get the buildid
> without -H and you get no buildid with -H? In other words, I don't
> connect the dots between what -H does on the buildid change for the
> kernel. Looks like you have the buildid in the perf.data file.

Without -H, it just prints all DSOs found in build-id table (rebuilt
during read perf data file header) and skips processing events.  But
with -H, it'd process the event records and so set kernel map to
'[kernel.kallsyms]' - since the kernel mmap event always has the name -
and mark it as hit.  Thus the actual vmlinux can't be marked and then
cannot be printed.

Hmm.. now I'm curious that why the -H option is needed at all.. the perf
record already wrote build-ids that are actually hits..

Thanks,
Namhyung

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Arnaldo Carvalho de Melo]

Em Fri, Sep 05, 2014 at 09:23:56AM +0900, Namhyung Kim escreveu:
> Hmm.. now I'm curious that why the -H option is needed at all.. the perf
> record already wrote build-ids that are actually hits..

Probably because when 'perf buildid-list' was introduced 'perf record'
was inserting all the PERF_RECORD_MMAP dsos into the buildid-list, not
just the ones with hits.

Then, later on, 'perf record' probably moved to register just the ones
with hits.

This way -H ends up being a nop, i.e. should produce the same result as
'perf buildid-list' with no options.

If it doesn't, then it is a bug.

- Arnaldo

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Arnaldo Carvalho de Melo]

Em Fri, Sep 05, 2014 at 09:09:49AM +0900, Namhyung Kim escreveu:
> >> Before:
> >>   $ perf record -a usleep 1

> >>   $ perf buildid-list
> >>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
> >>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so

> >>   $ perf buildid-list -H
> >>   0000000000000000000000000000000000000000 [kernel.kallsyms]
> >>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so

> >> After:
> >>   $ perf record -a usleep 1

> >>   $ perf buildid-list
> >>   00d5ff078efe1d30b8492854f259215fd877ce30 [kernel.kallsyms]

> > We are losing information, namely the pathname for the kernel used, that
> > may be useful in analysis.
 
> Right.  That's a problem.

> > Why not make sure that if there is a build-id in the perf.data header,
> > then we completely refusing anything that doesn't match the build-id?
> > I.e. the name is irrelevant for this purpose, the contents, as keyed by
> > the build-id, is what matters.
 
> The perf report rebuilds machine states from the event records only.  In
> this case, the kernel map was recorded in the name of [kernel.kallsyms]
> so it couldn't find the build-id from the table.

Ok, but then we can special case this one, no?

Somehow mark in the buildid table that that entry is the one for the
kernel and hook it up to the synthesized event that has the
[kernel.kallsyms].ref_reloc_sym entry.

- Arnaldo

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Namhyung Kim]

On Fri, Sep 5, 2014 at 10:18 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Fri, Sep 05, 2014 at 09:09:49AM +0900, Namhyung Kim escreveu:
>> The perf report rebuilds machine states from the event records only.  In
>> this case, the kernel map was recorded in the name of [kernel.kallsyms]
>> so it couldn't find the build-id from the table.
>
> Ok, but then we can special case this one, no?
>
> Somehow mark in the buildid table that that entry is the one for the
> kernel and hook it up to the synthesized event that has the
> [kernel.kallsyms].ref_reloc_sym entry.

Maybe we can search vmlinux in machine->kernel_dsos first when
processing kernel mmap event.  And in this case do you want replace
the name of mapping to a fullname of vmlinux?

Thanks,
Namhyung

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Namhyung Kim]

On Fri, Sep 5, 2014 at 10:42 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> On Fri, Sep 5, 2014 at 10:18 AM, Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
>> Em Fri, Sep 05, 2014 at 09:09:49AM +0900, Namhyung Kim escreveu:
>>> The perf report rebuilds machine states from the event records only.  In
>>> this case, the kernel map was recorded in the name of [kernel.kallsyms]
>>> so it couldn't find the build-id from the table.
>>
>> Ok, but then we can special case this one, no?
>>
>> Somehow mark in the buildid table that that entry is the one for the
>> kernel and hook it up to the synthesized event that has the
>> [kernel.kallsyms].ref_reloc_sym entry.
>
> Maybe we can search vmlinux in machine->kernel_dsos first when
> processing kernel mmap event.  And in this case do you want replace
> the name of mapping to a fullname of vmlinux?

Ah.. the name of mapping is not used at all after finding a matching
dso.  Ok then, I'll post a new patch to search vmlinux before
kallsyms.

Thanks
Namhyung

Re: [PATCH] perf tools: Fix build-id matching on vmlinux

[Stephane Eranian]

On Fri, Sep 5, 2014 at 2:23 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> Hi Stephane,
>
> On Thu, 4 Sep 2014 16:37:51 +0200, Stephane Eranian wrote:
>> On Tue, Aug 26, 2014 at 8:38 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>>>
>>> There's a problem on finding correct kernel symbols when perf report
>>> runs on a different kernel.  Although a part of the problem was solved
>>> by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel
>>> version when finding vmlinux"), there's a remaining problem still.
>>>
>>> When perf records samples, it synthesizes the kernel map using
>>> machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text".
>>> You can easily see it using 'perf report -D' command.
>>>
>>> After finishing record, it goes through the recorded events to find
>>> maps/dsos actually used.  And then record build-id info of them.
>>>
>>> During this process, it needs to load symbols in a dso and it'd call
>>> dso__load_vmlinux() since the default value of the symbol_conf.try_
>>> vmlinux_path is true.  However it changes dso->long_name to a real
>>> path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux)
>>> if one is running on a custom kernel.
>>>
>>> It resulted in that perf report reads the build-id of the vmlinux, but
>>> cannot use it since it only knows about the [kernel.kallsyms] map.  It
>>> then falls back to possible vmlinux paths by using the recorded kernel
>>> version (in case of a recent version) or a running kernel silently
>>> (which might break the result).  I think it's worth going to the
>>> stable tree.
>>>
>>> I can think of a couple of ways to fix it.  In this patch, I changed
>>> to use the name of "[kernel.kallsyms]" for the kernel build-id event
>>> instead of not trying vmlinux paths.  This way we can provide maximum
>>> info (like annotation) with minimum change IMHO.
>>>
>>> Before:
>>>
>>>   $ perf record -a usleep 1
>>>
>>>   $ perf buildid-list
>>>   00d5ff078efe1d30b8492854f259215fd877ce30 /lib/modules/3.16.0-rc2+/build/vmlinux
>>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>>>
>>>   $ perf buildid-list -H
>>>   0000000000000000000000000000000000000000 [kernel.kallsyms]
>>>   78186287bba77069a056a5ccbeb14b7fd2ca3a4b /usr/lib64/libc-2.17.so
>>>   4eadca6cb82e0a85edb87c15b5e3980742514501 /usr/lib64/ld-2.17.so
>>>   1e272ca30081e81ef41935a630eb2f4c636798b4 /usr/lib64/dri/swrast_dri.so
>>>   0000000000000000000000000000000000000000 /tmp/perf-2523.map
>>>
>> There is something I don't understand in your example above.  The -H
>> option shows only DSO with samples. So why do you get the buildid
>> without -H and you get no buildid with -H? In other words, I don't
>> connect the dots between what -H does on the buildid change for the
>> kernel. Looks like you have the buildid in the perf.data file.
>
> Without -H, it just prints all DSOs found in build-id table (rebuilt
> during read perf data file header) and skips processing events.  But
> with -H, it'd process the event records and so set kernel map to
> '[kernel.kallsyms]' - since the kernel mmap event always has the name -
> and mark it as hit.  Thus the actual vmlinux can't be marked and then
> cannot be printed.
>
Still don't follow this. You're saying because as part of processing the events,
you create or replace the mmap record corresponding to the kernel from the
synthesized mmap (actual kernel filename) to the generic kernel.kallsyms, you
lose the buildid. Why not just transfer it? It has to be the one
listed without -H.
This would certainly be much less confusing (to me at least)! Seems to me
you have one piece of information or the other (buildid or filename) but
never both.



> Hmm.. now I'm curious that why the -H option is needed at all.. the perf
> record already wrote build-ids that are actually hits..
>
> Thanks,
> Namhyung