bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btf: Add support for the floating-point types
@ 2021-03-06  2:22 Ilya Leoshkevich
  2021-03-07  3:16 ` Andrii Nakryiko
  0 siblings, 1 reply; 9+ messages in thread
From: Ilya Leoshkevich @ 2021-03-06  2:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: dwarves, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Yonghong Song, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich

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

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 btf_loader.c   | 21 +++++++++++++++++++--
 dwarf_loader.c | 11 +++++++++++
 lib/bpf        |  2 +-
 libbtf.c       | 28 +++++++++++++++++++++++++++-
 4 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/btf_loader.c b/btf_loader.c
index ec286f4..7cc39aa 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -160,7 +160,7 @@ static struct variable *variable__new(strings_t name, uint32_t linkage)
 	return var;
 }
 
-static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
+static int create_new_int_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
 	uint32_t attrs = btf_int_encoding(tp);
 	strings_t name = tp->name_off;
@@ -175,6 +175,20 @@ static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp,
 	return 0;
 }
 
+static int create_new_float_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
+{
+	strings_t name = tp->name_off;
+	struct base_type *base = base_type__new(name, 0, BT_FP_SINGLE, tp->size * 8);
+
+	if (base == NULL)
+		return -ENOMEM;
+
+	base->tag.tag = DW_TAG_base_type;
+	cu__add_tag_with_id(btfe->priv, &base->tag, id);
+
+	return 0;
+}
+
 static int create_new_array(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
 	struct btf_array *ap = btf_array(tp);
@@ -397,7 +411,7 @@ static int btf_elf__load_types(struct btf_elf *btfe)
 
 		switch (type) {
 		case BTF_KIND_INT:
-			err = create_new_base_type(btfe, type_ptr, type_index);
+			err = create_new_int_type(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_ARRAY:
 			err = create_new_array(btfe, type_ptr, type_index);
@@ -442,6 +456,9 @@ static int btf_elf__load_types(struct btf_elf *btfe)
 			// BTF_KIND_FUNC corresponding to a defined subprogram.
 			err = create_new_function(btfe, type_ptr, type_index);
 			break;
+		case BTF_KIND_FLOAT:
+			err = create_new_float_type(btfe, type_ptr, type_index);
+			break;
 		default:
 			fprintf(stderr, "BTF: idx: %d, Unknown kind %d\n", type_index, type);
 			fflush(stderr);
diff --git a/dwarf_loader.c b/dwarf_loader.c
index b73d786..c5e6681 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -461,6 +461,16 @@ static struct ptr_to_member_type *ptr_to_member_type__new(Dwarf_Die *die,
 	return ptr;
 }
 
+static uint8_t encoding_to_float_type(uint64_t encoding)
+{
+	switch (encoding) {
+	case DW_ATE_complex_float:	return BT_FP_CMPLX;
+	case DW_ATE_float:		return BT_FP_SINGLE;
+	case DW_ATE_imaginary_float:	return BT_FP_IMGRY;
+	default:			return 0;
+	}
+}
+
 static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
 {
 	struct base_type *bt = tag__alloc(cu, sizeof(*bt));
@@ -474,6 +484,7 @@ static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
 		bt->is_signed = encoding == DW_ATE_signed;
 		bt->is_varargs = false;
 		bt->name_has_encoding = true;
+		bt->float_type = encoding_to_float_type(encoding);
 	}
 
 	return bt;
diff --git a/lib/bpf b/lib/bpf
index 5af3d86..986962f 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 5af3d86b5a2c5fecdc3ab83822d083edd32b4396
+Subproject commit 986962fade5dfa89c2890f3854eb040d2a64ab38
diff --git a/libbtf.c b/libbtf.c
index 9f76283..ccd9f90 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -227,6 +227,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
 	[BTF_KIND_VAR]          = "VAR",
 	[BTF_KIND_DATASEC]      = "DATASEC",
+	[BTF_KIND_FLOAT]        = "FLOAT",
 };
 
 static const char *btf_elf__printable_name(const struct btf_elf *btfe, uint32_t offset)
@@ -367,6 +368,27 @@ static void btf_log_func_param(const struct btf_elf *btfe,
 	}
 }
 
+static int32_t btf_elf__add_float_type(struct btf_elf *btfe,
+				       const struct base_type *bt,
+				       const char *name)
+{
+	int32_t id;
+
+	id = btf__add_float(btfe->btf, name, BITS_ROUNDUP_BYTES(bt->bit_size));
+	if (id < 0) {
+		btf_elf__log_err(btfe, BTF_KIND_FLOAT, name, true, "Error emitting BTF type");
+	} else {
+		const struct btf_type *t;
+
+		t = btf__type_by_id(btfe->btf, id);
+		btf_elf__log_type(btfe, t, false, true,
+				  "size=%u nr_bits=%u",
+				  t->size, bt->bit_size);
+	}
+
+	return id;
+}
+
 int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
 			       const char *name)
 {
@@ -380,7 +402,11 @@ int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
 	} else if (bt->is_bool) {
 		encoding = BTF_INT_BOOL;
 	} else if (bt->float_type) {
-		fprintf(stderr, "float_type is not supported\n");
+		if (bt->float_type == BT_FP_SINGLE ||
+		    bt->float_type == BT_FP_DOUBLE ||
+		    bt->float_type == BT_FP_LDBL)
+			return btf_elf__add_float_type(btfe, bt, name);
+		fprintf(stderr, "Complex, interval and imaginary float types are not supported\n");
 		return -1;
 	}
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-06  2:22 [PATCH] btf: Add support for the floating-point types Ilya Leoshkevich
@ 2021-03-07  3:16 ` Andrii Nakryiko
  2021-03-07 13:16   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2021-03-07  3:16 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Arnaldo Carvalho de Melo, dwarves, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Yonghong Song, Heiko Carstens,
	Vasily Gorbik

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
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---

[PATCH dwarves] would make it a bit clearer that this is pahole patch.

But LGTM.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  btf_loader.c   | 21 +++++++++++++++++++--
>  dwarf_loader.c | 11 +++++++++++
>  lib/bpf        |  2 +-
>  libbtf.c       | 28 +++++++++++++++++++++++++++-
>  4 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/btf_loader.c b/btf_loader.c
> index ec286f4..7cc39aa 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -160,7 +160,7 @@ static struct variable *variable__new(strings_t name, uint32_t linkage)
>         return var;
>  }
>
> -static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> +static int create_new_int_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
>  {
>         uint32_t attrs = btf_int_encoding(tp);
>         strings_t name = tp->name_off;
> @@ -175,6 +175,20 @@ static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp,
>         return 0;
>  }
>
> +static int create_new_float_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> +{
> +       strings_t name = tp->name_off;
> +       struct base_type *base = base_type__new(name, 0, BT_FP_SINGLE, tp->size * 8);
> +
> +       if (base == NULL)
> +               return -ENOMEM;
> +
> +       base->tag.tag = DW_TAG_base_type;
> +       cu__add_tag_with_id(btfe->priv, &base->tag, id);
> +
> +       return 0;
> +}
> +
>  static int create_new_array(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
>  {
>         struct btf_array *ap = btf_array(tp);
> @@ -397,7 +411,7 @@ static int btf_elf__load_types(struct btf_elf *btfe)
>
>                 switch (type) {
>                 case BTF_KIND_INT:
> -                       err = create_new_base_type(btfe, type_ptr, type_index);
> +                       err = create_new_int_type(btfe, type_ptr, type_index);
>                         break;
>                 case BTF_KIND_ARRAY:
>                         err = create_new_array(btfe, type_ptr, type_index);
> @@ -442,6 +456,9 @@ static int btf_elf__load_types(struct btf_elf *btfe)
>                         // BTF_KIND_FUNC corresponding to a defined subprogram.
>                         err = create_new_function(btfe, type_ptr, type_index);
>                         break;
> +               case BTF_KIND_FLOAT:
> +                       err = create_new_float_type(btfe, type_ptr, type_index);
> +                       break;
>                 default:
>                         fprintf(stderr, "BTF: idx: %d, Unknown kind %d\n", type_index, type);
>                         fflush(stderr);
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index b73d786..c5e6681 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -461,6 +461,16 @@ static struct ptr_to_member_type *ptr_to_member_type__new(Dwarf_Die *die,
>         return ptr;
>  }
>
> +static uint8_t encoding_to_float_type(uint64_t encoding)
> +{
> +       switch (encoding) {
> +       case DW_ATE_complex_float:      return BT_FP_CMPLX;
> +       case DW_ATE_float:              return BT_FP_SINGLE;
> +       case DW_ATE_imaginary_float:    return BT_FP_IMGRY;
> +       default:                        return 0;
> +       }
> +}
> +
>  static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
>  {
>         struct base_type *bt = tag__alloc(cu, sizeof(*bt));
> @@ -474,6 +484,7 @@ static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
>                 bt->is_signed = encoding == DW_ATE_signed;
>                 bt->is_varargs = false;
>                 bt->name_has_encoding = true;
> +               bt->float_type = encoding_to_float_type(encoding);
>         }
>
>         return bt;
> diff --git a/lib/bpf b/lib/bpf
> index 5af3d86..986962f 160000
> --- a/lib/bpf
> +++ b/lib/bpf
> @@ -1 +1 @@
> -Subproject commit 5af3d86b5a2c5fecdc3ab83822d083edd32b4396
> +Subproject commit 986962fade5dfa89c2890f3854eb040d2a64ab38
> diff --git a/libbtf.c b/libbtf.c
> index 9f76283..ccd9f90 100644
> --- a/libbtf.c
> +++ b/libbtf.c
> @@ -227,6 +227,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
>         [BTF_KIND_FUNC_PROTO]   = "FUNC_PROTO",
>         [BTF_KIND_VAR]          = "VAR",
>         [BTF_KIND_DATASEC]      = "DATASEC",
> +       [BTF_KIND_FLOAT]        = "FLOAT",
>  };
>
>  static const char *btf_elf__printable_name(const struct btf_elf *btfe, uint32_t offset)
> @@ -367,6 +368,27 @@ static void btf_log_func_param(const struct btf_elf *btfe,
>         }
>  }
>
> +static int32_t btf_elf__add_float_type(struct btf_elf *btfe,
> +                                      const struct base_type *bt,
> +                                      const char *name)
> +{
> +       int32_t id;
> +
> +       id = btf__add_float(btfe->btf, name, BITS_ROUNDUP_BYTES(bt->bit_size));
> +       if (id < 0) {
> +               btf_elf__log_err(btfe, BTF_KIND_FLOAT, name, true, "Error emitting BTF type");
> +       } else {
> +               const struct btf_type *t;
> +
> +               t = btf__type_by_id(btfe->btf, id);
> +               btf_elf__log_type(btfe, t, false, true,
> +                                 "size=%u nr_bits=%u",
> +                                 t->size, bt->bit_size);
> +       }
> +
> +       return id;
> +}
> +
>  int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
>                                const char *name)
>  {
> @@ -380,7 +402,11 @@ int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
>         } else if (bt->is_bool) {
>                 encoding = BTF_INT_BOOL;
>         } else if (bt->float_type) {
> -               fprintf(stderr, "float_type is not supported\n");
> +               if (bt->float_type == BT_FP_SINGLE ||
> +                   bt->float_type == BT_FP_DOUBLE ||
> +                   bt->float_type == BT_FP_LDBL)
> +                       return btf_elf__add_float_type(btfe, bt, name);
> +               fprintf(stderr, "Complex, interval and imaginary float types are not supported\n");
>                 return -1;
>         }
>
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-07  3:16 ` Andrii Nakryiko
@ 2021-03-07 13:16   ` Arnaldo Carvalho de Melo
  2021-03-07 13:44     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-07 13:16 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Ilya Leoshkevich, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

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
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> 
> [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...

- Arnaldo
 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> >  btf_loader.c   | 21 +++++++++++++++++++--
> >  dwarf_loader.c | 11 +++++++++++
> >  lib/bpf        |  2 +-
> >  libbtf.c       | 28 +++++++++++++++++++++++++++-
> >  4 files changed, 58 insertions(+), 4 deletions(-)
> >
> > diff --git a/btf_loader.c b/btf_loader.c
> > index ec286f4..7cc39aa 100644
> > --- a/btf_loader.c
> > +++ b/btf_loader.c
> > @@ -160,7 +160,7 @@ static struct variable *variable__new(strings_t name, uint32_t linkage)
> >         return var;
> >  }
> >
> > -static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> > +static int create_new_int_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> >  {
> >         uint32_t attrs = btf_int_encoding(tp);
> >         strings_t name = tp->name_off;
> > @@ -175,6 +175,20 @@ static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp,
> >         return 0;
> >  }
> >
> > +static int create_new_float_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> > +{
> > +       strings_t name = tp->name_off;
> > +       struct base_type *base = base_type__new(name, 0, BT_FP_SINGLE, tp->size * 8);
> > +
> > +       if (base == NULL)
> > +               return -ENOMEM;
> > +
> > +       base->tag.tag = DW_TAG_base_type;
> > +       cu__add_tag_with_id(btfe->priv, &base->tag, id);
> > +
> > +       return 0;
> > +}
> > +
> >  static int create_new_array(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> >  {
> >         struct btf_array *ap = btf_array(tp);
> > @@ -397,7 +411,7 @@ static int btf_elf__load_types(struct btf_elf *btfe)
> >
> >                 switch (type) {
> >                 case BTF_KIND_INT:
> > -                       err = create_new_base_type(btfe, type_ptr, type_index);
> > +                       err = create_new_int_type(btfe, type_ptr, type_index);
> >                         break;
> >                 case BTF_KIND_ARRAY:
> >                         err = create_new_array(btfe, type_ptr, type_index);
> > @@ -442,6 +456,9 @@ static int btf_elf__load_types(struct btf_elf *btfe)
> >                         // BTF_KIND_FUNC corresponding to a defined subprogram.
> >                         err = create_new_function(btfe, type_ptr, type_index);
> >                         break;
> > +               case BTF_KIND_FLOAT:
> > +                       err = create_new_float_type(btfe, type_ptr, type_index);
> > +                       break;
> >                 default:
> >                         fprintf(stderr, "BTF: idx: %d, Unknown kind %d\n", type_index, type);
> >                         fflush(stderr);
> > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > index b73d786..c5e6681 100644
> > --- a/dwarf_loader.c
> > +++ b/dwarf_loader.c
> > @@ -461,6 +461,16 @@ static struct ptr_to_member_type *ptr_to_member_type__new(Dwarf_Die *die,
> >         return ptr;
> >  }
> >
> > +static uint8_t encoding_to_float_type(uint64_t encoding)
> > +{
> > +       switch (encoding) {
> > +       case DW_ATE_complex_float:      return BT_FP_CMPLX;
> > +       case DW_ATE_float:              return BT_FP_SINGLE;
> > +       case DW_ATE_imaginary_float:    return BT_FP_IMGRY;
> > +       default:                        return 0;
> > +       }
> > +}
> > +
> >  static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
> >  {
> >         struct base_type *bt = tag__alloc(cu, sizeof(*bt));
> > @@ -474,6 +484,7 @@ static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
> >                 bt->is_signed = encoding == DW_ATE_signed;
> >                 bt->is_varargs = false;
> >                 bt->name_has_encoding = true;
> > +               bt->float_type = encoding_to_float_type(encoding);
> >         }
> >
> >         return bt;
> > diff --git a/lib/bpf b/lib/bpf
> > index 5af3d86..986962f 160000
> > --- a/lib/bpf
> > +++ b/lib/bpf
> > @@ -1 +1 @@
> > -Subproject commit 5af3d86b5a2c5fecdc3ab83822d083edd32b4396
> > +Subproject commit 986962fade5dfa89c2890f3854eb040d2a64ab38
> > diff --git a/libbtf.c b/libbtf.c
> > index 9f76283..ccd9f90 100644
> > --- a/libbtf.c
> > +++ b/libbtf.c
> > @@ -227,6 +227,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
> >         [BTF_KIND_FUNC_PROTO]   = "FUNC_PROTO",
> >         [BTF_KIND_VAR]          = "VAR",
> >         [BTF_KIND_DATASEC]      = "DATASEC",
> > +       [BTF_KIND_FLOAT]        = "FLOAT",
> >  };
> >
> >  static const char *btf_elf__printable_name(const struct btf_elf *btfe, uint32_t offset)
> > @@ -367,6 +368,27 @@ static void btf_log_func_param(const struct btf_elf *btfe,
> >         }
> >  }
> >
> > +static int32_t btf_elf__add_float_type(struct btf_elf *btfe,
> > +                                      const struct base_type *bt,
> > +                                      const char *name)
> > +{
> > +       int32_t id;
> > +
> > +       id = btf__add_float(btfe->btf, name, BITS_ROUNDUP_BYTES(bt->bit_size));
> > +       if (id < 0) {
> > +               btf_elf__log_err(btfe, BTF_KIND_FLOAT, name, true, "Error emitting BTF type");
> > +       } else {
> > +               const struct btf_type *t;
> > +
> > +               t = btf__type_by_id(btfe->btf, id);
> > +               btf_elf__log_type(btfe, t, false, true,
> > +                                 "size=%u nr_bits=%u",
> > +                                 t->size, bt->bit_size);
> > +       }
> > +
> > +       return id;
> > +}
> > +
> >  int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
> >                                const char *name)
> >  {
> > @@ -380,7 +402,11 @@ int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
> >         } else if (bt->is_bool) {
> >                 encoding = BTF_INT_BOOL;
> >         } else if (bt->float_type) {
> > -               fprintf(stderr, "float_type is not supported\n");
> > +               if (bt->float_type == BT_FP_SINGLE ||
> > +                   bt->float_type == BT_FP_DOUBLE ||
> > +                   bt->float_type == BT_FP_LDBL)
> > +                       return btf_elf__add_float_type(btfe, bt, name);
> > +               fprintf(stderr, "Complex, interval and imaginary float types are not supported\n");
> >                 return -1;
> >         }
> >
> > --
> > 2.29.2
> >

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-07 13:16   ` Arnaldo Carvalho de Melo
@ 2021-03-07 13:44     ` Arnaldo Carvalho de Melo
  2021-03-07 13:50       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-07 13:44 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Ilya Leoshkevich, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

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]$

- Arnaldo
 
> - Arnaldo
>  
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > 
> > >  btf_loader.c   | 21 +++++++++++++++++++--
> > >  dwarf_loader.c | 11 +++++++++++
> > >  lib/bpf        |  2 +-
> > >  libbtf.c       | 28 +++++++++++++++++++++++++++-
> > >  4 files changed, 58 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/btf_loader.c b/btf_loader.c
> > > index ec286f4..7cc39aa 100644
> > > --- a/btf_loader.c
> > > +++ b/btf_loader.c
> > > @@ -160,7 +160,7 @@ static struct variable *variable__new(strings_t name, uint32_t linkage)
> > >         return var;
> > >  }
> > >
> > > -static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> > > +static int create_new_int_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> > >  {
> > >         uint32_t attrs = btf_int_encoding(tp);
> > >         strings_t name = tp->name_off;
> > > @@ -175,6 +175,20 @@ static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp,
> > >         return 0;
> > >  }
> > >
> > > +static int create_new_float_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> > > +{
> > > +       strings_t name = tp->name_off;
> > > +       struct base_type *base = base_type__new(name, 0, BT_FP_SINGLE, tp->size * 8);
> > > +
> > > +       if (base == NULL)
> > > +               return -ENOMEM;
> > > +
> > > +       base->tag.tag = DW_TAG_base_type;
> > > +       cu__add_tag_with_id(btfe->priv, &base->tag, id);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static int create_new_array(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
> > >  {
> > >         struct btf_array *ap = btf_array(tp);
> > > @@ -397,7 +411,7 @@ static int btf_elf__load_types(struct btf_elf *btfe)
> > >
> > >                 switch (type) {
> > >                 case BTF_KIND_INT:
> > > -                       err = create_new_base_type(btfe, type_ptr, type_index);
> > > +                       err = create_new_int_type(btfe, type_ptr, type_index);
> > >                         break;
> > >                 case BTF_KIND_ARRAY:
> > >                         err = create_new_array(btfe, type_ptr, type_index);
> > > @@ -442,6 +456,9 @@ static int btf_elf__load_types(struct btf_elf *btfe)
> > >                         // BTF_KIND_FUNC corresponding to a defined subprogram.
> > >                         err = create_new_function(btfe, type_ptr, type_index);
> > >                         break;
> > > +               case BTF_KIND_FLOAT:
> > > +                       err = create_new_float_type(btfe, type_ptr, type_index);
> > > +                       break;
> > >                 default:
> > >                         fprintf(stderr, "BTF: idx: %d, Unknown kind %d\n", type_index, type);
> > >                         fflush(stderr);
> > > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > > index b73d786..c5e6681 100644
> > > --- a/dwarf_loader.c
> > > +++ b/dwarf_loader.c
> > > @@ -461,6 +461,16 @@ static struct ptr_to_member_type *ptr_to_member_type__new(Dwarf_Die *die,
> > >         return ptr;
> > >  }
> > >
> > > +static uint8_t encoding_to_float_type(uint64_t encoding)
> > > +{
> > > +       switch (encoding) {
> > > +       case DW_ATE_complex_float:      return BT_FP_CMPLX;
> > > +       case DW_ATE_float:              return BT_FP_SINGLE;
> > > +       case DW_ATE_imaginary_float:    return BT_FP_IMGRY;
> > > +       default:                        return 0;
> > > +       }
> > > +}
> > > +
> > >  static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
> > >  {
> > >         struct base_type *bt = tag__alloc(cu, sizeof(*bt));
> > > @@ -474,6 +484,7 @@ static struct base_type *base_type__new(Dwarf_Die *die, struct cu *cu)
> > >                 bt->is_signed = encoding == DW_ATE_signed;
> > >                 bt->is_varargs = false;
> > >                 bt->name_has_encoding = true;
> > > +               bt->float_type = encoding_to_float_type(encoding);
> > >         }
> > >
> > >         return bt;
> > > diff --git a/lib/bpf b/lib/bpf
> > > index 5af3d86..986962f 160000
> > > --- a/lib/bpf
> > > +++ b/lib/bpf
> > > @@ -1 +1 @@
> > > -Subproject commit 5af3d86b5a2c5fecdc3ab83822d083edd32b4396
> > > +Subproject commit 986962fade5dfa89c2890f3854eb040d2a64ab38
> > > diff --git a/libbtf.c b/libbtf.c
> > > index 9f76283..ccd9f90 100644
> > > --- a/libbtf.c
> > > +++ b/libbtf.c
> > > @@ -227,6 +227,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
> > >         [BTF_KIND_FUNC_PROTO]   = "FUNC_PROTO",
> > >         [BTF_KIND_VAR]          = "VAR",
> > >         [BTF_KIND_DATASEC]      = "DATASEC",
> > > +       [BTF_KIND_FLOAT]        = "FLOAT",
> > >  };
> > >
> > >  static const char *btf_elf__printable_name(const struct btf_elf *btfe, uint32_t offset)
> > > @@ -367,6 +368,27 @@ static void btf_log_func_param(const struct btf_elf *btfe,
> > >         }
> > >  }
> > >
> > > +static int32_t btf_elf__add_float_type(struct btf_elf *btfe,
> > > +                                      const struct base_type *bt,
> > > +                                      const char *name)
> > > +{
> > > +       int32_t id;
> > > +
> > > +       id = btf__add_float(btfe->btf, name, BITS_ROUNDUP_BYTES(bt->bit_size));
> > > +       if (id < 0) {
> > > +               btf_elf__log_err(btfe, BTF_KIND_FLOAT, name, true, "Error emitting BTF type");
> > > +       } else {
> > > +               const struct btf_type *t;
> > > +
> > > +               t = btf__type_by_id(btfe->btf, id);
> > > +               btf_elf__log_type(btfe, t, false, true,
> > > +                                 "size=%u nr_bits=%u",
> > > +                                 t->size, bt->bit_size);
> > > +       }
> > > +
> > > +       return id;
> > > +}
> > > +
> > >  int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
> > >                                const char *name)
> > >  {
> > > @@ -380,7 +402,11 @@ int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
> > >         } else if (bt->is_bool) {
> > >                 encoding = BTF_INT_BOOL;
> > >         } else if (bt->float_type) {
> > > -               fprintf(stderr, "float_type is not supported\n");
> > > +               if (bt->float_type == BT_FP_SINGLE ||
> > > +                   bt->float_type == BT_FP_DOUBLE ||
> > > +                   bt->float_type == BT_FP_LDBL)
> > > +                       return btf_elf__add_float_type(btfe, bt, name);
> > > +               fprintf(stderr, "Complex, interval and imaginary float types are not supported\n");
> > >                 return -1;
> > >         }
> > >
> > > --
> > > 2.29.2
> > >
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-07 13:44     ` Arnaldo Carvalho de Melo
@ 2021-03-07 13:50       ` Arnaldo Carvalho de Melo
  2021-03-07 14:09         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-07 13:50 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Ilya Leoshkevich, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

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.

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-07 13:50       ` Arnaldo Carvalho de Melo
@ 2021-03-07 14:09         ` Arnaldo Carvalho de Melo
  2021-03-08  3:02           ` Ilya Leoshkevich
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-07 14:09 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Ilya Leoshkevich, Jiri Olsa, Arnaldo Carvalho de Melo, dwarves,
	bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-07 14:09         ` Arnaldo Carvalho de Melo
@ 2021-03-08  3:02           ` Ilya Leoshkevich
       [not found]             ` <YEYgVmo0ryuM3SUY@kernel.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Ilya Leoshkevich @ 2021-03-08  3:02 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Andrii Nakryiko
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

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?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
       [not found]             ` <YEYgVmo0ryuM3SUY@kernel.org>
@ 2021-03-08 22:16               ` Andrii Nakryiko
  2021-03-08 22:31                 ` Ilya Leoshkevich
  0 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2021-03-08 22:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ilya Leoshkevich, Jiri Olsa, Arnaldo Carvalho de Melo, dwarves,
	bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

On Mon, Mar 8, 2021 at 5:02 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Mon, Mar 08, 2021 at 04:02:58AM +0100, Ilya Leoshkevich escreveu:
> > 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:
> > > > 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
>
> > > Ideas?
>
> > 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?
>
> Yes, that would be best, some sort of capability querying and then a
> decision about using the new feature.

pahole could be used to add .BTF post-factum to vmlinux image of a
very old kernel, even the one that doesn't support BTF at all. So
whatever detection system is going to be added, we should make it easy
to turn it off.

>
> - Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] btf: Add support for the floating-point types
  2021-03-08 22:16               ` Andrii Nakryiko
@ 2021-03-08 22:31                 ` Ilya Leoshkevich
  0 siblings, 0 replies; 9+ messages in thread
From: Ilya Leoshkevich @ 2021-03-08 22:31 UTC (permalink / raw)
  To: Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Heiko Carstens, Vasily Gorbik

On Mon, 2021-03-08 at 14:16 -0800, Andrii Nakryiko wrote:
> On Mon, Mar 8, 2021 at 5:02 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> > 
> > Em Mon, Mar 08, 2021 at 04:02:58AM +0100, Ilya Leoshkevich
> > escreveu:
> > > 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:
> > > > > 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
> > 
> > > > Ideas?
> > 
> > > 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?
> > 
> > Yes, that would be best, some sort of capability querying and then
> > a
> > decision about using the new feature.
> 
> pahole could be used to add .BTF post-factum to vmlinux image of a
> very old kernel, even the one that doesn't support BTF at all. So
> whatever detection system is going to be added, we should make it
> easy
> to turn it off.

I'd rather not have detection at all. Instead, pahole should encode
floats as ints by default (that's not fully correct, but that's the way
it is today). Then we can pass --libbpf-compat=0.4.0 in link-vmlinux.sh
in the newer kernels, which would turn on the new feature.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-08 22:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-06  2:22 [PATCH] btf: Add support for the floating-point types Ilya Leoshkevich
2021-03-07  3:16 ` Andrii Nakryiko
2021-03-07 13:16   ` Arnaldo Carvalho de Melo
2021-03-07 13:44     ` Arnaldo Carvalho de Melo
2021-03-07 13:50       ` Arnaldo Carvalho de Melo
2021-03-07 14:09         ` Arnaldo Carvalho de Melo
2021-03-08  3:02           ` Ilya Leoshkevich
     [not found]             ` <YEYgVmo0ryuM3SUY@kernel.org>
2021-03-08 22:16               ` Andrii Nakryiko
2021-03-08 22:31                 ` Ilya Leoshkevich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).