diff mbox series

[bpf-next,1/2] bpf: btf: Support optional flags for BTF_SET8 sets

Message ID 29644dc7906c7c0e6843d8acf92c3e29089845d0.1704324602.git.dxu@dxuuu.xyz (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Annotate kfuncs in .BTF_ids section | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2664 this patch: 2664
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 1276 this patch: 1276
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 2741 this patch: 2741
netdev/checkpatch fail ERROR: Macros with complex values should be enclosed in parentheses WARNING: line length of 88 exceeds 80 columns WARNING: macros should not use a trailing semicolon WARNING: unnecessary whitespace before a quoted newline
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc

Commit Message

Daniel Xu Jan. 3, 2024, 11:31 p.m. UTC 
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@dxuuu.xyz>
---
 include/linux/btf_ids.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Jiri Olsa Jan. 4, 2024, 11:23 a.m. UTC | #1 
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@dxuuu.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, ...


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

jirka


>  
>  #define BTF_SET8_END(name)				\
>  asm(							\
> @@ -214,7 +218,7 @@ extern struct btf_id_set8 name;
>  #define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 };
>  #define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 };
>  #define BTF_SET_END(name)
> -#define BTF_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
> +#define BTF_SET8_START(name, ...) static struct btf_id_set8 __maybe_unused name = { 0 };
>  #define BTF_SET8_END(name)
>  
>  #endif /* CONFIG_DEBUG_INFO_BTF */
> -- 
> 2.42.1
>
Alexei Starovoitov Jan. 4, 2024, 5:11 p.m. UTC | #2 
On Thu, Jan 4, 2024 at 3:23 AM Jiri Olsa <olsajiri@gmail.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@dxuuu.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.
Daniel Xu Jan. 5, 2024, 1:23 a.m. UTC | #3 
On Thu, Jan 04, 2024 at 09:11:56AM -0800, Alexei Starovoitov wrote:
> On Thu, Jan 4, 2024 at 3:23 AM Jiri Olsa <olsajiri@gmail.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@dxuuu.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.

Ack, will change.

> 
> >
> > 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.

Here's a checkpointed branch: https://github.com/danobi/pahole/tree/kfunc_btf-mailed .
I won't force push to it.

It should work against this patchset. I might need to clean it up a bit still.

Thanks,
Daniel
diff mbox series

Patch

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)
 
 #define BTF_SET8_END(name)				\
 asm(							\
@@ -214,7 +218,7 @@  extern struct btf_id_set8 name;
 #define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 };
 #define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 };
 #define BTF_SET_END(name)
-#define BTF_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
+#define BTF_SET8_START(name, ...) static struct btf_id_set8 __maybe_unused name = { 0 };
 #define BTF_SET8_END(name)
 
 #endif /* CONFIG_DEBUG_INFO_BTF */