* [PATCH dwarves v2] btf: Add support for the floating-point types
@ 2021-03-08 23:59 Ilya Leoshkevich
2021-03-09 11:48 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Ilya Leoshkevich @ 2021-03-08 23:59 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 the DWARF loader fill base_type.float_type.
- Introduce libbpf compatibility level command-line parameter, so that
pahole could be used to build both the older and the newer kernels.
- libbpf introduced the 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 not in the compatibility mode and base_type.float_type is
set.
- Make the BTF loader recognize the new BTF kind.
Example of the resulting entry in the vmlinux BTF:
[7164] FLOAT 'double' size=8
when building with:
LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} --libbpf_compat=0.4.0
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
v1: https://lore.kernel.org/dwarves/20210306022203.152930-1-iii@linux.ibm.com/
v1 -> v2: Introduce libbpf compatibility level command-line parameter.
The code should now work for both bpf-next/master and
v5.12-rc2.
btf_loader.c | 21 +++++++++++++++++++--
dwarf_loader.c | 11 +++++++++++
lib/bpf | 2 +-
libbtf.c | 36 ++++++++++++++++++++++++++++++++++--
libbtf.h | 8 ++++++++
pahole.c | 26 ++++++++++++++++++++++++++
6 files changed, 99 insertions(+), 5 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..c8a1dc1 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -30,6 +30,7 @@
struct btf *base_btf;
uint8_t btf_elf__verbose;
uint8_t btf_elf__force;
+int libbpf_compat = LIBBPF_COMPAT_MIN;
static int btf_var_secinfo_cmp(const void *a, const void *b)
{
@@ -227,6 +228,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 +369,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)
{
@@ -379,8 +402,17 @@ int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
encoding = BTF_INT_SIGNED;
} else if (bt->is_bool) {
encoding = BTF_INT_BOOL;
- } else if (bt->float_type) {
- fprintf(stderr, "float_type is not supported\n");
+ } else if (bt->float_type && libbpf_compat >= LIBBPF_COMPAT_FLOAT) {
+ /*
+ * Encode floats using libbpf. In compatibility mode, encode
+ * them as ints - that's not fully correct, but that's what it
+ * used to be.
+ */
+ 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;
}
diff --git a/libbtf.h b/libbtf.h
index 191f586..9fe42be 100644
--- a/libbtf.h
+++ b/libbtf.h
@@ -36,6 +36,14 @@ extern uint8_t btf_elf__verbose;
extern uint8_t btf_elf__force;
#define btf_elf__verbose_log(fmt, ...) { if (btf_elf__verbose) printf(fmt, __VA_ARGS__); }
+/* DEBUG_INFO_BTF was added in Linux 5.2, which corresponds to libbpf 0.0.3. */
+#define LIBBPF_COMPAT_MIN 0x000003
+
+/* The floating-point types were added in libbpf 0.4.0. */
+#define LIBBPF_COMPAT_FLOAT 0x000400
+
+extern int libbpf_compat;
+
#define PERCPU_SECTION ".data..percpu"
struct cu;
diff --git a/pahole.c b/pahole.c
index 4a34ba5..74d2cbb 100644
--- a/pahole.c
+++ b/pahole.c
@@ -825,6 +825,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
#define ARGP_just_packed_structs 319
#define ARGP_numeric_version 320
#define ARGP_btf_base 321
+#define ARGP_libbpf_compat 322
static const struct argp_option pahole__options[] = {
{
@@ -1119,6 +1120,12 @@ static const struct argp_option pahole__options[] = {
.key = ARGP_btf_encode_force,
.doc = "Ignore those symbols found invalid when encoding BTF."
},
+ {
+ .name = "libbpf_compat",
+ .key = ARGP_libbpf_compat,
+ .arg = "LIBBPF_VERSION",
+ .doc = "Produce output compatible with this libbpf version."
+ },
{
.name = "structs",
.key = ARGP_just_structs,
@@ -1144,6 +1151,23 @@ static const struct argp_option pahole__options[] = {
}
};
+static int parse_version(char *arg)
+{
+ int version, patchlevel = 0, extraversion = 0;
+ char *dot;
+
+ version = atoi(arg);
+ dot = strchr(arg, '.');
+ if (dot) {
+ patchlevel = atoi(dot + 1);
+ dot = strchr(dot + 1, '.');
+ if (dot)
+ extraversion = atoi(dot + 1);
+ }
+
+ return (version << 16) | (patchlevel << 8) | extraversion;
+}
+
static error_t pahole__options_parser(int key, char *arg,
struct argp_state *state)
{
@@ -1254,6 +1278,8 @@ static error_t pahole__options_parser(int key, char *arg,
base_btf_file = arg; break;
case ARGP_numeric_version:
print_numeric_version = true; break;
+ case ARGP_libbpf_compat:
+ libbpf_compat = parse_version(arg); break;
default:
return ARGP_ERR_UNKNOWN;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-08 23:59 [PATCH dwarves v2] btf: Add support for the floating-point types Ilya Leoshkevich
@ 2021-03-09 11:48 ` Arnaldo Carvalho de Melo
2021-03-09 13:06 ` Ilya Leoshkevich
2021-03-09 21:37 ` Andrii Nakryiko
0 siblings, 2 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-09 11:48 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
Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich escreveu:
> Some BPF programs compiled on s390 fail to load, because s390
> arch-specific linux headers contain float and double types.
>
> Fix as follows:
>
> - Make the DWARF loader fill base_type.float_type.
>
> - Introduce libbpf compatibility level command-line parameter, so that
> pahole could be used to build both the older and the newer kernels.
>
> - libbpf introduced the 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 not in the compatibility mode and base_type.float_type is
> set.
>
> - Make the BTF loader recognize the new BTF kind.
>
> Example of the resulting entry in the vmlinux BTF:
>
> [7164] FLOAT 'double' size=8
>
> when building with:
>
> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} --libbpf_compat=0.4.0
I'm testing it now, and added as a followup patch the man page entry,
please check that the wording is appropriate.
Thanks,
- Arnaldo
[acme@five pahole]$ vim man-pages/pahole.1
[acme@five pahole]$ git diff
diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
index 352bb5e45f319da4..787771753d1933b1 100644
--- a/man-pages/pahole.1
+++ b/man-pages/pahole.1
@@ -199,6 +199,12 @@ Path to the base BTF file, for instance: vmlinux when encoding kernel module BTF
This may be inferred when asking for a /sys/kernel/btf/MODULE, when it will be autoconfigured
to "/sys/kernel/btf/vmlinux".
+.TP
+.B \-\-libbpf_compat=LIBBPF_VERSION
+Produce output compatible with this libbpf version. For instance, specifying 0.4.0 as
+the version would encode BTF_KIND_FLOAT entries in systems where the vmlinux DWARF
+information has float types.
+
.TP
.B \-l, \-\-show_first_biggest_size_base_type_member
Show first biggest size base_type member.
[acme@five pahole]$
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-09 11:48 ` Arnaldo Carvalho de Melo
@ 2021-03-09 13:06 ` Ilya Leoshkevich
2021-03-09 21:37 ` Andrii Nakryiko
1 sibling, 0 replies; 8+ messages in thread
From: Ilya Leoshkevich @ 2021-03-09 13:06 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song, Heiko Carstens,
Vasily Gorbik
On Tue, 2021-03-09 at 08:48 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich escreveu:
> > Some BPF programs compiled on s390 fail to load, because s390
> > arch-specific linux headers contain float and double types.
> >
> > Fix as follows:
> >
> > - Make the DWARF loader fill base_type.float_type.
> >
> > - Introduce libbpf compatibility level command-line parameter, so
> > that
> > pahole could be used to build both the older and the newer
> > kernels.
> >
> > - libbpf introduced the 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 not in the compatibility mode and base_type.float_type
> > is
> > set.
> >
> > - Make the BTF loader recognize the new BTF kind.
> >
> > Example of the resulting entry in the vmlinux BTF:
> >
> > [7164] FLOAT 'double' size=8
> >
> > when building with:
> >
> > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} --libbpf_compat=0.4.0
>
> I'm testing it now, and added as a followup patch the man page entry,
> please check that the wording is appropriate.
>
> Thanks,
>
> - Arnaldo
>
> [acme@five pahole]$ vim man-pages/pahole.1
> [acme@five pahole]$ git diff
> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
> index 352bb5e45f319da4..787771753d1933b1 100644
> --- a/man-pages/pahole.1
> +++ b/man-pages/pahole.1
> @@ -199,6 +199,12 @@ Path to the base BTF file, for instance: vmlinux
> when encoding kernel module BTF
> This may be inferred when asking for a /sys/kernel/btf/MODULE, when
> it will be autoconfigured
> to "/sys/kernel/btf/vmlinux".
>
> +.TP
> +.B \-\-libbpf_compat=LIBBPF_VERSION
> +Produce output compatible with this libbpf version. For instance,
> specifying 0.4.0 as
> +the version would encode BTF_KIND_FLOAT entries in systems where the
> vmlinux DWARF
> +information has float types.
> +
> .TP
> .B \-l, \-\-show_first_biggest_size_base_type_member
> Show first biggest size base_type member.
> [acme@five pahole]$
The wording matches what I had in mind for this new flag, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-09 11:48 ` Arnaldo Carvalho de Melo
2021-03-09 13:06 ` Ilya Leoshkevich
@ 2021-03-09 21:37 ` Andrii Nakryiko
2021-03-09 21:57 ` Ilya Leoshkevich
1 sibling, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2021-03-09 21:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ilya Leoshkevich, Arnaldo Carvalho de Melo, dwarves, bpf,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Yonghong Song, Heiko Carstens, Vasily Gorbik
On Tue, Mar 9, 2021 at 3:48 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich escreveu:
> > Some BPF programs compiled on s390 fail to load, because s390
> > arch-specific linux headers contain float and double types.
> >
> > Fix as follows:
> >
> > - Make the DWARF loader fill base_type.float_type.
> >
> > - Introduce libbpf compatibility level command-line parameter, so that
> > pahole could be used to build both the older and the newer kernels.
> >
> > - libbpf introduced the 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 not in the compatibility mode and base_type.float_type is
> > set.
> >
> > - Make the BTF loader recognize the new BTF kind.
> >
> > Example of the resulting entry in the vmlinux BTF:
> >
> > [7164] FLOAT 'double' size=8
> >
> > when building with:
> >
> > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} --libbpf_compat=0.4.0
>
> I'm testing it now, and added as a followup patch the man page entry,
> please check that the wording is appropriate.
>
> Thanks,
>
> - Arnaldo
>
> [acme@five pahole]$ vim man-pages/pahole.1
> [acme@five pahole]$ git diff
> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
> index 352bb5e45f319da4..787771753d1933b1 100644
> --- a/man-pages/pahole.1
> +++ b/man-pages/pahole.1
> @@ -199,6 +199,12 @@ Path to the base BTF file, for instance: vmlinux when encoding kernel module BTF
> This may be inferred when asking for a /sys/kernel/btf/MODULE, when it will be autoconfigured
> to "/sys/kernel/btf/vmlinux".
>
> +.TP
> +.B \-\-libbpf_compat=LIBBPF_VERSION
> +Produce output compatible with this libbpf version. For instance, specifying 0.4.0 as
> +the version would encode BTF_KIND_FLOAT entries in systems where the vmlinux DWARF
> +information has float types.
TBH, I think it's not exactly right to call out libbpf version here.
It's BTF "version" (if we had such a thing) that determines the set of
supported BTF kinds. There could be other libraries that might want to
parse BTF. So I don't know what this should be called, but
libbpf_compat is probably a wrong name for it.
If we do want to teach pahole to not emit some parts of BTF, it should
probably be a set of BPF features, not some arbitrary library
versions.
> +
> .TP
> .B \-l, \-\-show_first_biggest_size_base_type_member
> Show first biggest size base_type member.
> [acme@five pahole]$
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-09 21:37 ` Andrii Nakryiko
@ 2021-03-09 21:57 ` Ilya Leoshkevich
2021-03-10 4:14 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Ilya Leoshkevich @ 2021-03-09 21:57 UTC (permalink / raw)
To: Andrii Nakryiko, Arnaldo Carvalho de Melo
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song, Heiko Carstens,
Vasily Gorbik
On Tue, 2021-03-09 at 13:37 -0800, Andrii Nakryiko wrote:
> On Tue, Mar 9, 2021 at 3:48 AM Arnaldo Carvalho de Melo <
> acme@kernel.org> wrote:
> >
> > Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich
> > escreveu:
> > > Some BPF programs compiled on s390 fail to load, because s390
> > > arch-specific linux headers contain float and double types.
> > >
> > > Fix as follows:
> > >
> > > - Make the DWARF loader fill base_type.float_type.
> > >
> > > - Introduce libbpf compatibility level command-line parameter, so
> > > that
> > > pahole could be used to build both the older and the newer
> > > kernels.
> > >
> > > - libbpf introduced the 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 not in the compatibility mode and
> > > base_type.float_type is
> > > set.
> > >
> > > - Make the BTF loader recognize the new BTF kind.
> > >
> > > Example of the resulting entry in the vmlinux BTF:
> > >
> > > [7164] FLOAT 'double' size=8
> > >
> > > when building with:
> > >
> > > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} --
> > > libbpf_compat=0.4.0
> >
> > I'm testing it now, and added as a followup patch the man page
> > entry,
> > please check that the wording is appropriate.
> >
> > Thanks,
> >
> > - Arnaldo
> >
> > [acme@five pahole]$ vim man-pages/pahole.1
> > [acme@five pahole]$ git diff
> > diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
> > index 352bb5e45f319da4..787771753d1933b1 100644
> > --- a/man-pages/pahole.1
> > +++ b/man-pages/pahole.1
> > @@ -199,6 +199,12 @@ Path to the base BTF file, for instance:
> > vmlinux when encoding kernel module BTF
> > This may be inferred when asking for a /sys/kernel/btf/MODULE,
> > when it will be autoconfigured
> > to "/sys/kernel/btf/vmlinux".
> >
> > +.TP
> > +.B \-\-libbpf_compat=LIBBPF_VERSION
> > +Produce output compatible with this libbpf version. For instance,
> > specifying 0.4.0 as
> > +the version would encode BTF_KIND_FLOAT entries in systems where
> > the vmlinux DWARF
> > +information has float types.
>
> TBH, I think it's not exactly right to call out libbpf version here.
> It's BTF "version" (if we had such a thing) that determines the set
> of
> supported BTF kinds. There could be other libraries that might want
> to
> parse BTF. So I don't know what this should be called, but
> libbpf_compat is probably a wrong name for it.
BTF version seems to exist: btf_header.version. Should we maybe bump
this?
>
> If we do want to teach pahole to not emit some parts of BTF, it
> should
> probably be a set of BPF features, not some arbitrary library
> versions.
I thought about just adding --btf-allow-floats, but if new features
will be added in the future, the list of options will become unwieldy.
So I thought it would be good to settle for something that increases
monotonically.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-09 21:57 ` Ilya Leoshkevich
@ 2021-03-10 4:14 ` Andrii Nakryiko
2021-03-10 13:37 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2021-03-10 4:14 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, dwarves, bpf,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Yonghong Song, Heiko Carstens, Vasily Gorbik
On Tue, Mar 9, 2021 at 1:57 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Tue, 2021-03-09 at 13:37 -0800, Andrii Nakryiko wrote:
> > On Tue, Mar 9, 2021 at 3:48 AM Arnaldo Carvalho de Melo <
> > acme@kernel.org> wrote:
> > >
> > > Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich
> > > escreveu:
> > > > Some BPF programs compiled on s390 fail to load, because s390
> > > > arch-specific linux headers contain float and double types.
> > > >
> > > > Fix as follows:
> > > >
> > > > - Make the DWARF loader fill base_type.float_type.
> > > >
> > > > - Introduce libbpf compatibility level command-line parameter, so
> > > > that
> > > > pahole could be used to build both the older and the newer
> > > > kernels.
> > > >
> > > > - libbpf introduced the 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 not in the compatibility mode and
> > > > base_type.float_type is
> > > > set.
> > > >
> > > > - Make the BTF loader recognize the new BTF kind.
> > > >
> > > > Example of the resulting entry in the vmlinux BTF:
> > > >
> > > > [7164] FLOAT 'double' size=8
> > > >
> > > > when building with:
> > > >
> > > > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} --
> > > > libbpf_compat=0.4.0
> > >
> > > I'm testing it now, and added as a followup patch the man page
> > > entry,
> > > please check that the wording is appropriate.
> > >
> > > Thanks,
> > >
> > > - Arnaldo
> > >
> > > [acme@five pahole]$ vim man-pages/pahole.1
> > > [acme@five pahole]$ git diff
> > > diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
> > > index 352bb5e45f319da4..787771753d1933b1 100644
> > > --- a/man-pages/pahole.1
> > > +++ b/man-pages/pahole.1
> > > @@ -199,6 +199,12 @@ Path to the base BTF file, for instance:
> > > vmlinux when encoding kernel module BTF
> > > This may be inferred when asking for a /sys/kernel/btf/MODULE,
> > > when it will be autoconfigured
> > > to "/sys/kernel/btf/vmlinux".
> > >
> > > +.TP
> > > +.B \-\-libbpf_compat=LIBBPF_VERSION
> > > +Produce output compatible with this libbpf version. For instance,
> > > specifying 0.4.0 as
> > > +the version would encode BTF_KIND_FLOAT entries in systems where
> > > the vmlinux DWARF
> > > +information has float types.
> >
> > TBH, I think it's not exactly right to call out libbpf version here.
> > It's BTF "version" (if we had such a thing) that determines the set
> > of
> > supported BTF kinds. There could be other libraries that might want
> > to
> > parse BTF. So I don't know what this should be called, but
> > libbpf_compat is probably a wrong name for it.
>
> BTF version seems to exist: btf_header.version. Should we maybe bump
> this?
That seems excessive. If the kernel doesn't use FLOATs, then no one
would even notice a difference. While if we bump this version, then
everything will automatically become incompatible.
>
> >
> > If we do want to teach pahole to not emit some parts of BTF, it
> > should
> > probably be a set of BPF features, not some arbitrary library
> > versions.
>
> I thought about just adding --btf-allow-floats, but if new features
> will be added in the future, the list of options will become unwieldy.
> So I thought it would be good to settle for something that increases
> monotonically.
>
BTF_KIND_FLOAT is the first extension in a long while. I'd worry about
the proliferation of new options when we actually see some proof of
that being a problem in practice.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-10 4:14 ` Andrii Nakryiko
@ 2021-03-10 13:37 ` Arnaldo Carvalho de Melo
2021-03-10 13:39 ` Ilya Leoshkevich
0 siblings, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-10 13:37 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 Tue, Mar 09, 2021 at 08:14:50PM -0800, Andrii Nakryiko escreveu:
> On Tue, Mar 9, 2021 at 1:57 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> > On Tue, 2021-03-09 at 13:37 -0800, Andrii Nakryiko wrote:
> > > On Tue, Mar 9, 2021 at 3:48 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich escreveu:
> > > TBH, I think it's not exactly right to call out libbpf version
> > > here. It's BTF "version" (if we had such a thing) that determines
> > > the set of supported BTF kinds. There could be other libraries
> > > that might want to parse BTF. So I don't know what this should be
> > > called, but libbpf_compat is probably a wrong name for it.
> > BTF version seems to exist: btf_header.version. Should we maybe bump
> > this?
> That seems excessive. If the kernel doesn't use FLOATs, then no one
> would even notice a difference. While if we bump this version, then
> everything will automatically become incompatible.
> > > If we do want to teach pahole to not emit some parts of BTF, it
> > > should
> > > probably be a set of BPF features, not some arbitrary library
> > > versions.
> > I thought about just adding --btf-allow-floats, but if new features
> > will be added in the future, the list of options will become unwieldy.
> > So I thought it would be good to settle for something that increases
> > monotonically.
> BTF_KIND_FLOAT is the first extension in a long while. I'd worry about
> the proliferation of new options when we actually see some proof of
> that being a problem in practice.
I tend to agree, Ilya, can you rework the patch in that direction?
Something like --encode-btf-kind-float that starts disabled or other
suitable name?
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves v2] btf: Add support for the floating-point types
2021-03-10 13:37 ` Arnaldo Carvalho de Melo
@ 2021-03-10 13:39 ` Ilya Leoshkevich
0 siblings, 0 replies; 8+ messages in thread
From: Ilya Leoshkevich @ 2021-03-10 13:39 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Andrii Nakryiko
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song, Heiko Carstens,
Vasily Gorbik
On Wed, 2021-03-10 at 10:37 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 09, 2021 at 08:14:50PM -0800, Andrii Nakryiko escreveu:
> > On Tue, Mar 9, 2021 at 1:57 PM Ilya Leoshkevich <iii@linux.ibm.com>
> > wrote:
> > > On Tue, 2021-03-09 at 13:37 -0800, Andrii Nakryiko wrote:
> > > > On Tue, Mar 9, 2021 at 3:48 AM Arnaldo Carvalho de Melo
> > > > <acme@kernel.org> wrote:
> > > > > Em Tue, Mar 09, 2021 at 12:59:13AM +0100, Ilya Leoshkevich
> > > > > escreveu:
> > > > TBH, I think it's not exactly right to call out libbpf version
> > > > here. It's BTF "version" (if we had such a thing) that
> > > > determines
> > > > the set of supported BTF kinds. There could be other libraries
> > > > that might want to parse BTF. So I don't know what this should
> > > > be
> > > > called, but libbpf_compat is probably a wrong name for it.
>
> > > BTF version seems to exist: btf_header.version. Should we maybe
> > > bump
> > > this?
>
> > That seems excessive. If the kernel doesn't use FLOATs, then no one
> > would even notice a difference. While if we bump this version, then
> > everything will automatically become incompatible.
>
> > > > If we do want to teach pahole to not emit some parts of BTF, it
> > > > should
> > > > probably be a set of BPF features, not some arbitrary library
> > > > versions.
>
> > > I thought about just adding --btf-allow-floats, but if new
> > > features
> > > will be added in the future, the list of options will become
> > > unwieldy.
> > > So I thought it would be good to settle for something that
> > > increases
> > > monotonically.
>
> > BTF_KIND_FLOAT is the first extension in a long while. I'd worry
> > about
> > the proliferation of new options when we actually see some proof of
> > that being a problem in practice.
>
> I tend to agree, Ilya, can you rework the patch in that direction?
> Something like --encode-btf-kind-float that starts disabled or other
> suitable name?
>
> - Arnaldo
Sure, I'm actually testing v3 that does this right now. So far I went
with simply --btf_float, but I'll change it to the more explicit
--encode_btf_kind_float.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-10 13:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 23:59 [PATCH dwarves v2] btf: Add support for the floating-point types Ilya Leoshkevich
2021-03-09 11:48 ` Arnaldo Carvalho de Melo
2021-03-09 13:06 ` Ilya Leoshkevich
2021-03-09 21:37 ` Andrii Nakryiko
2021-03-09 21:57 ` Ilya Leoshkevich
2021-03-10 4:14 ` Andrii Nakryiko
2021-03-10 13:37 ` Arnaldo Carvalho de Melo
2021-03-10 13:39 ` Ilya Leoshkevich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.