Re: [PATCH dwarves 0/1] Support for new btf_type_tag encoding

[Eduard Zingerman <eddyz87@gmail.com>]

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@oracle.com/
> > [2] Suggestion to use btfdiff
> >     https://lore.kernel.org/dwarves/ZAKpZGSHTvsS4r8E@kernel.org/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
> > 
>