On Mon, 2023-03-13 at 17:32 -0300, Arnaldo Carvalho de Melo wrote:
[...]
> > @ -72998,8 +73022,8 @ struct sock {
> > /* --- cacheline 19 boundary (1216 bytes) --- */
> > int (*sk_backlog_rcv)(struct sock *, struct sk_buff *); /* 1216 8 */
> > void (*sk_destruct)(struct sock *); /* 1224 8 */
> > - rcu * sk_reuseport_cb; /* 1232 8 */
> > - rcu * sk_bpf_storage; /* 1240 8 */
> > + <ERROR > sk_reuseport_cb; /* 1232 8 */
> > + <ERROR > sk_bpf_storage; /* 1240 8 */
> >
> > (Also DWARF names refer to TYPE_TAG, not actual type name, fixed in pahole-new).
> >
> > Warn #1: pahole-next complains about unexpected child tags generated
> > by clang, e.g.:
> >
> > die__create_new_tag: unspecified_type WITH children!
> > die__create_new_base_type: DW_TAG_base_type WITH children!
Hi Arnaldo,
> Sure, we can remove those if the children is the expected one, leaving
> the warning maybe for debug sessions.
Yes, I handle it in the pahole-new. Warnings are not reported for
DW_TAG_LLVM_annotation children when these are applicable.
>
> Thanks for the detailed implementation notes, references and tests
> performed, please consider breaking it up into smaller pieces or
> ellaborate on why you think can't be done.
I can split the patch into several parts,
it would be a bit stretched, though:
1. change in btf_loader / dwarves_fprintf to correctly print names for
types with type tags;
2. consolidation of `struct btf_type_tag_type` and `struct llvm_annotation`;
3. the logic to handle "btf:type_tags";
4. support for "void __tag*" as unspecified type.
Part #3 would still be quite big, +500 LoC or something like this.
Will submit v2 today or tomorrow.
Thanks,
Eduard
>
> Thanks!
>
> - Arnaldo
>
> >
> > Performance impact
> > ------------------
> >
> > The update to `struct tag` might raise concerns regarding memory
> > usage, additional steps in recode phase might raise concerns regarding
> > execution time. Below is statistics collected for Kernel BTF
> > generation.
> >
> > LLVM-new / LLVM-new / pahole-new:
> >
> > $ /usr/bin/time -v pahole -J --btf_gen_floats -j --lang_exclude=rust .tmp_vmlinux.btf
> > ...
> > User time (seconds): 22.29
> > System time (seconds): 0.47
> > Percent of CPU this job got: 483%
> > ...
> > Maximum resident set size (kbytes): 714524
> > ...
> >
> > LLVM-new / LLVM-new / pahole-next:
> >
> > $ /usr/bin/time -v pahole -J --btf_gen_floats -j --lang_exclude=rust .tmp_vmlinux.btf
> > ...
> > User time (seconds): 20.96
> > System time (seconds): 0.44
> > Percent of CPU this job got: 473%
> > ...
> > Maximum resident set size (kbytes): 700848
> > ...
> >
> > Links & revisions
> > -----------------
> >
> > [1] Mailing list discussion regarding `btf:type_tag`
> > https://lore.kernel.org/bpf/87r0w9jjoq.fsf@xxxxxxxxxx/
> > [2] Suggestion to use btfdiff
> > https://lore.kernel.org/dwarves/ZAKpZGSHTvsS4r8E@xxxxxxxxxx/T/#mddbfe661e339485fb2b0e706b31329b46bf61bda
> > [3] f759275c1c8e ("[AMDGPU] Regenerate sdwa-peephole.ll")
> > [4] a9498899109d ("dwarf_loader: Support for btf:type_tag")
> > [5] 49b5300f1f8f ("Merge branch 'Support stashing local kptrs with bpf_kptr_xchg'")
> > [6] LLVM changes to generate btf:type_tag, revisions stack:
> > https://reviews.llvm.org/D143966
> > https://reviews.llvm.org/D143967
> > https://reviews.llvm.org/D145891
> >
> > Eduard Zingerman (1):
> > dwarf_loader: Support for btf:type_tag
> >
> > btf_encoder.c | 13 +-
> > btf_loader.c | 15 +-
> > dwarf_loader.c | 763 +++++++++++++++++++++++++++++++++++++---------
> > dwarves.c | 1 +
> > dwarves.h | 68 +++--
> > dwarves_fprintf.c | 13 +
> > 6 files changed, 693 insertions(+), 180 deletions(-)
> >
> > --
> > 2.39.1
> >
>
