[PATCH bpf-next v3 2/3] bpf: btf: Add BTF_KFUNCS_START/END macro pair

Daniel Xu posted 3 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH bpf-next v3 2/3] bpf: btf: Add BTF_KFUNCS_START/END macro pair
Posted by Daniel Xu 1 year, 11 months ago
This macro pair is functionally equivalent to BTF_SET8_START/END, except
with BTF_SET8_KFUNCS flag set in the btf_id_set8 flags field. The next
commit will codemod all kfunc set8s to this new variant such that all
kfuncs are tagged as such in .BTF_ids section.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 include/linux/btf_ids.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index dca09b7f21dc..0fe4f1cd1918 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -8,6 +8,9 @@ struct btf_id_set {
 	u32 ids[];
 };
 
+/* This flag implies BTF_SET8 holds kfunc(s) */
+#define BTF_SET8_KFUNCS		(1 << 0)
+
 struct btf_id_set8 {
 	u32 cnt;
 	u32 flags;
@@ -204,6 +207,12 @@ asm(							\
 ".popsection;                                 \n");	\
 extern struct btf_id_set8 name;
 
+#define BTF_KFUNCS_START(name)				\
+__BTF_SET8_START(name, local, BTF_SET8_KFUNCS)
+
+#define BTF_KFUNCS_END(name)				\
+BTF_SET8_END(name)
+
 #else
 
 #define BTF_ID_LIST(name) static u32 __maybe_unused name[64];
@@ -218,6 +227,8 @@ extern struct btf_id_set8 name;
 #define BTF_SET_END(name)
 #define BTF_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
 #define BTF_SET8_END(name)
+#define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
+#define BTF_KFUNCS_END(name)
 
 #endif /* CONFIG_DEBUG_INFO_BTF */
 
-- 
2.42.1
Re: [PATCH bpf-next v3 2/3] bpf: btf: Add BTF_KFUNCS_START/END macro pair
Posted by Lorenz Bauer 1 year, 11 months ago
On Sat, Jan 6, 2024 at 7:25 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> This macro pair is functionally equivalent to BTF_SET8_START/END, except
> with BTF_SET8_KFUNCS flag set in the btf_id_set8 flags field. The next
> commit will codemod all kfunc set8s to this new variant such that all
> kfuncs are tagged as such in .BTF_ids section.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  include/linux/btf_ids.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index dca09b7f21dc..0fe4f1cd1918 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -8,6 +8,9 @@ struct btf_id_set {
>         u32 ids[];
>  };
>
> +/* This flag implies BTF_SET8 holds kfunc(s) */
> +#define BTF_SET8_KFUNCS                (1 << 0)

Nit: could this be an enum so that the flag is discoverable via BTF?
Also, isn't this UAPI if pahole interprets this flag?
Re: [PATCH bpf-next v3 2/3] bpf: btf: Add BTF_KFUNCS_START/END macro pair
Posted by Daniel Xu 1 year, 11 months ago
On Mon, Jan 08, 2024 at 10:14:13AM +0100, Lorenz Bauer wrote:
> On Sat, Jan 6, 2024 at 7:25 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> >
> > This macro pair is functionally equivalent to BTF_SET8_START/END, except
> > with BTF_SET8_KFUNCS flag set in the btf_id_set8 flags field. The next
> > commit will codemod all kfunc set8s to this new variant such that all
> > kfuncs are tagged as such in .BTF_ids section.
> >
> > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> > ---
> >  include/linux/btf_ids.h | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > index dca09b7f21dc..0fe4f1cd1918 100644
> > --- a/include/linux/btf_ids.h
> > +++ b/include/linux/btf_ids.h
> > @@ -8,6 +8,9 @@ struct btf_id_set {
> >         u32 ids[];
> >  };
> >
> > +/* This flag implies BTF_SET8 holds kfunc(s) */
> > +#define BTF_SET8_KFUNCS                (1 << 0)
> 
> Nit: could this be an enum so that the flag is discoverable via BTF?

Sure, makes sense.

> Also, isn't this UAPI if pahole interprets this flag?

Not sure. I guess it'd fall under same category as any of the structs
the kernel lays out in .BTF_ids, like `struct btf_id_set8`. IMO it's
not, as that's kinda confusing to call anything in ELF uapi. Eg I don't
think people would consider layout of `.data..percpu` section uapi.

Thanks,
Daniel
Re: [PATCH bpf-next v3 2/3] bpf: btf: Add BTF_KFUNCS_START/END macro pair
Posted by Daniel Xu 1 year, 11 months ago
On Mon, Jan 08, 2024 at 10:59:53AM -0700, Daniel Xu wrote:
> On Mon, Jan 08, 2024 at 10:14:13AM +0100, Lorenz Bauer wrote:
> > On Sat, Jan 6, 2024 at 7:25 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> > >
> > > This macro pair is functionally equivalent to BTF_SET8_START/END, except
> > > with BTF_SET8_KFUNCS flag set in the btf_id_set8 flags field. The next
> > > commit will codemod all kfunc set8s to this new variant such that all
> > > kfuncs are tagged as such in .BTF_ids section.
> > >
> > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> > > ---
> > >  include/linux/btf_ids.h | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > > index dca09b7f21dc..0fe4f1cd1918 100644
> > > --- a/include/linux/btf_ids.h
> > > +++ b/include/linux/btf_ids.h
> > > @@ -8,6 +8,9 @@ struct btf_id_set {
> > >         u32 ids[];
> > >  };
> > >
> > > +/* This flag implies BTF_SET8 holds kfunc(s) */
> > > +#define BTF_SET8_KFUNCS                (1 << 0)
> > 
> > Nit: could this be an enum so that the flag is discoverable via BTF?
> 
> Sure, makes sense.

I took a look - don't think we can make it an enum. See
include/linux/btf.h:

      /* These need to be macros, as the expressions are used in assembler input */
      #define KF_ACQUIRE      (1 << 0) /* kfunc is an acquire function */
      #define KF_RELEASE      (1 << 1) /* kfunc is a release function */
      [..]

Could do some redefines but maybe not worth it. The new flag is a pretty
deep impl detail anyways.

Thanks,
Daniel