* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-08-26 6:38 [PATCH] perf tools: Fix build-id matching on vmlinux Namhyung Kim
@ 2014-09-04 2:44 ` Namhyung Kim
2014-09-04 14:18 ` Arnaldo Carvalho de Melo
2014-09-04 14:37 ` Stephane Eranian
2 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2014-09-04 2:44 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Adrian Hunter, Andi Kleen,
Stephane Eranian, stable
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;
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-08-26 6:38 [PATCH] perf tools: Fix build-id matching on vmlinux Namhyung Kim
2014-09-04 2:44 ` Namhyung Kim
@ 2014-09-04 14:18 ` Arnaldo Carvalho de Melo
2014-09-05 0:09 ` Namhyung Kim
2014-09-04 14:37 ` Stephane Eranian
2 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-09-04 14:18 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Adrian Hunter, Andi Kleen,
Stephane Eranian, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-04 14:18 ` Arnaldo Carvalho de Melo
@ 2014-09-05 0:09 ` Namhyung Kim
2014-09-05 1:18 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2014-09-05 0:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Adrian Hunter, Andi Kleen,
Stephane Eranian, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-05 0:09 ` Namhyung Kim
@ 2014-09-05 1:18 ` Arnaldo Carvalho de Melo
2014-09-05 1:42 ` Namhyung Kim
0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-09-05 1:18 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Adrian Hunter, Andi Kleen,
Stephane Eranian, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-05 1:18 ` Arnaldo Carvalho de Melo
@ 2014-09-05 1:42 ` Namhyung Kim
2014-09-05 2:30 ` Namhyung Kim
0 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2014-09-05 1:42 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Adrian Hunter, Andi Kleen,
Stephane Eranian, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-05 1:42 ` Namhyung Kim
@ 2014-09-05 2:30 ` Namhyung Kim
0 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2014-09-05 2:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Adrian Hunter, Andi Kleen,
Stephane Eranian, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-08-26 6:38 [PATCH] perf tools: Fix build-id matching on vmlinux Namhyung Kim
2014-09-04 2:44 ` Namhyung Kim
2014-09-04 14:18 ` Arnaldo Carvalho de Melo
@ 2014-09-04 14:37 ` Stephane Eranian
2014-09-05 0:23 ` Namhyung Kim
2 siblings, 1 reply; 11+ messages in thread
From: Stephane Eranian @ 2014-09-04 14:37 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Namhyung Kim, LKML, Jiri Olsa, David Ahern,
Adrian Hunter, Andi Kleen, stable
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-04 14:37 ` Stephane Eranian
@ 2014-09-05 0:23 ` Namhyung Kim
2014-09-05 1:13 ` Arnaldo Carvalho de Melo
2014-09-05 3:19 ` Stephane Eranian
0 siblings, 2 replies; 11+ messages in thread
From: Namhyung Kim @ 2014-09-05 0:23 UTC (permalink / raw)
To: Stephane Eranian
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Namhyung Kim, LKML, Jiri Olsa, David Ahern,
Adrian Hunter, Andi Kleen, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-05 0:23 ` Namhyung Kim
@ 2014-09-05 1:13 ` Arnaldo Carvalho de Melo
2014-09-05 3:19 ` Stephane Eranian
1 sibling, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-09-05 1:13 UTC (permalink / raw)
To: Namhyung Kim
Cc: Stephane Eranian, Peter Zijlstra, Ingo Molnar, Paul Mackerras,
Namhyung Kim, LKML, Jiri Olsa, David Ahern, Adrian Hunter,
Andi Kleen, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf tools: Fix build-id matching on vmlinux
2014-09-05 0:23 ` Namhyung Kim
2014-09-05 1:13 ` Arnaldo Carvalho de Melo
@ 2014-09-05 3:19 ` Stephane Eranian
1 sibling, 0 replies; 11+ messages in thread
From: Stephane Eranian @ 2014-09-05 3:19 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Namhyung Kim, LKML, Jiri Olsa, David Ahern,
Adrian Hunter, Andi Kleen, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread