Re: [PATCH v5 0/6] perf annotate-data: Fix hangs on broken debug info, AMD mem record

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Sep 25, 2026 at 05:06:51PM +0200, Arnaldo Carvalho de Melo wrote:
> Hi,
> 
> This originated in another patch series, 'perf tools: Annotate fixes,
> stdio progress indication, debuginfo-client in more places', and is
> being split off so that these fixes can be reviewed right away; the
> debuginfod download feature work from it will come later, separately,
> based on this series.
> 
>   - 'perf report -s type' spins forever on the dwz compressed debug info of
>     zlib-ng (libz.so.1): die_collect_vars() saves a type DIE offset that is
>     relative to the file the DIE lives in, the dwz alt file for types shared
>     by several CUs, and resolving it in the main file parses whatever is at
>     that offset, here a typedef whose DW_AT_type refers to itself, making the
>     typedef/qualifier chase spin (patch 3), with the chases bounded so that
>     other kinds of broken debug info don't hang perf either (patches 1, 2
>     and 4);
> 
>   - 'perf mem record' requests PERF_SAMPLE_CPU (patch 5) and uses the IBS
>     swfilt filter when the kernel exposes it (patch 6), with the tables
>     carrying the term kept in the arch/x86 code, where the knowledge that
>     IBS needs it stays, as Ravi Bangoria suggested reviewing Namhyung
>     Kim's v6 review remark on this patch
>     (<4349c387-5b8a-4e8d-932a-5175a7598e1e@amd.com>, replying to
>     <aqsRVABj23jaaiSq@google.com>).
> 
> About PATCH 5, answering Namhyung Kim's v6 review question
> (<aqsPKGVUkWk6wvYA@google.com>) about the --sample-cpu default: the
> data source field is about the memory hierarchy level of the access,
> it has no record of which CPU issued it, and while the TID is in
> every sample and in the CTF stream, a thread time-sliced on one CPU
> or moved between SMT siblings is not told apart by it from cross-core
> contention, so it is the CPU id that keys it, and it is what the
> false-sharing detector in pahole needs.  The TID is recorded as well.
> 
> Requires elfutils 0.160 for dwarf_cu_getdwarf(), so the libdw feature test
> probes for it and Makefile.config says 0.160: older versions now disable
> dwarf support with that message instead of failing to link.

I have a nitpick on the patch 4, but otherwise looks good to me.

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> 
> Best regards,
> 
> - Arnaldo
> 
> What changed from v4:
> 
> - Added Ravi Bangoria's Reviewed-by (<76ef085b-e8e3-41a5-b029-2cd0489aa430@amd.com>),
>   given together with his rationale for keeping the IBS swfilt selection
>   explicit: 'perf record' fails rather than transparently retrying with
>   /swfilt=1/, so that the user is aware software filtering is being used
>   and that it is not overhead free.
> - Trimmed the comment on the AMD mem event tables in
>   tools/perf/arch/x86/util/mem-events.c per that review, dropping its
>   last sentence about how 'perf record' handles exclude bits on open
>   failure; the code is unchanged.
> - Patch 4: reworded the comment on the truncated flag; the JSON exporter
>   that reads it is added in a later series.
> - Patch 6: dropped a leftover no-op hunk from tools/perf/util/mem-events.c;
>   the swfilt selection is all in the arch/x86 code.
>   Command to see this delta: git diff eb568956a7129c91..HEAD
> 
> What changed from v3:
> 
> - Dropped patch 5/7, "perf annotate-data: Show the sample count in the
>   data-type browser": it is already in perf-tools-next as commit
>   9db4e9d6cc7c, so the series is now 6 patches and the subject no longer
>   mentions it.
> - Rebased onto current perf-tools-next (base-commit below; v3 was based on
>   29f320d221c1), patch 4 adapted to the upstream __add_member_cb() cleanup
>   that dropped the member_type local, no behavior change.
> 
> What changed from v2:
> 
> Only aggregate (struct/union) members are marked as truncated when the
> MAX_MEMBER_DEPTH limit is reached in the member nesting recursion (patch
> 4), addressing a sashiko [Medium] review finding
> (<20260921165704.38DBD1F000FF@smtp.kernel.org>): the v2 check was made
> before looking at the member's type, so a primitive field that merely
> landed on the limit, e.g. an int in a struct nested 31 deep, was marked
> as truncated and logged even though it has no children to expand.  The
> limit still bounds the recursion, as only aggregates are expanded.
> 
> What changed from v1:
> 
> Bump MAX_MEMBER_DEPTH from 8 to 32, addressing a review comment from Namhyung.
> 
>  tools/build/feature/test-libdw.c           |  10 ++-
>  tools/perf/Documentation/perf-mem.txt      |   4 +
>  tools/perf/Makefile.config                 |   2 +-
>  tools/perf/arch/x86/util/mem-events.c      |  17 ++++
>  tools/perf/arch/x86/util/mem-events.h      |   2 +
>  tools/perf/arch/x86/util/pmu.c             |  10 ++-
>  tools/perf/builtin-mem.c                   |   7 +-
>  tools/perf/tests/shell/test_data_symbol.sh |   6 +-
>  tools/perf/util/annotate-data.c            |  41 +++++++--
>  tools/perf/util/annotate-data.h            |   3 +
>  tools/perf/util/dwarf-aux.c                | 140 ++++++++++++++++++++++++-----
>  tools/perf/util/dwarf-aux.h                |  13 +++
>  12 files changed, 217 insertions(+), 38 deletions(-)
> 
> v4-head: eb568956a7129c91a8c0ccebb5979440a046c7ef
> v3-head: e2504280f5c4086e9851a758ed1bae8df7ef269e
> base-commit: edd8a9fe2eca009599e013a29c421c7a6b5ad1b9
> --