On Tue, Feb 26, 2019 at 9:25 AM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 2/26/19 9:22 AM, Andrii Nakryiko wrote:
> > 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?
>
> float is not an "int". Encoding "float" in INT_KIND does not sound right
> in my opinion.
So isn't "bool". But in the end, it's just a collection of bits that
are interpreted according to some encoding based on convention. From
application perspective, none of float, int or bool has any more
structure, those are "opaque" values. So maybe name BTF_KIND_INT is a
bit unfortunate, it should be more like BTF_KIND_PRIMITIVE or
something (or in pahole's lingo: base type).
>
> >
> >>
> >>>
> >>>>
> >>>>>
> >>>>> Though for the latter, it probably would be better to resolve base
> >>>>> type size using BTF data itself.
> >>>>>
> >>>>>>> [acme@quaco pahole]$
