[PATCH v3 0/9] perf: Default use of build IDs and improvements

Ian Rogers posted 9 patches 9 months, 2 weeks ago
tools/perf/bench/inject-buildid.c             |   2 +-
tools/perf/builtin-buildid-cache.c            |  20 ++--
tools/perf/builtin-buildid-list.c             |   6 +-
tools/perf/builtin-inject.c                   |  36 +++---
tools/perf/builtin-record.c                   |  33 ++++--
tools/perf/builtin-report.c                   |  11 +-
tools/perf/include/perf/perf_dlfilter.h       |   2 +-
tools/perf/tests/pe-file-parsing.c            |   2 +-
tools/perf/tests/sdt.c                        |   2 +-
tools/perf/tests/symbols.c                    |   4 +-
tools/perf/util/build-id.c                    |  63 ++++++----
tools/perf/util/build-id.h                    |   8 +-
tools/perf/util/debuginfo.c                   |   2 +-
tools/perf/util/disasm.c                      |   2 +-
tools/perf/util/dso.c                         | 111 ++++++++++--------
tools/perf/util/dso.h                         |  75 ++++++------
tools/perf/util/dsos.c                        |  20 ++--
tools/perf/util/event.c                       |   2 +-
tools/perf/util/header.c                      |   2 +-
tools/perf/util/jitdump.c                     |  21 +++-
tools/perf/util/machine.c                     |  34 +++---
tools/perf/util/map.c                         |  15 ++-
tools/perf/util/map.h                         |   5 +-
tools/perf/util/probe-event.c                 |   3 +-
tools/perf/util/probe-file.c                  |   4 +-
tools/perf/util/probe-finder.c                |   3 +-
.../scripting-engines/trace-event-python.c    |   7 +-
tools/perf/util/sort.c                        |  27 +++--
tools/perf/util/symbol-minimal.c              |   2 +-
tools/perf/util/symbol.c                      |   7 +-
tools/perf/util/symbol_conf.h                 |   2 +-
tools/perf/util/synthetic-events.c            |  44 ++++---
tools/perf/util/thread.c                      |   8 +-
tools/perf/util/thread.h                      |   2 +-
34 files changed, 337 insertions(+), 250 deletions(-)
[PATCH v3 0/9] perf: Default use of build IDs and improvements
Posted by Ian Rogers 9 months, 2 weeks ago
Build ID mmap2 events have been available since Linux v5.12 and avoid
certain races. Enable these by default as discussed in:
https://lore.kernel.org/linux-perf-users/CAP-5=fXP7jN_QrGUcd55_QH5J-Y-FCaJ6=NaHVtyx0oyNh8_-Q@mail.gmail.com/

The dso_id is used to indentify a DSO that may change by being
overwritten. The inode generation isn't present in /proc/pid/maps and
so was already only optionally filled in. With build ID mmap events
the other major, minor and inode varialbes aren't filled in. Change
the dso_id implementation to make optional values explicit, rather
than injecting a dso_id we want to improve it during find operations,
add the buildid to the dso_id for sorting and so that matching fails
when build IDs vary between DSOs.

Mark the callchain for buildids and not just the sample IP, fixing
missing DSOs.

Fix sample__for_each_callchain_node to populate the map even when
symbols aren't computed.

Other minor bits of build_id clean up.

v3: Ensure the struct build_id is initialized empty prior to use as
    read paths may fail (Namhyung).

v2: Make marking DSOs still the default even with the defaulted build
    ID mmap. The command line option still disables this to avoid
    regressions. Add callchain patches and jitdump fix.

Ian Rogers (9):
  perf callchain: Always populate the addr_location map when adding IP
  perf build-id: Reduce size of "size" variable
  perf build-id: Truncate to avoid overflowing the build_id data
  perf build-id: Change sprintf functions to snprintf
  perf build-id: Mark DSO in sample callchains
  perf build-id: Ensure struct build_id is empty before use
  perf dso: Move build_id to dso_id
  perf jitdump: Directly mark the jitdump DSO
  perf record: Make --buildid-mmap the default

 tools/perf/bench/inject-buildid.c             |   2 +-
 tools/perf/builtin-buildid-cache.c            |  20 ++--
 tools/perf/builtin-buildid-list.c             |   6 +-
 tools/perf/builtin-inject.c                   |  36 +++---
 tools/perf/builtin-record.c                   |  33 ++++--
 tools/perf/builtin-report.c                   |  11 +-
 tools/perf/include/perf/perf_dlfilter.h       |   2 +-
 tools/perf/tests/pe-file-parsing.c            |   2 +-
 tools/perf/tests/sdt.c                        |   2 +-
 tools/perf/tests/symbols.c                    |   4 +-
 tools/perf/util/build-id.c                    |  63 ++++++----
 tools/perf/util/build-id.h                    |   8 +-
 tools/perf/util/debuginfo.c                   |   2 +-
 tools/perf/util/disasm.c                      |   2 +-
 tools/perf/util/dso.c                         | 111 ++++++++++--------
 tools/perf/util/dso.h                         |  75 ++++++------
 tools/perf/util/dsos.c                        |  20 ++--
 tools/perf/util/event.c                       |   2 +-
 tools/perf/util/header.c                      |   2 +-
 tools/perf/util/jitdump.c                     |  21 +++-
 tools/perf/util/machine.c                     |  34 +++---
 tools/perf/util/map.c                         |  15 ++-
 tools/perf/util/map.h                         |   5 +-
 tools/perf/util/probe-event.c                 |   3 +-
 tools/perf/util/probe-file.c                  |   4 +-
 tools/perf/util/probe-finder.c                |   3 +-
 .../scripting-engines/trace-event-python.c    |   7 +-
 tools/perf/util/sort.c                        |  27 +++--
 tools/perf/util/symbol-minimal.c              |   2 +-
 tools/perf/util/symbol.c                      |   7 +-
 tools/perf/util/symbol_conf.h                 |   2 +-
 tools/perf/util/synthetic-events.c            |  44 ++++---
 tools/perf/util/thread.c                      |   8 +-
 tools/perf/util/thread.h                      |   2 +-
 34 files changed, 337 insertions(+), 250 deletions(-)

-- 
2.49.0.901.g37484f566f-goog
Re: [PATCH v3 0/9] perf: Default use of build IDs and improvements
Posted by Ian Rogers 8 months, 2 weeks ago
On Mon, Apr 28, 2025 at 2:34 PM Ian Rogers <irogers@google.com> wrote:
>
> Build ID mmap2 events have been available since Linux v5.12 and avoid
> certain races. Enable these by default as discussed in:
> https://lore.kernel.org/linux-perf-users/CAP-5=fXP7jN_QrGUcd55_QH5J-Y-FCaJ6=NaHVtyx0oyNh8_-Q@mail.gmail.com/
>
> The dso_id is used to indentify a DSO that may change by being
> overwritten. The inode generation isn't present in /proc/pid/maps and
> so was already only optionally filled in. With build ID mmap events
> the other major, minor and inode varialbes aren't filled in. Change
> the dso_id implementation to make optional values explicit, rather
> than injecting a dso_id we want to improve it during find operations,
> add the buildid to the dso_id for sorting and so that matching fails
> when build IDs vary between DSOs.
>
> Mark the callchain for buildids and not just the sample IP, fixing
> missing DSOs.
>
> Fix sample__for_each_callchain_node to populate the map even when
> symbols aren't computed.
>
> Other minor bits of build_id clean up.
>
> v3: Ensure the struct build_id is initialized empty prior to use as
>     read paths may fail (Namhyung).
>
> v2: Make marking DSOs still the default even with the defaulted build
>     ID mmap. The command line option still disables this to avoid
>     regressions. Add callchain patches and jitdump fix.

Ping. Thanks,
Ian

> Ian Rogers (9):
>   perf callchain: Always populate the addr_location map when adding IP
>   perf build-id: Reduce size of "size" variable
>   perf build-id: Truncate to avoid overflowing the build_id data
>   perf build-id: Change sprintf functions to snprintf
>   perf build-id: Mark DSO in sample callchains
>   perf build-id: Ensure struct build_id is empty before use
>   perf dso: Move build_id to dso_id
>   perf jitdump: Directly mark the jitdump DSO
>   perf record: Make --buildid-mmap the default
>
>  tools/perf/bench/inject-buildid.c             |   2 +-
>  tools/perf/builtin-buildid-cache.c            |  20 ++--
>  tools/perf/builtin-buildid-list.c             |   6 +-
>  tools/perf/builtin-inject.c                   |  36 +++---
>  tools/perf/builtin-record.c                   |  33 ++++--
>  tools/perf/builtin-report.c                   |  11 +-
>  tools/perf/include/perf/perf_dlfilter.h       |   2 +-
>  tools/perf/tests/pe-file-parsing.c            |   2 +-
>  tools/perf/tests/sdt.c                        |   2 +-
>  tools/perf/tests/symbols.c                    |   4 +-
>  tools/perf/util/build-id.c                    |  63 ++++++----
>  tools/perf/util/build-id.h                    |   8 +-
>  tools/perf/util/debuginfo.c                   |   2 +-
>  tools/perf/util/disasm.c                      |   2 +-
>  tools/perf/util/dso.c                         | 111 ++++++++++--------
>  tools/perf/util/dso.h                         |  75 ++++++------
>  tools/perf/util/dsos.c                        |  20 ++--
>  tools/perf/util/event.c                       |   2 +-
>  tools/perf/util/header.c                      |   2 +-
>  tools/perf/util/jitdump.c                     |  21 +++-
>  tools/perf/util/machine.c                     |  34 +++---
>  tools/perf/util/map.c                         |  15 ++-
>  tools/perf/util/map.h                         |   5 +-
>  tools/perf/util/probe-event.c                 |   3 +-
>  tools/perf/util/probe-file.c                  |   4 +-
>  tools/perf/util/probe-finder.c                |   3 +-
>  .../scripting-engines/trace-event-python.c    |   7 +-
>  tools/perf/util/sort.c                        |  27 +++--
>  tools/perf/util/symbol-minimal.c              |   2 +-
>  tools/perf/util/symbol.c                      |   7 +-
>  tools/perf/util/symbol_conf.h                 |   2 +-
>  tools/perf/util/synthetic-events.c            |  44 ++++---
>  tools/perf/util/thread.c                      |   8 +-
>  tools/perf/util/thread.h                      |   2 +-
>  34 files changed, 337 insertions(+), 250 deletions(-)
>
> --
> 2.49.0.901.g37484f566f-goog
>
Re: [PATCH v3 0/9] perf: Default use of build IDs and improvements
Posted by Arnaldo Carvalho de Melo 8 months, 2 weeks ago
On Tue, May 27, 2025 at 01:48:43PM -0700, Ian Rogers wrote:
> On Mon, Apr 28, 2025 at 2:34 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Build ID mmap2 events have been available since Linux v5.12 and avoid
> > certain races. Enable these by default as discussed in:
> > https://lore.kernel.org/linux-perf-users/CAP-5=fXP7jN_QrGUcd55_QH5J-Y-FCaJ6=NaHVtyx0oyNh8_-Q@mail.gmail.com/
> >
> > The dso_id is used to indentify a DSO that may change by being
> > overwritten. The inode generation isn't present in /proc/pid/maps and
> > so was already only optionally filled in. With build ID mmap events
> > the other major, minor and inode varialbes aren't filled in. Change
> > the dso_id implementation to make optional values explicit, rather
> > than injecting a dso_id we want to improve it during find operations,
> > add the buildid to the dso_id for sorting and so that matching fails
> > when build IDs vary between DSOs.
> >
> > Mark the callchain for buildids and not just the sample IP, fixing
> > missing DSOs.
> >
> > Fix sample__for_each_callchain_node to populate the map even when
> > symbols aren't computed.
> >
> > Other minor bits of build_id clean up.
> >
> > v3: Ensure the struct build_id is initialized empty prior to use as
> >     read paths may fail (Namhyung).
> >
> > v2: Make marking DSOs still the default even with the defaulted build
> >     ID mmap. The command line option still disables this to avoid
> >     regressions. Add callchain patches and jitdump fix.
> 
> Ping. Thanks,
> Ian

⬢ [acme@toolbx perf-tools-next]$        git am ./v3_20250428_irogers_perf_default_use_of_build_ids_and_improvements.mbx
Applying: perf callchain: Always populate the addr_location map when adding IP
Applying: perf build-id: Reduce size of "size" variable
Applying: perf build-id: Truncate to avoid overflowing the build_id data
Applying: perf build-id: Change sprintf functions to snprintf
Applying: perf build-id: Mark DSO in sample callchains
Applying: perf build-id: Ensure struct build_id is empty before use
Applying: perf dso: Move build_id to dso_id
Applying: perf jitdump: Directly mark the jitdump DSO
Applying: perf record: Make --buildid-mmap the default
error: patch failed: tools/perf/builtin-record.c:3349
error: tools/perf/builtin-record.c: patch does not apply
Patch failed at 0009 perf record: Make --buildid-mmap the default
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
⬢ [acme@toolbx perf-tools-next]$

- Arnaldo
Re: [PATCH v3 0/9] perf: Default use of build IDs and improvements
Posted by Ian Rogers 8 months, 2 weeks ago
On Wed, May 28, 2025 at 11:48 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Tue, May 27, 2025 at 01:48:43PM -0700, Ian Rogers wrote:
> > On Mon, Apr 28, 2025 at 2:34 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > Build ID mmap2 events have been available since Linux v5.12 and avoid
> > > certain races. Enable these by default as discussed in:
> > > https://lore.kernel.org/linux-perf-users/CAP-5=fXP7jN_QrGUcd55_QH5J-Y-FCaJ6=NaHVtyx0oyNh8_-Q@mail.gmail.com/
> > >
> > > The dso_id is used to indentify a DSO that may change by being
> > > overwritten. The inode generation isn't present in /proc/pid/maps and
> > > so was already only optionally filled in. With build ID mmap events
> > > the other major, minor and inode varialbes aren't filled in. Change
> > > the dso_id implementation to make optional values explicit, rather
> > > than injecting a dso_id we want to improve it during find operations,
> > > add the buildid to the dso_id for sorting and so that matching fails
> > > when build IDs vary between DSOs.
> > >
> > > Mark the callchain for buildids and not just the sample IP, fixing
> > > missing DSOs.
> > >
> > > Fix sample__for_each_callchain_node to populate the map even when
> > > symbols aren't computed.
> > >
> > > Other minor bits of build_id clean up.
> > >
> > > v3: Ensure the struct build_id is initialized empty prior to use as
> > >     read paths may fail (Namhyung).
> > >
> > > v2: Make marking DSOs still the default even with the defaulted build
> > >     ID mmap. The command line option still disables this to avoid
> > >     regressions. Add callchain patches and jitdump fix.
> >
> > Ping. Thanks,
> > Ian
>
> ⬢ [acme@toolbx perf-tools-next]$        git am ./v3_20250428_irogers_perf_default_use_of_build_ids_and_improvements.mbx
> Applying: perf callchain: Always populate the addr_location map when adding IP
> Applying: perf build-id: Reduce size of "size" variable
> Applying: perf build-id: Truncate to avoid overflowing the build_id data
> Applying: perf build-id: Change sprintf functions to snprintf
> Applying: perf build-id: Mark DSO in sample callchains
> Applying: perf build-id: Ensure struct build_id is empty before use
> Applying: perf dso: Move build_id to dso_id
> Applying: perf jitdump: Directly mark the jitdump DSO
> Applying: perf record: Make --buildid-mmap the default
> error: patch failed: tools/perf/builtin-record.c:3349
> error: tools/perf/builtin-record.c: patch does not apply
> Patch failed at 0009 perf record: Make --buildid-mmap the default
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config set advice.mergeConflict false"
> ⬢ [acme@toolbx perf-tools-next]$

Thanks, I'll send the rebase in v4. I saw you had a branch on
perf-tools-next.git.

Thanks,
Ian

> - Arnaldo