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

[Alan Maguire <alan.maguire@oracle.com>]

On 17/09/2024 05:40, Eduard Zingerman wrote:
> 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?

yeah, I guess I was thinking the kfunc flag would be checked along with
KF_FASTCALL, but the code already knows it's a kfunc, so no need.  We
can add something like this if/when other flags are added.

Sorry for the noise!

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> 
> --- 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.
>

ah, ok sorry I thought it was UAPI, never mind.

>>
>>
>>> +struct btf_id_and_flag {
>>> +        uint32_t id;
>>> +        uint32_t flags;
>>> +};
> 
> [...]
>