linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf script: Show binary offsets for userspace addr
@ 2021-10-19  7:24 Lexi Shao
  2021-10-20 13:24 ` Jiri Olsa
  0 siblings, 1 reply; 3+ messages in thread
From: Lexi Shao @ 2021-10-19  7:24 UTC (permalink / raw)
  To: linux-perf-users, linux-kernel
  Cc: acme, mark.rutland, peterz, mingo, alexander.shishkin, jolsa,
	namhyung, shaolexi, qiuxi1, nixiaoming, wangbing6

Show binary offsets for userspace addr with map in perf script output
with callchain.

In commit 19610184693c("perf script: Show virtual addresses instead of
offsets"), the addr shown in perf script output with callchain is changed
from binary offsets to virtual address to fix the incorrectness when
displaying symbol offset.

This is inconvenient in scenario that the binary is stripped and
symbol cannot be resolved. If someone wants to further resolve symbols for
specific binaries later, he would need an extra step to translate virtual
address to binary offset with mapping information recorded in perf.data,
which can be difficult for people not familiar with perf.

This patch modifies function sample__fprintf_callchain to print binary
offset for userspace addr with dsos, and virtual address otherwise. It
does not affect symbol offset calculation so symoff remains correct.

Before applying this patch:
test  1512    78.711307:     533129 cycles:
	aaaae0da07f4 [unknown] (/tmp/test)
	aaaae0da0704 [unknown] (/tmp/test)
	ffffbe9f7ef4 __libc_start_main+0xe4 (/lib64/libc-2.31.so)

After this patch:
test  1519   111.330127:     406953 cycles:
	7f4 [unknown] (/tmp/test)
	704 [unknown] (/tmp/test)
	20ef4 __libc_start_main+0xe4 (/lib64/libc-2.31.so)

Fixes: 19610184693c("perf script: Show virtual addresses instead of offsets")

Signed-off-by: Lexi Shao <shaolexi@huawei.com>
---
 tools/perf/util/evsel_fprintf.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index bfedd7b23521..8c2ea8001329 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -11,6 +11,7 @@
 #include "strlist.h"
 #include "symbol.h"
 #include "srcline.h"
+#include "dso.h"
 
 static int comma_fprintf(FILE *fp, bool *first, const char *fmt, ...)
 {
@@ -144,12 +145,17 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
 			if (print_arrow && !first)
 				printed += fprintf(fp, " <-");
 
-			if (print_ip)
-				printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
-
 			if (map)
 				addr = map->map_ip(map, node->ip);
 
+			if (print_ip) {
+				/* Show binary offset for userspace addr */
+				if (map && !map->dso->kernel)
+					printed += fprintf(fp, "%c%16" PRIx64, s, addr);
+				else
+					printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
+			}
+
 			if (print_sym) {
 				printed += fprintf(fp, " ");
 				node_al.addr = addr;
-- 
2.12.3


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

* Re: [PATCH] perf script: Show binary offsets for userspace addr
  2021-10-19  7:24 [PATCH] perf script: Show binary offsets for userspace addr Lexi Shao
@ 2021-10-20 13:24 ` Jiri Olsa
  2021-10-27 23:58   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2021-10-20 13:24 UTC (permalink / raw)
  To: Lexi Shao
  Cc: linux-perf-users, linux-kernel, acme, mark.rutland, peterz,
	mingo, alexander.shishkin, namhyung, qiuxi1, nixiaoming,
	wangbing6

On Tue, Oct 19, 2021 at 03:24:17PM +0800, Lexi Shao wrote:
> Show binary offsets for userspace addr with map in perf script output
> with callchain.
> 
> In commit 19610184693c("perf script: Show virtual addresses instead of
> offsets"), the addr shown in perf script output with callchain is changed
> from binary offsets to virtual address to fix the incorrectness when
> displaying symbol offset.
> 
> This is inconvenient in scenario that the binary is stripped and
> symbol cannot be resolved. If someone wants to further resolve symbols for
> specific binaries later, he would need an extra step to translate virtual
> address to binary offset with mapping information recorded in perf.data,
> which can be difficult for people not familiar with perf.
> 
> This patch modifies function sample__fprintf_callchain to print binary
> offset for userspace addr with dsos, and virtual address otherwise. It
> does not affect symbol offset calculation so symoff remains correct.
> 
> Before applying this patch:
> test  1512    78.711307:     533129 cycles:
> 	aaaae0da07f4 [unknown] (/tmp/test)
> 	aaaae0da0704 [unknown] (/tmp/test)
> 	ffffbe9f7ef4 __libc_start_main+0xe4 (/lib64/libc-2.31.so)
> 
> After this patch:
> test  1519   111.330127:     406953 cycles:
> 	7f4 [unknown] (/tmp/test)
> 	704 [unknown] (/tmp/test)
> 	20ef4 __libc_start_main+0xe4 (/lib64/libc-2.31.so)
> 
> Fixes: 19610184693c("perf script: Show virtual addresses instead of offsets")
> 
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>

makes sense to me

Acked-/Tested-by Jiri Olsa <jolsa@redhat.com>

should we have a way to display these callchain detail in perf report output as well?

thanks,
jirka

> ---
>  tools/perf/util/evsel_fprintf.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
> index bfedd7b23521..8c2ea8001329 100644
> --- a/tools/perf/util/evsel_fprintf.c
> +++ b/tools/perf/util/evsel_fprintf.c
> @@ -11,6 +11,7 @@
>  #include "strlist.h"
>  #include "symbol.h"
>  #include "srcline.h"
> +#include "dso.h"
>  
>  static int comma_fprintf(FILE *fp, bool *first, const char *fmt, ...)
>  {
> @@ -144,12 +145,17 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
>  			if (print_arrow && !first)
>  				printed += fprintf(fp, " <-");
>  
> -			if (print_ip)
> -				printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
> -
>  			if (map)
>  				addr = map->map_ip(map, node->ip);
>  
> +			if (print_ip) {
> +				/* Show binary offset for userspace addr */
> +				if (map && !map->dso->kernel)
> +					printed += fprintf(fp, "%c%16" PRIx64, s, addr);
> +				else
> +					printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
> +			}
> +
>  			if (print_sym) {
>  				printed += fprintf(fp, " ");
>  				node_al.addr = addr;
> -- 
> 2.12.3
> 


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

* Re: [PATCH] perf script: Show binary offsets for userspace addr
  2021-10-20 13:24 ` Jiri Olsa
@ 2021-10-27 23:58   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-10-27 23:58 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Lexi Shao, linux-perf-users, linux-kernel, mark.rutland, peterz,
	mingo, alexander.shishkin, namhyung, qiuxi1, nixiaoming,
	wangbing6

Em Wed, Oct 20, 2021 at 03:24:19PM +0200, Jiri Olsa escreveu:
> On Tue, Oct 19, 2021 at 03:24:17PM +0800, Lexi Shao wrote:
> > Show binary offsets for userspace addr with map in perf script output
> > with callchain.
> > 
> > In commit 19610184693c("perf script: Show virtual addresses instead of
> > offsets"), the addr shown in perf script output with callchain is changed
> > from binary offsets to virtual address to fix the incorrectness when
> > displaying symbol offset.
> > 
> > This is inconvenient in scenario that the binary is stripped and
> > symbol cannot be resolved. If someone wants to further resolve symbols for
> > specific binaries later, he would need an extra step to translate virtual
> > address to binary offset with mapping information recorded in perf.data,
> > which can be difficult for people not familiar with perf.
> > 
> > This patch modifies function sample__fprintf_callchain to print binary
> > offset for userspace addr with dsos, and virtual address otherwise. It
> > does not affect symbol offset calculation so symoff remains correct.
> > 
> > Before applying this patch:
> > test  1512    78.711307:     533129 cycles:
> > 	aaaae0da07f4 [unknown] (/tmp/test)
> > 	aaaae0da0704 [unknown] (/tmp/test)
> > 	ffffbe9f7ef4 __libc_start_main+0xe4 (/lib64/libc-2.31.so)
> > 
> > After this patch:
> > test  1519   111.330127:     406953 cycles:
> > 	7f4 [unknown] (/tmp/test)
> > 	704 [unknown] (/tmp/test)
> > 	20ef4 __libc_start_main+0xe4 (/lib64/libc-2.31.so)
> > 
> > Fixes: 19610184693c("perf script: Show virtual addresses instead of offsets")
> > 
> > Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> 
> makes sense to me
> 
> Acked-/Tested-by Jiri Olsa <jolsa@redhat.com>

Thanks, applied.

- Arnaldo

 
> should we have a way to display these callchain detail in perf report output as well?
> 
> thanks,
> jirka
> 
> > ---
> >  tools/perf/util/evsel_fprintf.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
> > index bfedd7b23521..8c2ea8001329 100644
> > --- a/tools/perf/util/evsel_fprintf.c
> > +++ b/tools/perf/util/evsel_fprintf.c
> > @@ -11,6 +11,7 @@
> >  #include "strlist.h"
> >  #include "symbol.h"
> >  #include "srcline.h"
> > +#include "dso.h"
> >  
> >  static int comma_fprintf(FILE *fp, bool *first, const char *fmt, ...)
> >  {
> > @@ -144,12 +145,17 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
> >  			if (print_arrow && !first)
> >  				printed += fprintf(fp, " <-");
> >  
> > -			if (print_ip)
> > -				printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
> > -
> >  			if (map)
> >  				addr = map->map_ip(map, node->ip);
> >  
> > +			if (print_ip) {
> > +				/* Show binary offset for userspace addr */
> > +				if (map && !map->dso->kernel)
> > +					printed += fprintf(fp, "%c%16" PRIx64, s, addr);
> > +				else
> > +					printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
> > +			}
> > +
> >  			if (print_sym) {
> >  				printed += fprintf(fp, " ");
> >  				node_al.addr = addr;
> > -- 
> > 2.12.3
> > 

-- 

- Arnaldo

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

end of thread, other threads:[~2021-10-27 23:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-19  7:24 [PATCH] perf script: Show binary offsets for userspace addr Lexi Shao
2021-10-20 13:24 ` Jiri Olsa
2021-10-27 23:58   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).