[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAADnVQKZZw-e12-BOJsMMiA3s+vOskQEYRqhviQC2rBMf4AckA@mail.gmail.com>
Date: Thu, 4 Jan 2024 09:11:56 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Jiri Olsa <olsajiri@...il.com>
Cc: Daniel Xu <dxu@...uu.xyz>, Martin KaFai Lau <martin.lau@...ux.dev>,
Andrii Nakryiko <andrii@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Alexei Starovoitov <ast@...nel.org>, Quentin Monnet <quentin@...valent.com>,
Alan Maguire <alan.maguire@...cle.com>, Kumar Kartikeya Dwivedi <memxor@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: btf: Support optional flags for
BTF_SET8 sets
On Thu, Jan 4, 2024 at 3:23 AM Jiri Olsa <olsajiri@...il.com> wrote:
>
> On Wed, Jan 03, 2024 at 04:31:55PM -0700, Daniel Xu wrote:
> > This commit adds support for optional flags on BTF_SET8s.
> > struct btf_id_set8 already supported 32 bits worth of flags, but was
> > only used for alignment purposes before.
> >
> > We now use these bits to encode flags. The next commit will tag all
> > kfunc sets with a flag so that pahole can recognize which
> > BTF_ID_FLAGS(func, ..) are actual kfuncs.
> >
> > Signed-off-by: Daniel Xu <dxu@...uu.xyz>
> > ---
> > include/linux/btf_ids.h | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > index a9cb10b0e2e9..88f914579fa1 100644
> > --- a/include/linux/btf_ids.h
> > +++ b/include/linux/btf_ids.h
> > @@ -183,17 +183,21 @@ extern struct btf_id_set name;
> > * .word (1 << 3) | (1 << 1) | (1 << 2)
> > *
> > */
> > -#define __BTF_SET8_START(name, scope) \
> > +#define ___BTF_SET8_START(name, scope, flags) \
> > asm( \
> > ".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
> > "." #scope " __BTF_ID__set8__" #name "; \n" \
> > "__BTF_ID__set8__" #name ":; \n" \
> > -".zero 8 \n" \
> > +".zero 4 \n" \
> > +".long " #flags "\n" \
> > ".popsection; \n");
> >
> > -#define BTF_SET8_START(name) \
> > +#define __BTF_SET8_START(name, scope, flags, ...) \
> > +___BTF_SET8_START(name, scope, flags)
> > +
> > +#define BTF_SET8_START(name, ...) \
> > __BTF_ID_LIST(name, local) \
> > -__BTF_SET8_START(name, local)
> > +__BTF_SET8_START(name, local, ##__VA_ARGS__, 0)
>
> I think it'd better to use something like:
>
> BTF_SET8_KFUNCS_START(fsverity_set_ids)
>
> instead of:
>
> BTF_SET8_START(fsverity_set_ids, BTF_SET8_KFUNC)
>
> and to keep current BTF_SET8_START without flags argument, like:
>
> #define BTF_SET8_START(name) \
> __BTF_SET8_START(... , 0, ...
>
> #define BTF_SET8_KFUNCS_START(name) \
> __BTF_SET8_START(... , BTF_SET8_KFUNC, ...
I was about to suggest the same :)
We can drop SET8 part as well, since it's implementation detail.
Just BTF_KFUNCS_START and pair it with BTF_KFUNCS_END
that will be the same as BTF_SET8_END.
Until we need to do something else with these macros.
>
> also I'd rename BTF_SET8_KFUNC to BTF_SET8_KFUNCS (with S)
>
> do you have the pahole changes somewhere? would be great to
> see all the related changes and try the whole thing
+1
without corresponding pahole changes it's not clear whether
it actually helps.
Powered by blists - more mailing lists