All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: fix symbols__fixup_end heuristic for corner cases
@ 2017-03-15 21:53 Daniel Borkmann
  2017-03-16  1:07 ` Arnaldo Carvalho de Melo
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Daniel Borkmann @ 2017-03-15 21:53 UTC (permalink / raw)
  To: acme; +Cc: ast, linux-kernel, Daniel Borkmann

The current symbols__fixup_end() heuristic for the last entry in the
rb tree is suboptimal as it leads to not being able to recognize the
symbol in the call graph in a couple of corner cases, for example:

 i) If the symbol has a start address (f.e. exposed via kallsyms)
    that is at a page boundary, then the roundup(curr->start, 4096)
    for the last entry will result in curr->start == curr->end with
    a symbol length of zero.

ii) If the symbol has a start address that is shortly before a page
    boundary, then also here, curr->end - curr->start will just be
    very few bytes, where it's unrealistic that we could perform a
    match against.

Instead, change the heuristic to roundup(curr->start, 4096) + 4096,
so that we can catch such corner cases and have a better chance to
find that specific symbol. It's still just best effort as the real
end of the symbol is unknown to us (and could even be at a larger
offset than the current range), but better than the current situation.

Alexei reported that he recently run into case i) with a JITed eBPF
program (these are all page aligned) as the last symbol which wasn't
properly shown in the call graph (while other eBPF program symbols
in the rb tree were displayed correctly). Since this is a generic
issue, lets try to improve the heuristic a bit.

Fixes: 2e538c4a1847 ("perf tools: Improve kernel/modules symbol lookup")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/perf/util/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 70e389b..9b4d8ba 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -202,7 +202,7 @@ void symbols__fixup_end(struct rb_root *symbols)
 
 	/* Last entry */
 	if (curr->end == curr->start)
-		curr->end = roundup(curr->start, 4096);
+		curr->end = roundup(curr->start, 4096) + 4096;
 }
 
 void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
-- 
1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: fix symbols__fixup_end heuristic for corner cases
  2017-03-15 21:53 [PATCH] perf: fix symbols__fixup_end heuristic for corner cases Daniel Borkmann
@ 2017-03-16  1:07 ` Arnaldo Carvalho de Melo
  2017-03-16  9:07   ` Daniel Borkmann
  2017-03-16 18:08 ` Alexei Starovoitov
  2017-03-17 14:19 ` [tip:perf/urgent] perf symbols: Fix " tip-bot for Daniel Borkmann
  2 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-16  1:07 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: ast, linux-kernel

Em Wed, Mar 15, 2017 at 10:53:37PM +0100, Daniel Borkmann escreveu:
> The current symbols__fixup_end() heuristic for the last entry in the
> rb tree is suboptimal as it leads to not being able to recognize the
> symbol in the call graph in a couple of corner cases, for example:

Thanks, will apply, test and push to Ingo via perf/urgent, tomorrow.

- Arnaldo
 
>  i) If the symbol has a start address (f.e. exposed via kallsyms)
>     that is at a page boundary, then the roundup(curr->start, 4096)
>     for the last entry will result in curr->start == curr->end with
>     a symbol length of zero.
> 
> ii) If the symbol has a start address that is shortly before a page
>     boundary, then also here, curr->end - curr->start will just be
>     very few bytes, where it's unrealistic that we could perform a
>     match against.
> 
> Instead, change the heuristic to roundup(curr->start, 4096) + 4096,
> so that we can catch such corner cases and have a better chance to
> find that specific symbol. It's still just best effort as the real
> end of the symbol is unknown to us (and could even be at a larger
> offset than the current range), but better than the current situation.
> 
> Alexei reported that he recently run into case i) with a JITed eBPF
> program (these are all page aligned) as the last symbol which wasn't
> properly shown in the call graph (while other eBPF program symbols
> in the rb tree were displayed correctly). Since this is a generic
> issue, lets try to improve the heuristic a bit.
> 
> Fixes: 2e538c4a1847 ("perf tools: Improve kernel/modules symbol lookup")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  tools/perf/util/symbol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 70e389b..9b4d8ba 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -202,7 +202,7 @@ void symbols__fixup_end(struct rb_root *symbols)
>  
>  	/* Last entry */
>  	if (curr->end == curr->start)
> -		curr->end = roundup(curr->start, 4096);
> +		curr->end = roundup(curr->start, 4096) + 4096;
>  }
>  
>  void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: fix symbols__fixup_end heuristic for corner cases
  2017-03-16  1:07 ` Arnaldo Carvalho de Melo
@ 2017-03-16  9:07   ` Daniel Borkmann
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2017-03-16  9:07 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: ast, linux-kernel

On 03/16/2017 02:07 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 15, 2017 at 10:53:37PM +0100, Daniel Borkmann escreveu:
>> The current symbols__fixup_end() heuristic for the last entry in the
>> rb tree is suboptimal as it leads to not being able to recognize the
>> symbol in the call graph in a couple of corner cases, for example:
>
> Thanks, will apply, test and push to Ingo via perf/urgent, tomorrow.

Great, thanks Arnaldo!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: fix symbols__fixup_end heuristic for corner cases
  2017-03-15 21:53 [PATCH] perf: fix symbols__fixup_end heuristic for corner cases Daniel Borkmann
  2017-03-16  1:07 ` Arnaldo Carvalho de Melo
@ 2017-03-16 18:08 ` Alexei Starovoitov
  2017-03-17 14:19 ` [tip:perf/urgent] perf symbols: Fix " tip-bot for Daniel Borkmann
  2 siblings, 0 replies; 5+ messages in thread
From: Alexei Starovoitov @ 2017-03-16 18:08 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: acme, ast, linux-kernel

On Wed, Mar 15, 2017 at 10:53:37PM +0100, Daniel Borkmann wrote:
> The current symbols__fixup_end() heuristic for the last entry in the
> rb tree is suboptimal as it leads to not being able to recognize the
> symbol in the call graph in a couple of corner cases, for example:
> 
>  i) If the symbol has a start address (f.e. exposed via kallsyms)
>     that is at a page boundary, then the roundup(curr->start, 4096)
>     for the last entry will result in curr->start == curr->end with
>     a symbol length of zero.
> 
> ii) If the symbol has a start address that is shortly before a page
>     boundary, then also here, curr->end - curr->start will just be
>     very few bytes, where it's unrealistic that we could perform a
>     match against.
> 
> Instead, change the heuristic to roundup(curr->start, 4096) + 4096,
> so that we can catch such corner cases and have a better chance to
> find that specific symbol. It's still just best effort as the real
> end of the symbol is unknown to us (and could even be at a larger
> offset than the current range), but better than the current situation.
> 
> Alexei reported that he recently run into case i) with a JITed eBPF
> program (these are all page aligned) as the last symbol which wasn't
> properly shown in the call graph (while other eBPF program symbols
> in the rb tree were displayed correctly). Since this is a generic
> issue, lets try to improve the heuristic a bit.
> 
> Fixes: 2e538c4a1847 ("perf tools: Improve kernel/modules symbol lookup")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Acked-by: Alexei Starovoitov <ast@kernel.org>
Tested-by: Alexei Starovoitov <ast@kernel.org>

thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:perf/urgent] perf symbols: Fix symbols__fixup_end heuristic for corner cases
  2017-03-15 21:53 [PATCH] perf: fix symbols__fixup_end heuristic for corner cases Daniel Borkmann
  2017-03-16  1:07 ` Arnaldo Carvalho de Melo
  2017-03-16 18:08 ` Alexei Starovoitov
@ 2017-03-17 14:19 ` tip-bot for Daniel Borkmann
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Daniel Borkmann @ 2017-03-17 14:19 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: daniel, acme, ast, mingo, tglx, linux-kernel, hpa

Commit-ID:  e7ede72a6d40cb3a30c087142d79381ca8a31dab
Gitweb:     http://git.kernel.org/tip/e7ede72a6d40cb3a30c087142d79381ca8a31dab
Author:     Daniel Borkmann <daniel@iogearbox.net>
AuthorDate: Wed, 15 Mar 2017 22:53:37 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Mar 2017 10:30:22 -0300

perf symbols: Fix symbols__fixup_end heuristic for corner cases

The current symbols__fixup_end() heuristic for the last entry in the rb
tree is suboptimal as it leads to not being able to recognize the symbol
in the call graph in a couple of corner cases, for example:

 i) If the symbol has a start address (f.e. exposed via kallsyms)
    that is at a page boundary, then the roundup(curr->start, 4096)
    for the last entry will result in curr->start == curr->end with
    a symbol length of zero.

ii) If the symbol has a start address that is shortly before a page
    boundary, then also here, curr->end - curr->start will just be
    very few bytes, where it's unrealistic that we could perform a
    match against.

Instead, change the heuristic to roundup(curr->start, 4096) + 4096, so
that we can catch such corner cases and have a better chance to find
that specific symbol. It's still just best effort as the real end of the
symbol is unknown to us (and could even be at a larger offset than the
current range), but better than the current situation.

Alexei reported that he recently run into case i) with a JITed eBPF
program (these are all page aligned) as the last symbol which wasn't
properly shown in the call graph (while other eBPF program symbols in
the rb tree were displayed correctly). Since this is a generic issue,
lets try to improve the heuristic a bit.

Reported-and-Tested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Fixes: 2e538c4a1847 ("perf tools: Improve kernel/modules symbol lookup")
Link: http://lkml.kernel.org/r/bb5c80d27743be6f12afc68405f1956a330e1bc9.1489614365.git.daniel@iogearbox.net
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 70e389b..9b4d8ba 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -202,7 +202,7 @@ void symbols__fixup_end(struct rb_root *symbols)
 
 	/* Last entry */
 	if (curr->end == curr->start)
-		curr->end = roundup(curr->start, 4096);
+		curr->end = roundup(curr->start, 4096) + 4096;
 }
 
 void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-17 14:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-15 21:53 [PATCH] perf: fix symbols__fixup_end heuristic for corner cases Daniel Borkmann
2017-03-16  1:07 ` Arnaldo Carvalho de Melo
2017-03-16  9:07   ` Daniel Borkmann
2017-03-16 18:08 ` Alexei Starovoitov
2017-03-17 14:19 ` [tip:perf/urgent] perf symbols: Fix " tip-bot for Daniel Borkmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.