Re: [PATCH v6 1/8] perf: support specify vdso path in cmdline

[Adrian Hunter]

On 25/07/24 05:15, Changbin Du wrote:
> The vdso dumped from process memory (in buildid-cache) lacks debugging
> info. To annotate vdso symbols with source lines we need specify a
> debugging version.
> 
> For x86, we can find them from your local build as
> arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may reside in
> /lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that
> the buildid has to match.
> 
> $ sudo perf record -a
> $ sudo perf report --objdump=llvm-objdump \
>   --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg
> 
> Samples: 17K of event 'cycles:P', 4000 Hz, Event count (approx.): 1760
> __vdso_clock_gettime  /work/linux-host/arch/x86/entry/vdso/vdso64.so.d
> Percent│       movq    -48(%rbp),%rsi
>        │       testq   %rax,%rax
>        │     ;               return vread_hvclock();
>        │       movq    %rax,%rdx
>        │     ;               if (unlikely(!vdso_cycles_ok(cycles)))
>        │     ↑ js      eb
>        │     ↑ jmp     74
>        │     ;               ts->tv_sec = vdso_ts->sec;
>   0.02 │147:   leaq    2(%rbx),%rax
>        │       shlq    $4, %rax
>        │       addq    %r10,%rax
>        │     ;               while ((seq = READ_ONCE(vd->seq)) & 1) {
>   9.38 │152:   movl    (%r10),%ecx
> 
> When doing cross platform analysis, we also need specify the vdso path if
> we are interested in its symbols.
> 
> v2: update documentation.
> 
> Signed-off-by: Changbin Du <changbin.du@xxxxxxxxxx>
> ---
>  tools/perf/Documentation/perf-annotate.txt |  3 +
>  tools/perf/Documentation/perf-c2c.txt      |  3 +
>  tools/perf/Documentation/perf-inject.txt   |  3 +
>  tools/perf/Documentation/perf-report.txt   |  3 +
>  tools/perf/Documentation/perf-script.txt   |  3 +
>  tools/perf/Documentation/perf-top.txt      |  3 +
>  tools/perf/builtin-annotate.c              |  2 +
>  tools/perf/builtin-c2c.c                   |  2 +
>  tools/perf/builtin-inject.c                |  2 +
>  tools/perf/builtin-report.c                |  2 +
>  tools/perf/builtin-script.c                |  2 +
>  tools/perf/builtin-top.c                   |  2 +
>  tools/perf/util/disasm.c                   |  7 +-
>  tools/perf/util/symbol.c                   | 82 +++++++++++++++++++++-
>  tools/perf/util/symbol_conf.h              |  5 ++
>  15 files changed, 119 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> index b95524bea021..4b6692f9a793 100644
> --- a/tools/perf/Documentation/perf-annotate.txt
> +++ b/tools/perf/Documentation/perf-annotate.txt
> @@ -58,6 +58,9 @@ OPTIONS
>  --ignore-vmlinux::
>  	Ignore vmlinux files.
>  
> +--vdso=<vdso1[,vdso2]>::
> +	Specify vdso pathnames. You can specify up to two for multiarch-support.
> +
>  --itrace::
>  	Options for decoding instruction tracing data. The options are:
>  

<SNIP>

> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index b10b7f005658..e0aa657e6ca0 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -742,6 +742,8 @@ int cmd_annotate(int argc, const char **argv)
>  		   "file", "vmlinux pathname"),
>  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
>  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  	OPT_BOOLEAN('l', "print-line", &annotate_opts.print_lines,
>  		    "print matching source lines (may be slow)"),
>  	OPT_BOOLEAN('P', "full-paths", &annotate_opts.full_path,

<SNIP>

> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index e10558b79504..7e26d5215640 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -16,6 +16,7 @@
>  #include "debug.h"
>  #include "disasm.h"
>  #include "dso.h"
> +#include "vdso.h"
>  #include "env.h"
>  #include "evsel.h"
>  #include "map.h"
> @@ -1126,7 +1127,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  	if (pos && strlen(pos) < SBUILD_ID_SIZE - 2)
>  		dirname(build_id_path);
>  
> -	if (dso__is_kcore(dso))
> +	if (dso__is_kcore(dso) || dso__is_vdso(dso))

Sorry for very long delay.

This patch (probably this bit here) breaks annotation of vdso.
To allow for bisection, you need to arrange changes so that each
patch leaves things in a working state.

However, I disagree with adding --vdso option since with just
patch 8 alone, it would be possible to do:

  perf buildid-cache --remove /work/linux/arch/x86/entry/vdso/vdso64.so.dbg
  perf buildid-cache --add /work/linux/arch/x86/entry/vdso/vdso64.so.dbg

and same of vdso32.

That would leave the buildid-cache containing only the debug versions,
which would mean you will only get the debug versions, and it would only
need to be done once per kernel instead of having to add --vdso to
every perf command.