Re: [PATCH RESEND] Add a "-m" option to "perf buildid-list".

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Jun 07, 2022 at 12:15:50PM -0700, Blake Jones wrote:
> This new option displays all of the information needed to do external
> BuildID-based symbolization of kernel stack traces, such as those collected
> by bpf_get_stackid(). For each kernel module plus the main kernel, it
> displays the BuildID, the start and end virtual addresses of that module's
> text range (rounded out to page boundaries), and the pathname of the
> module.
> 
> When run as a non-privileged user, the actual addresses of the modules'
> text ranges are not available, so the tools displays "0, <text length>" for
> kernel modules and "0, ffffffffffffffff" for the kernel itself.
> 
> This patch has been rebased as of this afternoon, but is otherwise
> identical to the version I sent out last month.
> 
> Sample output:
> 
> root# perf buildid-list -m
> cf6df852fd4da122d616153353cc8f560fd12fe0 ffffffffa5400000 ffffffffa6001e27 [kernel.kallsyms]
> 1aa7209aa2acb067d66ed6cf7676d65066384d61 ffffffffc0087000 ffffffffc008b000 /lib/modules/5.15.15-1rodete2-amd64/kernel/crypto/sha512_generic.ko
> 3857815b5bf0183697b68f8fe0ea06121644041e ffffffffc008c000 ffffffffc0098000 /lib/modules/5.15.15-1rodete2-amd64/kernel/arch/x86/crypto/sha512-ssse3.ko
> 4081fde0bca2bc097cb3e9d1efcb836047d485f1 ffffffffc0099000 ffffffffc009f000 /lib/modules/5.15.15-1rodete2-amd64/kernel/drivers/acpi/button.ko
> 1ef81ba4890552ea6b0314f9635fc43fc8cef568 ffffffffc00a4000 ffffffffc00aa000 /lib/modules/5.15.15-1rodete2-amd64/kernel/crypto/cryptd.ko
> cc5c985506cb240d7d082b55ed260cbb851f983e ffffffffc00af000 ffffffffc00b6000 /lib/modules/5.15.15-1rodete2-amd64/kernel/drivers/i2c/busses/i2c-piix4.ko
> [...]
> 
> Signed-off-by: Blake Jones <blakejones@google.com>
> ---
>  .../perf/Documentation/perf-buildid-list.txt  |  3 ++
>  tools/perf/builtin-buildid-list.c             | 33 ++++++++++++++++++-
>  tools/perf/util/dso.c                         | 23 +++++++++++++
>  tools/perf/util/dso.h                         |  2 ++
>  tools/perf/util/machine.c                     | 13 ++++++++
>  tools/perf/util/machine.h                     |  5 +++
>  6 files changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/Documentation/perf-buildid-list.txt b/tools/perf/Documentation/perf-buildid-list.txt
> index 25c52efcc7f0..a82e92c2667d 100644
> --- a/tools/perf/Documentation/perf-buildid-list.txt
> +++ b/tools/perf/Documentation/perf-buildid-list.txt
> @@ -33,6 +33,9 @@ OPTIONS
>  -k::
>  --kernel::
>  	Show running kernel build id.
> +-m::
> +--modules::
> +	Show buildid, start/end text address, and path of kernel and modules.

why 'modules' ? it shows all maps (including kernel)
so perhaps -m/--maps would be better?

also please state that it's from running kernel

SNIP

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 5ac13958d1bd..7c490b5ce449 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1227,6 +1227,19 @@ void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated)
>  	dso__set_long_name_id(dso, name, NULL, name_allocated);
>  }
>  
> +void dso__get_long_name(const struct dso *dso, char *name, size_t len)
> +{
> +	if (len == 0)
> +		return;
> +	if (dso->long_name == NULL) {
> +		name[0] = '\0';
> +		return;
> +	}
> +
> +	strncpy(name, dso->long_name, len);
> +	name[len - 1] = '\0';
> +}
> +
>  void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated)
>  {
>  	if (name == NULL)
> @@ -1359,6 +1372,16 @@ void dso__set_build_id(struct dso *dso, struct build_id *bid)
>  	dso->has_build_id = 1;
>  }
>  
> +void dso__get_build_id(struct dso *dso, struct build_id *bid)
> +{
> +	if (dso->has_build_id) {
> +		*bid = dso->bid;
> +	} else {
> +		bid->size = 0;
> +		memset(bid->data, 0, sizeof(*bid->data));
> +	}
> +}

any reason why not use the dso fields directly?

thanks,
jirka