Re: [PATCHv3 00/25] perf: Add mmap2 build id support

[Namhyung Kim <namhyung@kernel.org>]

Hi Jiri,

On Tue, Nov 24, 2020 at 8:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> adding the support to have buildid stored in mmap2 event,
> so we can bypass the final perf record hunt on build ids.
>
> This patchset allows perf to record build ID in mmap2 event,
> and adds perf tooling to store/download binaries to .debug
> cache based on these build IDs.
>
> Note that the build id retrieval code is stolen from bpf
> code, where it's been used (together with file offsets)
> to replace IPs in user space stack traces. It's now added
> under lib directory.
>
> v3 changes:
>   - added acks
>   - removed forgotten debug code [Arnaldo]
>   - fixed readlink termination [Ian]
>   - fixed doc for --debuginfod=URLs [Ian]
>   - adopted kernel's memchr_inv function and used
>     it in build_id__is_defined function [Arnaldo]

[SNIP]
> ---
> Jiri Olsa (25):
>       bpf: Move stack_map_get_build_id into lib
>       bpf: Add size arg to build_id_parse function
>       perf: Add build id data in mmap2 event
>       tools headers uapi: Sync tools/include/uapi/linux/perf_event.h
>       tools lib: Adopt memchr_inv() from kernel
>       perf tools: Do not swap mmap2 fields in case it contains build id
>       perf tools: Add build_id__is_defined function
>       perf tools: Add filename__decompress function
>       perf tools: Add support to read build id from compressed elf
>       perf tools: Add check for existing link in buildid dir
>       perf tools: Use struct extra_kernel_map in machine__process_kernel_mmap_event
>       perf tools: Try to load vmlinux from buildid database
>       perf tools: Store build id from mmap2 events
>       perf tools: Allow mmap2 event to synthesize kernel image
>       perf tools: Allow mmap2 event to synthesize modules
>       perf tools: Synthesize build id for kernel/modules/tasks
>       perf tools: Add support to display build id for mmap2 events
>       perf tools: Use machine__for_each_dso in perf_session__cache_build_ids
>       perf tools: Add __perf_session__cache_build_ids function
>       perf tools: Add is_perf_data function
>       perf tools: Add build_id_cache__add function
>       perf buildid-cache: Add support to add build ids from perf data
>       perf buildid-cache: Add --debuginfod option
>       perf buildid-list: Add support for mmap2's buildid events
>       perf record: Add --buildid-mmap option to enable mmap's build id

Mostly looks good!

I only have some comments on the buildid-cache part.

Thanks,
Namhyung

>
>  include/linux/buildid.h                           |  12 +++++
>  include/uapi/linux/perf_event.h                   |  42 +++++++++++++++---
>  kernel/bpf/stackmap.c                             | 143 ++---------------------------------------------------------
>  kernel/events/core.c                              |  32 ++++++++++++--
>  lib/Makefile                                      |   3 +-
>  lib/buildid.c                                     | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/include/linux/string.h                      |   1 +
>  tools/include/uapi/linux/perf_event.h             |  42 +++++++++++++++---
>  tools/lib/perf/include/perf/event.h               |  18 ++++++--
>  tools/lib/string.c                                |  58 ++++++++++++++++++++++++
>  tools/perf/Documentation/perf-buildid-cache.txt   |  18 +++++++-
>  tools/perf/Documentation/perf-config.txt          |  10 ++++-
>  tools/perf/Documentation/perf-record.txt          |   3 ++
>  tools/perf/builtin-buildid-cache.c                | 243 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  tools/perf/builtin-buildid-list.c                 |   3 ++
>  tools/perf/builtin-record.c                       |  20 +++++++++
>  tools/perf/tests/shell/trace+probe_vfs_getname.sh |   2 +-
>  tools/perf/util/build-id.c                        | 127 ++++++++++++++++++++++++++++++++++-------------------
>  tools/perf/util/build-id.h                        |   8 ++++
>  tools/perf/util/data.c                            |  19 ++++++++
>  tools/perf/util/data.h                            |   1 +
>  tools/perf/util/dso.c                             |  31 ++++++++-----
>  tools/perf/util/dso.h                             |   2 +
>  tools/perf/util/event.c                           |  41 ++++++++++++-----
>  tools/perf/util/evsel.c                           |  10 +++--
>  tools/perf/util/machine.c                         |  80 ++++++++++++++++++++-------------
>  tools/perf/util/map.c                             |   8 +++-
>  tools/perf/util/map.h                             |   3 +-
>  tools/perf/util/perf_api_probe.c                  |  10 +++++
>  tools/perf/util/perf_api_probe.h                  |   1 +
>  tools/perf/util/perf_event_attr_fprintf.c         |   2 +
>  tools/perf/util/probe-event.c                     |   6 +--
>  tools/perf/util/record.h                          |   1 +
>  tools/perf/util/session.c                         |  11 +++--
>  tools/perf/util/symbol-elf.c                      |  37 +++++++++++++++-
>  tools/perf/util/symbol.c                          |  16 +++++++
>  tools/perf/util/symbol_conf.h                     |   3 +-
>  tools/perf/util/synthetic-events.c                | 121 +++++++++++++++++++++++++++++++++++++-------------
>  38 files changed, 1026 insertions(+), 311 deletions(-)
>  create mode 100644 include/linux/buildid.h
>  create mode 100644 lib/buildid.c
>