On Mon, Feb 25, 2019 at 9:38 PM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 2/25/19 1:36 PM, Andrii Nakryiko wrote:
> > On Mon, Feb 25, 2019 at 12:51 PM Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:
> >>
> >> On Mon, Feb 25, 2019 at 12:08 PM Arnaldo Carvalho de Melo
> >> <arnaldo.melo@xxxxxxxxx> wrote:
> >>>
> >>> Em Mon, Feb 25, 2019 at 11:42:29AM -0800, Andrii Nakryiko escreveu:
> >>>> On Mon, Feb 25, 2019 at 11:00 AM Arnaldo Carvalho de Melo> <arnaldo.melo@xxxxxxxxx> wrote:
> >>>
> >>>>> Em Mon, Feb 25, 2019 at 01:02:39PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>>>>> So yeah, the BTF encoder/decoder is working just fine, the problem is in
> >>>>>> pahole's DWARF code, lemme see...
> >>>>>
> >>>>> Please try the patch below, for me btfdiff continues to show no diff for
> >>>>> all types in my vmlinux and now it also produces the same output for
> >>>>> when the first element of a bitfield has its bit_size equal to its
> >>>>> byte_size * 8:
> >>>>
> >>>> Yes, this fixes all the issues I've seen. btfdiff output is now empty
> >>>> for my kernel image. Thanks for quick fix!
> >>>>
> >>>> Reviewed-by: Andrii Nakryiko <andriin@xxxxxx>
> >>>
> >>> Thanks, just for some extra testing I tried encoding BTF for glibc 2.28:
> >>>
> >>> [acme@quaco pahole]$ pahole -JV libc-2.28.so.debug
> >>> <BIG snip>
> >>
> >> <snip>
> >>
> >>> [386075] INT long unsigned int size=8 bit_offset=0 nr_bits=64 encoding=(none)
> >>> Cannot open libc-2.28.so.debug
> >>> Failed to encode BTF
> >>
> >> I've tried the same on libc-2.26.so.debug and it worked.
> >>
> >> It failes after BTF encoding and deduping is done, exactly when it
> >> tries to re-open libc-2.28.so.debug to write out new contents. Not
> >> sure why it would fail, as it seems to be just plain open() call, but
> >> the fact that we both can repro this means there is something there.
> >>
> >> It also seems that libc-2.28.so.debug is corrupted:
> >>
> >> $ readelf -n /home/andriin/local/btf/libc-2.26.so.debug | grep
> >> 'Unknown note type' | wc -l
> >> 0
> >> $ readelf -n /home/andriin/local/btf/libc-2.28.so.debug | grep
> >> 'Unknown note type' | wc -l
> >> 17953
> >>
> >
> > Ignore everything I've said (though 'Unknown note type' issue is
> > strange), for me it was just permissions issues.
> >
> > $ chmod +rw /home/andriin/local/btf/libc-2.28.so.debug
> >
> > $ ~/local/pahole/build/pahole -J ~/local/btf/libc-2.28.so.debug
> >
> > $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
> > base_type__name_to_size: base_type _Float128
> > class__fixup_btf_bitfields: unknown base type name "_Float128"!
> > base_type__name_to_size: base_type _Float128
> > class__fixup_btf_bitfields: unknown base type name "_Float128"!
> > base_type__name_to_size: base_type _Float128
> > class__fixup_btf_bitfields: unknown base type name "_Float128"!
> > base_type__name_to_size: base_type _Float128
> > class__fixup_btf_bitfields: unknown base type name "_Float128"!
> > base_type__name_to_size: base_type _Float128
> > class__fixup_btf_bitfields: unknown base type name "_Float128"!
> > base_type__name_to_size: base_type _Float128
> > class__fixup_btf_bitfields: unknown base type name "_Float128"!
> > --- /tmp/btfdiff.dwarf.aV4wSL 2019-02-25 13:31:54.787923673 -0800
> > +++ /tmp/btfdiff.btf.22NQmJ 2019-02-25 13:31:54.802923668 -0800
> > @@ -461,9 +461,15 @@ struct La_x86_64_retval {
> > uint64_t lrv_rdx; /* 8 8 */
> > La_x86_64_xmm lrv_xmm0; /* 16 16 */
> > La_x86_64_xmm lrv_xmm1; /* 32 16 */
> > - long double lrv_st0; /* 48 16 */
> > + long double lrv_st0; /* 48 8 */
> > +
> > + /* XXX 8 bytes hole, try to pack */
> > +
> > /* --- cacheline 1 boundary (64 bytes) --- */
> > - long double lrv_st1; /* 64 16 */
> > + long double lrv_st1; /* 64 8 */
> > +
> > + /* XXX 8 bytes hole, try to pack */
> > +
> > La_x86_64_vector lrv_vector0; /* 80 64 */
> > /* --- cacheline 2 boundary (128 bytes) was 16 bytes ago --- */
> > La_x86_64_vector lrv_vector1; /* 144 64 */
> > @@ -472,6 +478,7 @@ struct La_x86_64_retval {
> > __int128 lrv_bnd1; /* 224 16 */
> >
> > /* size: 240, cachelines: 4, members: 10 */
> > + /* sum members: 224, holes: 2, sum holes: 16 */
> > /* last cacheline: 48 bytes */
> > };
> > struct r_debug {
> > @@ -2044,7 +2051,7 @@ union ieee754_float {
> > } ieee_nan; /* 0 4 */
> > };
> > union ieee854_long_double {
> > - long double d; /* 0 16 */
> > + long double d; /* 0 8 */
> > struct {
> > unsigned int mantissa1:32; /* 0: 0 4 */
> > unsigned int mantissa0:32; /* 4: 0 4 */
> > @@ -2141,7 +2148,7 @@ struct ucontext_t {
> > /* last cacheline: 8 bytes */
> > };
> > union ieee854_float128 {
> > - _Float128 d; /* 0 16 */
> > + _Float128 d; /* 0 0 */
> > struct {
> > unsigned int mantissa3:32; /* 0: 0 4 */
> > unsigned int mantissa2:32; /* 4: 0 4 */
> > @@ -2219,7 +2226,7 @@ union printf_arg {
> > long unsigned int pa_u_long_int; /* 0 8 */
> > long long unsigned int pa_u_long_long_int; /* 0 8 */
> > double pa_double; /* 0 8 */
> > - long double pa_long_double; /* 0 16 */
> > + long double pa_long_double; /* 0 8 */
> > const char * pa_string; /* 0 8 */
> > const wchar_t * pa_wstring; /* 0 8 */
> > void * pa_pointer; /* 0 8 */
> >
> > Seems like "long double" size is invalid for BTF. And we should
> > probably add "_Float128" as another "well known" base type name?
>
> BPF does not support floating point instructions. As the result,
> BTF design does not contain floating point types.
>
> Looks like kernel also uses float types (in certain drivers and arch's),
> but types referred by bpf program mostly will not contain float types.
>
> Currently, both llvm and pahole will ignore floating types, so
> from type comparison point of view, we will be fine.
> The issue may come up with people using vmlinux BTF inside the
> kernel for different purpose where a float type needs to be
> specified, e.g., for a floating point function argument in ftrace.
> But this is rare too.
>
> I am not 100% sure whether pahole dwarf/btf parity is considered
> as a strong reason to add floating point type support in BTF or not.
Isn't float and double just another base type with corresponding name?
Fundamentally, how is it different from int or long? It's still just a
collection of bits in 32-bit, 64-bit, etc number, except interpreted
differently. Do you mean that for BTF to properly support floats,
we'll need to extends set of possible encodings in INT type (in
addition to today's CHAR, SIGNED and UNSIGNED)?
>
> >
> > Though for the latter, it probably would be better to resolve base
> > type size using BTF data itself.
> >
> >>> [acme@quaco pahole]$
