On Mon, Feb 25, 2019 at 10:03 PM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 2/25/19 9:45 PM, Andrii Nakryiko wrote:
> > 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>
> >>>>
<snip>
> >>>
> >>> 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)?
>
> Probably another kind like BTF_KIND_FLOAT with additional u32 (in
> addition to btf_type) indicating the size (4 bytes, 8 bytes and 16 bytes).
This sounds exactly like current format of BTF_KIND_INT. What
BTF_KIND_FLOAT will have that can't be represented by BTF_KIND_INT?
Wouldn't it be equivalent to just adding BTF_INT_FLOAT in addition to
existing BTF_INT_SIGNED, BTF_INT_CHAR, BTF_INT_BOOL to
BTF_INT_ENCODING? Btw, while BTF_INT_SIGNED seems like it deserves
separate bit, because it's mostly orthogonal to CHAR vs INT vs FLOAT
vs (not so much) BOOL, BTF_INT_CHAR and BTF_INT_BOOL take two
independent bits seems excessive. You can't have BOOL and CHAR, right?
So why don't we just repurpose bits 1 and 2 to be a small 4-value
enum:
enum btf_int_encoding {
INT = 0,
CHAR = 1,
BOOL = 2,
FLOAT = 3,
};
This is backwards compatible and covers all floats (long double vs
double vs float is just a question of bitness).
Am I missing something here?
>
> >
> >>
> >>>
> >>> Though for the latter, it probably would be better to resolve base
> >>> type size using BTF data itself.
> >>>
> >>>>> [acme@quaco pahole]$
