Re: [PATCH bpf-next v3 4/6] selftests/bpf: Remove __arg_arena from the codebase

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Leftover `__arena` typedefs in header files cause their corresponding pointer types to silently drop the newly introduced `btf_type_tag("arena")` annotation.
- [Low] The removal of the fallback `__arena` macro from `bpf_arena_spin_lock.h` breaks the header's self-containment.
--

commit b79db788bee1871973404a0b3039c644cb84cce8
Author: Emil Tsalapatis <emil@etsalapatis.com>

selftests/bpf: Remove __arg_arena from the codebase

This commit removes the __arg_arena annotation by subsuming it into __arena,
which now includes a BTF type tag. To avoid compiler issues with BTF type
tags on non-pointer types, typedefs previously used for __arena are replaced
with direct pointer annotations.

> diff --git a/tools/testing/selftests/bpf/bpf_arena_htab.h b/tools/testing/selftests/bpf/bpf_arena_htab.h
> index acc01a876668e..d7ba86362d862 100644
> --- a/tools/testing/selftests/bpf/bpf_arena_htab.h
> +++ b/tools/testing/selftests/bpf/bpf_arena_htab.h
> @@ -14,9 +14,8 @@ struct htab {
>  	htab_bucket_t *buckets;
>  	int n_buckets;
>  };
> -typedef struct htab __arena htab_t;
>  
> -static inline htab_bucket_t *__select_bucket(htab_t *htab, __u32 hash)
> +static inline htab_bucket_t *__select_bucket(struct htab __arena *htab, __u32 hash)
>  {

[Severity: Medium]
Since __arena now includes __attribute__((btf_type_tag("arena"))),
applying it to a non-pointer typedef causes the compiler to drop the BTF type
tag on pointers declared with that type.

This patch removes htab_t to fix this, but does this miss other __arena
typedefs like htab_bucket_t and hashtab_elem_t in this file, or
arena_list_node_t and arena_list_head_t in bpf_arena_list.h?

Will pointers declared with those leftover typedefs silently drop their
BTF type tag annotations?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/libarena/include/bpf_arena_spin_lock.h b/tools/testing/selftests/bpf/libarena/include/bpf_arena_spin_lock.h
> index 164638690a4df..ae6b72d15bb67 100644
> --- a/tools/testing/selftests/bpf/libarena/include/bpf_arena_spin_lock.h
> +++ b/tools/testing/selftests/bpf/libarena/include/bpf_arena_spin_lock.h
> @@ -16,10 +16,6 @@
>  #define EOPNOTSUPP 95
>  #define ETIMEDOUT 110
>  
> -#ifndef __arena
> -#define __arena __attribute__((address_space(1)))
> -#endif
> -
>  extern unsigned long CONFIG_NR_CPUS __kconfig;

[Severity: Low]
Does removing this fallback definition without including bpf_arena_common.h
break the self-containment of this header?

Any source file including bpf_arena_spin_lock.h on its own might now fail
to compile due to an undeclared __arena identifier.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/arena_spin_lock.c b/tools/testing/selftests/bpf/progs/arena_spin_lock.c
> index 7236d92d382f1..cf7cda79c16c0 100644
> --- a/tools/testing/selftests/bpf/progs/arena_spin_lock.c
> +++ b/tools/testing/selftests/bpf/progs/arena_spin_lock.c
> @@ -4,6 +4,7 @@
>  #include <bpf/bpf_tracing.h>
>  #include <bpf/bpf_helpers.h>
>  #include "bpf_misc.h"
> +#include <bpf_arena_common.h>
>  #include <bpf_arena_spin_lock.h>

[Severity: Low]
This isn't a bug in this file, but this looks like a workaround for the broken
header self-containment mentioned above. Should bpf_arena_common.h be
included directly in bpf_arena_spin_lock.h instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602004120.17087-1-emil@etsalapatis.com?part=4