* [PATCH] perf callchain: Pass relative address to hist entry
@ 2017-10-25 2:12 Namhyung Kim
2017-10-25 8:34 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2017-10-25 2:12 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
Milian Wolff
The addr_location->addr should have relative address to be passed to
hist entry so that it can compare with others which might come from a
different address space.
The callchain_cursor_node->ip already has a relative address thus it
should not map it again. But I found a bug handling cumulative
(children) entries. For chilren entries that has no self period, the
al->addr (so he->ip) ends up having an doubly-mapped address.
It seems to be there from the beginning but only affects entries that
have no srclines - finding srcline itself is done using a different
address but it will show the invalid address if no srcline was found.
Tested-by: Milian Wolff <milian.wolff@kdab.com>
Fixes: c7405d85d7a3 ("perf tools: Update cpumode for each cumulative entry")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/callchain.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 3a3916934a92..837012147c7b 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1091,10 +1091,7 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
al->map = node->map;
al->sym = node->sym;
al->srcline = node->srcline;
- if (node->map)
- al->addr = node->map->map_ip(node->map, node->ip);
- else
- al->addr = node->ip;
+ al->addr = node->ip;
if (al->sym == NULL) {
if (hide_unresolved)
--
2.14.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf callchain: Pass relative address to hist entry
2017-10-25 2:12 [PATCH] perf callchain: Pass relative address to hist entry Namhyung Kim
@ 2017-10-25 8:34 ` Jiri Olsa
2017-10-25 8:48 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2017-10-25 8:34 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
LKML, kernel-team, Milian Wolff
On Wed, Oct 25, 2017 at 11:12:09AM +0900, Namhyung Kim wrote:
> The addr_location->addr should have relative address to be passed to
> hist entry so that it can compare with others which might come from a
> different address space.
>
> The callchain_cursor_node->ip already has a relative address thus it
> should not map it again. But I found a bug handling cumulative
> (children) entries. For chilren entries that has no self period, the
> al->addr (so he->ip) ends up having an doubly-mapped address.
>
> It seems to be there from the beginning but only affects entries that
> have no srclines - finding srcline itself is done using a different
> address but it will show the invalid address if no srcline was found.
>
> Tested-by: Milian Wolff <milian.wolff@kdab.com>
> Fixes: c7405d85d7a3 ("perf tools: Update cpumode for each cumulative entry")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
hum, I'm getting differences in hist entries for:
$ perf record --all-user -ga sleep 1
$ perf perf report --stdio > ...
...
$ diff -puw /tmp/o.old /tmp/o.new
jirka
---
@@ -51,7 +51,7 @@
|
--0.94%--0x3
- 3.38% 0.00% Web Content libxul.so [.] 0xffff8087f8e046a5
+ 3.38% 0.00% Web Content libxul.so [.] 0x00000000031c16a5
|
---0x31c16a5
|
@@ -91,7 +91,7 @@
|
--1.62%--malloc
- 2.47% 0.00% Compositor libxul.so [.] 0xffff80e6f2807790
+ 2.47% 0.00% Compositor libxul.so [.] 0x0000000000b9b790
|
---0xb9b790
|
@@ -163,7 +163,7 @@
|
---0x72be62f5fdc55d00
- 1.05% 0.00% Compositor libxul.so [.] 0xffff80e6f280f100
+ 1.05% 0.00% Compositor libxul.so [.] 0x0000000000ba3100
|
---0xba3100
|
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] perf callchain: Pass relative address to hist entry
2017-10-25 8:34 ` Jiri Olsa
@ 2017-10-25 8:48 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2017-10-25 8:48 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
LKML, kernel-team, Milian Wolff
Hi Jiri,
On Wed, Oct 25, 2017 at 10:34:32AM +0200, Jiri Olsa wrote:
> On Wed, Oct 25, 2017 at 11:12:09AM +0900, Namhyung Kim wrote:
> > The addr_location->addr should have relative address to be passed to
> > hist entry so that it can compare with others which might come from a
> > different address space.
> >
> > The callchain_cursor_node->ip already has a relative address thus it
> > should not map it again. But I found a bug handling cumulative
> > (children) entries. For chilren entries that has no self period, the
> > al->addr (so he->ip) ends up having an doubly-mapped address.
> >
> > It seems to be there from the beginning but only affects entries that
> > have no srclines - finding srcline itself is done using a different
> > address but it will show the invalid address if no srcline was found.
> >
> > Tested-by: Milian Wolff <milian.wolff@kdab.com>
> > Fixes: c7405d85d7a3 ("perf tools: Update cpumode for each cumulative entry")
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> hum, I'm getting differences in hist entries for:
>
> $ perf record --all-user -ga sleep 1
> $ perf perf report --stdio > ...
> ...
> $ diff -puw /tmp/o.old /tmp/o.new
I'd like to say that the old address is invalid since user space
function address should not be started with 0xffff...
The last paragraph of the commit message should say that it also
affects entries don't have symbols.
Thanks,
Namhyung
>
>
> jirka
>
>
> ---
> @@ -51,7 +51,7 @@
> |
> --0.94%--0x3
>
> - 3.38% 0.00% Web Content libxul.so [.] 0xffff8087f8e046a5
> + 3.38% 0.00% Web Content libxul.so [.] 0x00000000031c16a5
> |
> ---0x31c16a5
> |
> @@ -91,7 +91,7 @@
> |
> --1.62%--malloc
>
> - 2.47% 0.00% Compositor libxul.so [.] 0xffff80e6f2807790
> + 2.47% 0.00% Compositor libxul.so [.] 0x0000000000b9b790
> |
> ---0xb9b790
> |
> @@ -163,7 +163,7 @@
> |
> ---0x72be62f5fdc55d00
>
> - 1.05% 0.00% Compositor libxul.so [.] 0xffff80e6f280f100
> + 1.05% 0.00% Compositor libxul.so [.] 0x0000000000ba3100
> |
> ---0xba3100
> |
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-25 8:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-25 2:12 [PATCH] perf callchain: Pass relative address to hist entry Namhyung Kim
2017-10-25 8:34 ` Jiri Olsa
2017-10-25 8:48 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox