Re: [PATCH] btf: Add support for the floating-point types

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Sun, 2021-03-07 at 11:09 -0300, Arnaldo Carvalho de Melo wrote:
> Adding Jiri to the CC list.
> 
> Em Sun, Mar 07, 2021 at 10:50:19AM -0300, Arnaldo Carvalho de Melo
> escreveu:
> > Em Sun, Mar 07, 2021 at 10:44:21AM -0300, Arnaldo Carvalho de Melo
> > escreveu:
> > > Em Sun, Mar 07, 2021 at 10:16:31AM -0300, Arnaldo Carvalho de Melo
> > > escreveu:
> > > > Em Sat, Mar 06, 2021 at 07:16:08PM -0800, Andrii Nakryiko
> > > > escreveu:
> > > > > On Fri, Mar 5, 2021 at 6:22 PM Ilya Leoshkevich
> > > > > <iii@linux.ibm.com> wrote:
> > > > > > 
> > > > > > Some BPF programs compiled on s390 fail to load, because s390
> > > > > > arch-specific linux headers contain float and double types.
> > > > > > 
> > > > > > Fix as follows:
> > > > > > 
> > > > > > - Make DWARF loader fill base_type.float_type.
> > > > > > 
> > > > > > - libbpf introduced support for the floating-point types in
> > > > > > commit
> > > > > >   986962fade5, so update the libbpf submodule to that version
> > > > > > and use
> > > > > >   the new btf__add_float() function in order to emit the
> > > > > > floating-point
> > > > > >   types when base_type.float_type is set.
> > > > > > 
> > > > > > Example of the resulting entry in the vmlinux BTF:
> > > > > > 
> > > > > >     [7164] FLOAT 'double' size=8
> > > 
> > > > > [PATCH dwarves] would make it a bit clearer that this is pahole
> > > > > patch.
> > > 
> > > > > But LGTM.
> > >  
> > > > So older versions of bpftool will fail with a .BTF section having
> > > > this
> > > > new float? I thought it would just skip it emitting a warning?
> > > > Probably
> > > > not possible as we don't have the record size encoded in a
> > > > header,
> > > > right?
> > >  
> > > > [acme@five pahole]$ bpftool btf dump file vmlinux | grep -w FLOAT
> > > > [acme@five pahole]$ type pahole
> > > > pahole is /home/acme/bin/pahole
> > > > [acme@five pahole]$ ls -la ~/bin/pahole
> > > > lrwxrwxrwx. 1 acme acme 34 Jan 29 11:00 /home/acme/bin/pahole ->
> > > > /home/acme/git/pahole/build/pahole
> > > > [acme@five pahole]$ pahole -J vmlinux
> > > > [acme@five pahole]$ bpftool btf dump file vmlinux | grep -w FLOAT
> > > > | head
> > > > Error: failed to load BTF from vmlinux: Invalid argument
> > > > [acme@five pahole]$
> > > > 
> > > > Perhaps the warning emitted by bpftool should suggest updating
> > > > the tool
> > > > as it found a record type it doesn't know about?
> > > > 
> > > > /me goes to update bpftool...
> > > 
> > > Works with the bpftool in bpf-next:
> > > 
> > > [acme@five pahole]$ bpftool btf dump file vmlinux | grep -w FLOAT |
> > > head
> > > [8006] FLOAT 'double' size=8
> > > [acme@five pahole]$
> > 
> > Applied, with this committer notes:
> > 
> > Committer testing:
> > 
> >   $ rm -rf build  # To update the libbpf git submodule
> >   $ mkdir build
> >   $ cd build
> >   $ cmake ..
> >   $ cd ..
> >   $ make -C build
> >   # No BTF_KIND_FLOAT before:
> >   $ bpftool btf dump file vmlinux | grep -w FLOAT
> >   $ type pahole
> >   pahole is /home/acme/bin/pahole
> >   $ ls -la ~/bin/pahole
> >   lrwxrwxrwx. 1 acme acme 34 Jan 29 11:00 /home/acme/bin/pahole ->
> > /home/acme/git/pahole/build/pahole
> >   # Encode BTF:
> >   $ pahole -J vmlinux
> >   $ bpftool btf dump file vmlinux | grep -w FLOAT | head
> >   Error: failed to load BTF from vmlinux: Invalid argument
> >   $
> >   # Update bpftool to what is in bpf-next, then try again:
> >   $ bpftool btf dump file vmlinux | grep -w FLOAT
> >   [8006] FLOAT 'double' size=8
> >   $
> >   # Now check that pahole works well, i.e. that the BTF loader works
> >   $ pahole -F btf vmlinux -C sk_buff_head
> >   struct sk_buff_head {
> >         struct sk_buff *           next;                 /*     0    
> > 8 */
> >         struct sk_buff *           prev;                 /*     8    
> > 8 */
> >         __u32                      qlen;                 /*    16    
> > 4 */
> >         spinlock_t                 lock;                 /*    20    
> > 4 */
> > 
> >         /* size: 24, cachelines: 1, members: 4 */
> >         /* last cacheline: 24 bytes */
> >   };
> >   $
> >   $ pahole -F btf vmlinux  | wc -l
> >   122676
> >   $
> > 
> > Now will build a kernel with this new version, reboot, then push
> > publicly.
> 
> So now trying to build v5.12-rc2 with pahole supporting BTF_KIND_FLOAT:
> 
>   AS      .tmp_vmlinux.kallsyms2.S
>   LD      vmlinux
>   BTFIDS  vmlinux
> FAILED: load BTF from vmlinux: Invalid argument
> make[1]: *** [/home/acme/git/linux/Makefile:1197: vmlinux] Error 255
> make[1]: Leaving directory '/home/acme/git/build/v5.12.0-rc2'
> make: *** [Makefile:215: __sub-make] Error 2
> [acme@five linux]$
> 
> [acme@five linux]$ egrep BTF\|DWARF  ../build/v5.12.0-rc2/.config
> CONFIG_VIDEO_SONY_BTF_MPX=m
> CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
> # CONFIG_DEBUG_INFO_DWARF4 is not set
> CONFIG_DEBUG_INFO_BTF=y
> CONFIG_PAHOLE_HAS_SPLIT_BTF=y
> CONFIG_DEBUG_INFO_BTF_MODULES=y
> [acme@five linux]$
> 
> Ideas?
> 
> - Arnaldo

So v5.12-rc2 does not have this series yet:

https://lore.kernel.org/bpf/20210226202256.116518-1-iii@linux.ibm.com/

pahole generates a BTF_KIND_FLOAT, but libbpf from v5.12-rc2 doesn't 
know how to handle it and resolve_btfids fails.

I guess this is the first time a new BTF kind is added? I checked the
history, and kernel v5.2, which introduced DEBUG_INFO_BTF, already had
BTF_KIND_DATASEC.

So should I add a command-line option to pahole, which would tell it
the desired libbpf compatibility level?