BPF List
 help / color / mirror / Atom feed
From: Ihor Solodrai <ihor.solodrai@linux.dev>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Alan Maguire <alan.maguire@oracle.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	dwarves@vger.kernel.org
Cc: bpf@vger.kernel.org, Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>, Tejun Heo <tj@kernel.org>,
	Emil Tsalapatis <emil@etsalapatis.com>
Subject: Re: [PATCH dwarves] btf_encoder: Infer arena kfunc arguments from suffixes
Date: Tue, 4 Aug 2026 11:43:34 -0700	[thread overview]
Message-ID: <4cb8bb30-1f01-4b78-a6b1-4ade3b965039@linux.dev> (raw)
In-Reply-To: <20260803125518.2279340-1-memxor@gmail.com>

On 8/3/26 5:55 AM, Kumar Kartikeya Dwivedi wrote:
> The kernel verifier recognizes __arena and __arena_nullable parameter
> suffixes for registered kfuncs. These arguments need the matching
> address_space(1) BTF type attribute so bpftool emits usable declarations.

Hi Kartikeya,

+cc: Emil, Tejun

This patch is certainly a no-go, because of the ongoing effort to move
decl/type tag BTF generation from pahole to resolve_btfids [1][2]. I'm
going to send the last unlanded bits of that soon.

*If* we decide to make this change, it shouldn't be done in pahole.

But even setting that aside:

> The kernel verifier recognizes __arena and __arena_nullable
> parameter suffixes for registered kfuncs.

This is not true. The only way the kernel can recognize an arena
argument is via one of the three kfunc flags: KF_ARENA_RET,
KF_ARENA_ARG1 and KF_ARENA_ARG2. No __arena suffix support exist:

  $ git log --oneline -n1
  7f333f85f83d (HEAD -> bpf-next, origin/for-next, origin/bpf-next, bpf-next/master, bpf-next/for-next, bpf-next/HEAD) Merge branch 'bpf-invalidate-rcu-pointers-after-final-spin-unlock'
  $ grep -r --include="*.[ch]" __arena  kernel/bpf/
    # ...nothing

__arena symbol is only used in sched_ext, libarena and selftests code
as an alias to __atrribute__((address_space(1))) or a type tag:

  $ grep -r --include="*.[ch]" 'define __arena '
  tools/sched_ext/include/scx/bpf_arena_common.bpf.h:#define __arena __attribute__((address_space(1)))
  tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h:#define __arena __attribute__((address_space(1))) __attribute__((btf_type_tag("arena")))
  tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h:#define __arena __attribute__((btf_type_tag("arena")))

AFAIR prior discussions that led to KF_ARENA_* flags implementation,
we decided to *not* add an __arena arg suffix support. We were talking
about getting rid of this suffix-annotation mechanism completely.

What we want long term is proper decl/type tags support from
compilers, so that in the kernel we could have and use:

  #define __arena __attribute__((btf_type_tag("arena")))

At the time KF_ARENA_* flags were introduced, this wasn't feasible
because GCC compiler didn't support the tags. I think it does since
recently, but even so we'll have to support older compiler builds for
quite a while.

So while I understand the reluctance to add KF_ARENA_ARG3..N, I don't
think we want to introduce and support yet another mechanism for arena
argument annotations. If we do, we'll be stuck with a mess of
supporting two/three ways of doing the same thing for the foreseeable future.

Maybe adding support for proper type-tagging is worthwhile long-term,
but not the name suffix.

[1] https://lore.kernel.org/bpf/20260601221805.821394-1-ihor.solodrai@linux.dev/
[2] https://lore.kernel.org/bpf/20260722233518.778854-1-ihor.solodrai@linux.dev/


> 
> Extend the existing KF_ARENA_ARG1/2 handling to select arguments by either
> the legacy flag or either suffix. Iterate over all parameters, allowing the
> suffix convention at any argument position and avoiding duplicate tags when
> a flag and suffix select the same argument.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  btf_encoder.c | 38 ++++++++++++++++++++++++++++----------
>  dutil.h       | 13 +++++++++++++
>  2 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 4b422e09800f..07ca4f41ac32 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -42,6 +42,8 @@
>  #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
>  #define BTF_FASTCALL_TAG       "bpf_fastcall"
>  #define BPF_ARENA_ATTR         "address_space(1)"
> +#define BPF_ARENA_SUFFIX       "__arena"
> +#define BPF_ARENA_NULLABLE_SUFFIX "__arena_nullable"
>  
>  /* kfunc flags, see include/linux/btf.h in the kernel source */
>  #define KF_FASTCALL   (1 << 12)
> @@ -808,12 +810,32 @@ static int btf__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state
>  	return id;
>  }
>  
> -/* Modifies state->ret_type_id and state->parms[i].type_id for flagged kfuncs */
> +static bool btf__is_bpf_arena_arg(const struct btf *btf,
> +				  const struct btf_encoder_func_state *state, int idx)
> +{
> +	uint32_t flags = state->elf->kfunc_flags;
> +	const char *name;
> +	size_t name_len;
> +
> +	if ((idx == 0 && (flags & KF_ARENA_ARG1)) ||
> +	    (idx == 1 && (flags & KF_ARENA_ARG2)))
> +		return true;
> +
> +	name = btf__name_by_offset(btf, state->parms[idx].name_off);
> +	if (!name)
> +		return false;
> +	name_len = strlen(name);
> +	return (name_len > sizeof(BPF_ARENA_SUFFIX) - 1 && strends(name, BPF_ARENA_SUFFIX)) ||
> +	       (name_len > sizeof(BPF_ARENA_NULLABLE_SUFFIX) - 1 &&
> +		strends(name, BPF_ARENA_NULLABLE_SUFFIX));
> +}
> +
> +/* Modifies state->ret_type_id and state->parms[i].type_id for arena kfuncs */
>  static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func_state *state)
>  {
>  	uint32_t flags = state->elf->kfunc_flags;
>  	int ret_type_id;
> -	int err;
> +	int err, i;
>  
>  	if (!btf__add_type_attr) {
>  		fprintf(stderr, "btf__add_type_attr is not available, is libbpf < 1.6?\n");
> @@ -830,14 +852,10 @@ static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func
>  		state->ret_type_id = ret_type_id;
>  	}
>  
> -	if (KF_ARENA_ARG1 & flags) {
> -		err = btf__tag_bpf_arena_arg(btf, state, 0);
> -		if (err < 0)
> -			return err;
> -	}
> -
> -	if (KF_ARENA_ARG2 & flags) {
> -		err = btf__tag_bpf_arena_arg(btf, state, 1);
> +	for (i = 0; i < state->nr_parms; i++) {
> +		if (!btf__is_bpf_arena_arg(btf, state, i))
> +			continue;
> +		err = btf__tag_bpf_arena_arg(btf, state, i);
>  		if (err < 0)
>  			return err;
>  	}
> diff --git a/dutil.h b/dutil.h
> index 603556fa0308..d55d01abc842 100644
> --- a/dutil.h
> +++ b/dutil.h
> @@ -335,6 +335,19 @@ static inline bool strstarts(const char *str, const char *prefix)
>  	return strncmp(str, prefix, strlen(prefix)) == 0;
>  }
>  
> +/**
> + * strends - does @str end with @suffix?
> + * @str: string to examine
> + * @suffix: suffix to look for.
> + */
> +static inline bool strends(const char *str, const char *suffix)
> +{
> +	size_t str_len = strlen(str);
> +	size_t suffix_len = strlen(suffix);
> +
> +	return suffix_len <= str_len && strcmp(str + str_len - suffix_len, suffix) == 0;
> +}
> +
>  void *zalloc(const size_t size);
>  
>  Elf_Scn *elf_section_by_name(Elf *elf, GElf_Shdr *shp, const char *name, size_t *index);


  reply	other threads:[~2026-08-04 18:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-03 12:55 [PATCH dwarves] btf_encoder: Infer arena kfunc arguments from suffixes Kumar Kartikeya Dwivedi
2026-08-04 18:43 ` Ihor Solodrai [this message]
2026-08-04 19:27   ` Kumar Kartikeya Dwivedi
2026-08-04 20:22     ` Eduard Zingerman
2026-08-04 21:19       ` Ihor Solodrai
2026-08-04 21:34         ` Eduard Zingerman
2026-08-04 21:46           ` Kumar Kartikeya Dwivedi
2026-08-04 21:51             ` Eduard Zingerman
2026-08-04 21:57               ` Kumar Kartikeya Dwivedi
2026-08-04 22:57               ` Ihor Solodrai
2026-08-04 23:17                 ` Kumar Kartikeya Dwivedi
2026-08-04 21:55           ` Ihor Solodrai
2026-08-04 22:05             ` Eduard Zingerman
2026-08-04 22:13               ` Ihor Solodrai
2026-08-04 21:36         ` Kumar Kartikeya Dwivedi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4cb8bb30-1f01-4b78-a6b1-4ade3b965039@linux.dev \
    --to=ihor.solodrai@linux.dev \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=memxor@gmail.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox