Re: [PATCH dwarves v1] pahole: generate "bpf_fastcall" decl tags for eligible kfuncs

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2024-09-16 at 11:16 +0100, Alan Maguire wrote:

[...]

> hi Eduard,
> 
> you've added support for multiple declaration tags as part of this, but
> I wonder if we could go slightly further to simplify any additional
> future KF_* flags -> decl tag needs?
> 
> Specifically if we had an array of <set8 flags, tag name> mappings such
> that we can add support for new declaration tags by simply adding a new
> flag and declaration tag string. When checking flags value in
> btf_encoder__tag_kfunc(), we'd just walk the array entries, and for each
> matching flag add the associated decl tag. Would that work?

Hi Alan,

That would be something like below.
It works, but looks a bit over-complicated for my taste, wdyt?

--- 8< ----------------------------------------
iff --git a/btf_encoder.c b/btf_encoder.c
index ae059e0..b6178c3 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -39,7 +39,6 @@
 #define BTF_ID_SET8_PFX                "__BTF_ID__set8__"
 #define BTF_SET8_KFUNCS                (1 << 0)
 #define BTF_KFUNC_TYPE_TAG     "bpf_kfunc"
-#define BTF_FASTCALL_TAG       "bpf_fastcall"
 #define KF_FASTCALL            (1 << 12)
 
 struct btf_id_and_flag {
@@ -1534,6 +1533,15 @@ static int add_kfunc_decl_tag(struct btf *btf, const char *tag, __u32 id, const
        return 0;
 }
 
+enum kf_bit_nums {
+       KF_BIT_NUM_FASTCALL = 12,
+       KF_BIT_NUM_FASTCALL_NR
+};
+
+static const char *kfunc_tags[KF_BIT_NUM_FASTCALL_NR] = {
+       [KF_BIT_NUM_FASTCALL] = "bpf_fastcall"
+};
+
 static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc, __u32 flags)
 {
        struct btf_func key = { .name = kfunc };
@@ -1559,8 +1567,11 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
        err = add_kfunc_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, kfunc);
        if (err < 0)
                return err;
-       if (flags & KF_FASTCALL) {
-               err = add_kfunc_decl_tag(btf, BTF_FASTCALL_TAG, target->type_id, kfunc);
+
+       for (uint32_t i = 0; i < KF_BIT_NUM_FASTCALL_NR; i++) {
+                if (!(flags & (1u << i)) || !kfunc_tags[i])
+                       continue;
+               err = add_kfunc_decl_tag(btf, kfunc_tags[i], target->type_id, kfunc);
                if (err < 0)
                        return err;
        }
---------------------------------------- >8 ---

> 
> > ---
> >  btf_encoder.c | 59 +++++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 43 insertions(+), 16 deletions(-)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 8a2d92e..ae059e0 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -39,15 +39,19 @@
> >  #define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
> >  #define BTF_SET8_KFUNCS		(1 << 0)
> >  #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
> > +#define BTF_FASTCALL_TAG	"bpf_fastcall"
> > +#define KF_FASTCALL		(1 << 12)
> > +
> 
> probably need an #ifndef KF_FASTCALL/#endif here once this makes it into
> uapi.

kfunc flags are defined in include/linux/btf.h so these should not be
visible in the uapi/linux/btf.h, unless I'm confused.

> 
> 
> > +struct btf_id_and_flag {
> > +        uint32_t id;
> > +        uint32_t flags;
> > +};

[...]