Received-SPF: pass (google.com: domain of linux-kernel+bounces-212979-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: duchangbin <changbin.du@huawei.com>
To: Adrian Hunter <adrian.hunter@intel.com>
CC: duchangbin <changbin.du@huawei.com>, Peter Zijlstra
	<peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, "Arnaldo Carvalho de
 Melo" <acme@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Nathan
 Chancellor <nathan@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa
	<jolsa@kernel.org>, "Ian Rogers" <irogers@google.com>, "Liang, Kan"
	<kan.liang@linux.intel.com>, "Nick Desaulniers" <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>, Justin Stitt <justinstitt@google.com>,
	"linux-perf-users@vger.kernel.org" <linux-perf-users@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"llvm@lists.linux.dev" <llvm@lists.linux.dev>
Subject: Re: [PATCH 1/2] perf: support specify vdso path in cmdline
Thread-Topic: [PATCH 1/2] perf: support specify vdso path in cmdline
Thread-Index: AQHavVvgPApptK0OsE6+S63iYUbWKrHE1RgAgACeKQA=
Date: Thu, 13 Jun 2024 09:49:48 +0000
Message-ID: <1e17854b16fe46a4b384b6ddec05029a@huawei.com>
References: <20240613063510.348692-1-changbin.du@huawei.com>
 <20240613063510.348692-2-changbin.du@huawei.com>
 <961fe800-7160-4c53-a3a0-f16700c433cd@intel.com>
In-Reply-To: <961fe800-7160-4c53-a3a0-f16700c433cd@intel.com>
Accept-Language: en-US
Content-Language: zh-CN
x-ms-exchange-messagesentrepresentingtype: 1
Content-Type: text/plain; charset="us-ascii"
Content-ID: <71BFC53452BD0846B1C74A7511D4179B@huawei.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0

On Thu, Jun 13, 2024 at 11:23:26AM +0300, Adrian Hunter wrote:
> On 13/06/24 09:35, 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.
> >=20
> > 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.
> >=20
> > $ sudo perf record -a
> > $ sudo perf report --objdump=3Dllvm-objdump \
> >   --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.s=
o.dbg
> >=20
> > When doing cross platform analysis, we also need specify the vdso path =
if
> > we are interested in its symbols.
>=20
> We already have logic to find debug files, and to deal with a symbol sour=
ce
> (syms_ss) and runtime symbol source (runtime_ss).
>=20
> Can't we make that work transparently for vdso so that the user does
> not have to.
>
For Ubuntu, we can find the debug files of vdso in
/lib/modules/<version>/vdso/vdso{32,64}.so. These two are debug version.

For local build, seems vdso{32,64}.so.dbg are not installed in /lib/modules=
/ nor
other locations.

> >=20
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> >  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/symbol.c      | 74 +++++++++++++++++++++++++++++++++++
> >  tools/perf/util/symbol_conf.h |  5 +++
> >  8 files changed, 91 insertions(+)
> >=20
> > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotat=
e.c
> > index 50d2fb222d48..ff466882065d 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,
> > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> > index c157bd31f2e5..4764f9139661 100644
> > --- a/tools/perf/builtin-c2c.c
> > +++ b/tools/perf/builtin-c2c.c
> > @@ -3018,6 +3018,8 @@ static int perf_c2c__report(int argc, const char =
**argv)
> >  	const struct option options[] =3D {
> >  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
> >  		   "file", "vmlinux pathname"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_STRING('i', "input", &input_name, "file",
> >  		   "the input file to process"),
> >  	OPT_INCR('N', "node-info", &c2c.node_info,
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index a212678d47be..e774e83d0a0f 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -2247,6 +2247,8 @@ int cmd_inject(int argc, const char **argv)
> >  			    "don't load vmlinux even if found"),
> >  		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
> >  			   "kallsyms pathname"),
> > +		OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  		OPT_BOOLEAN('f', "force", &data.force, "don't complain, do it"),
> >  		OPT_CALLBACK_OPTARG(0, "itrace", &inject.itrace_synth_opts,
> >  				    NULL, "opts", "Instruction Tracing options\n"
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 69618fb0110b..a64b48460dce 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -1324,6 +1324,8 @@ int cmd_report(int argc, const char **argv)
> >                      "don't load vmlinux even if found"),
> >  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
> >  		   "file", "kallsyms pathname"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"=
),
> >  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
> >  		    "load module symbols - WARNING: use only with -k and LIVE kernel=
"),
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index c16224b1fef3..2e358922a8d1 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -3965,6 +3965,8 @@ int cmd_script(int argc, const char **argv)
> >  		   "file", "vmlinux pathname"),
> >  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
> >  		   "file", "kallsyms pathname"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
> >  		    "When printing symbols do not display call chain"),
> >  	OPT_CALLBACK(0, "symfs", NULL, "directory",
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index 1d6aef51c122..a3cce4e76eb9 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -1479,6 +1479,8 @@ int cmd_top(int argc, const char **argv)
> >  		   "file", "kallsyms pathname"),
> >  	OPT_BOOLEAN('K', "hide_kernel_symbols", &top.hide_kernel_symbols,
> >  		    "hide kernel symbols"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_CALLBACK('m', "mmap-pages", &opts->mmap_pages, "pages",
> >  		     "number of mmap data pages", evlist__parse_mmap_pages),
> >  	OPT_INTEGER('r', "realtime", &top.realtime_prio,
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 9e5940b5bc59..8d040039a7ce 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -19,6 +19,7 @@
> >  #include "build-id.h"
> >  #include "cap.h"
> >  #include "dso.h"
> > +#include "vdso.h"
> >  #include "util.h" // lsdir()
> >  #include "debug.h"
> >  #include "event.h"
> > @@ -44,6 +45,7 @@
> > =20
> >  static int dso__load_kernel_sym(struct dso *dso, struct map *map);
> >  static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map=
);
> > +static int dso__load_vdso_sym(struct dso *dso, struct map *map);
> >  static bool symbol__is_idle(const char *name);
> > =20
> >  int vmlinux_path__nr_entries;
> > @@ -1833,6 +1835,12 @@ int dso__load(struct dso *dso, struct map *map)
> >  		goto out;
> >  	}
> > =20
> > +	if (dso__is_vdso(dso)) {
> > +		ret =3D dso__load_vdso_sym(dso, map);
> > +		if (ret > 0)
> > +			goto out;
> > +	}
> > +
> >  	dso__set_adjust_symbols(dso, false);
> > =20
> >  	if (perfmap) {
> > @@ -2349,6 +2357,72 @@ static int vmlinux_path__init(struct perf_env *e=
nv)
> >  	return -1;
> >  }
> > =20
> > +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
> > +			 const char *arg, int unset __maybe_unused)
> > +{
> > +	char *tmp, *tok, *str =3D strdup(arg);
> > +	unsigned int i =3D 0;
> > +
> > +	for (tok =3D strtok_r(str, ",", &tmp); tok && i < ARRAY_SIZE(symbol_c=
onf.vdso_name);
> > +	     tok =3D strtok_r(NULL, ",", &tmp)) {
> > +		symbol_conf.vdso_name[i++] =3D strdup(tok);
> > +	}
> > +
> > +	free(str);
> > +	return 0;
> > +}
> > +
> > +static int dso__load_vdso(struct dso *dso, struct map *map,
> > +			  const char *vdso)
> > +{
> > +	int err =3D -1;
> > +	struct symsrc ss;
> > +	char symfs_vdso[PATH_MAX];
> > +
> > +	if (vdso[0] =3D=3D '/')
> > +		snprintf(symfs_vdso, sizeof(symfs_vdso), "%s", vdso);
> > +	else
> > +		symbol__join_symfs(symfs_vdso, vdso);
> > +
> > +	if (symsrc__init(&ss, dso, symfs_vdso, DSO_BINARY_TYPE__SYSTEM_PATH_D=
SO))
> > +		return -1;
> > +
> > +	/*
> > +	 * dso__load_sym() may copy 'dso' which will result in the copies hav=
ing
> > +	 * an incorrect long name unless we set it here first.
> > +	 */
> > +	dso__set_long_name(dso, vdso, false);
> > +	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
> > +
> > +	err =3D dso__load_sym(dso, map, &ss, &ss, 0);
> > +	symsrc__destroy(&ss);
> > +
> > +	if (err > 0) {
> > +		dso__set_loaded(dso);
> > +		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(ds=
o));
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int dso__load_vdso_sym(struct dso *dso, struct map *map)
> > +{
> > +	int ret;
> > +
> > +	if (!dso__is_vdso(dso))
> > +		return -1;
> > +
> > +	for (unsigned int i =3D 0; i < ARRAY_SIZE(symbol_conf.vdso_name); i++=
) {
> > +		if (symbol_conf.vdso_name[i] !=3D NULL) {
> > +			ret =3D dso__load_vdso(dso, map, symbol_conf.vdso_name[i]);
> > +			if (ret > 0)
> > +				return ret;
> > +		}
> > +	}
> > +
> > +	return -1;
> > +}
> > +
> >  int setup_list(struct strlist **list, const char *list_str,
> >  		      const char *list_name)
> >  {
> > diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_con=
f.h
> > index c114bbceef40..108356e3c981 100644
> > --- a/tools/perf/util/symbol_conf.h
> > +++ b/tools/perf/util/symbol_conf.h
> > @@ -3,6 +3,7 @@
> >  #define __PERF_SYMBOL_CONF 1
> > =20
> >  #include <stdbool.h>
> > +#include <subcmd/parse-options.h>
> > =20
> >  struct strlist;
> >  struct intlist;
> > @@ -55,6 +56,7 @@ struct symbol_conf {
> >  	const char	*default_guest_vmlinux_name,
> >  			*default_guest_kallsyms,
> >  			*default_guest_modules;
> > +	const char	*vdso_name[2];
> >  	const char	*guestmount;
> >  	const char	*dso_list_str,
> >  			*comm_list_str,
> > @@ -85,4 +87,7 @@ struct symbol_conf {
> > =20
> >  extern struct symbol_conf symbol_conf;
> > =20
> > +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
> > +			 const char *arg, int unset __maybe_unused);
> > +
> >  #endif // __PERF_SYMBOL_CONF
>=20
>=20

--=20
Cheers,
Changbin Du