All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ihor Solodrai <ihor.solodrai@linux.dev>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Emil Tsalapatis <emil@etsalapatis.com>, Tejun Heo <tj@kernel.org>,
	kkd@meta.com, kernel-team@meta.com
Subject: Re: [PATCH bpf-next v1 1/2] resolve_btfids: Emit arena attributes from kfunc parameter suffixes
Date: Mon, 10 Aug 2026 17:11:24 -0700	[thread overview]
Message-ID: <b6c33651-05d1-485b-9a4b-f0135723c05f@linux.dev> (raw)
In-Reply-To: <20260809085155.3305519-2-memxor@gmail.com>

On 2026-08-09 1:51 a.m., Kumar Kartikeya Dwivedi wrote:
> Kfunc declarations can identify arena arguments through parameter name
> suffixes without repeating KF_ARENA_ARG flags in their BTF ID sets.
> resolve_btfids currently misses those arguments when synthesizing the
> address_space(1) attributes used by generated vmlinux.h files.

Continuing the previous thread [1] (progs/arena_kfunc.c:42):

     ret = bpf_kfunc_arena_arg_test((u64 *)val);

I'm thinking it would be useful to have selftests that generate a
vmlinux.h-equivalent header for a test kernel module with bpftool, and
then use it in BPF progs that depend on that module. This would give
us nice additional coverage for module BTF generation.

With this patch resolve_btfids adds attrs to BTF, but nothing consumes
them, as we don't have such kfuncs in the kernel yet. So the only test
is the super-custom resolve_btfids.test.o

This is not a blocker. Just putting it out there, maybe one of the new
eager contributors will pick this up.

[1] 
https://lore.kernel.org/bpf/4d3b9137362fd9151ecdaf29d863a6cc0b0799d1.camel@gmail.com/

> 
> Teach the arena prototype rewrite to recognize __arena and
> __arena__nullable directly on each parameter. Keep KF_ARENA_ARG1 and
> KF_ARENA_ARG2 handling for explicitly flagged kfuncs, while allowing
> suffixes on any argument without synthesizing kfunc flags.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>   tools/bpf/resolve_btfids/main.c | 62 ++++++++++++++++++++++++++++-----
>   1 file changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index d2e4176339da..3c88ea192f7b 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -65,7 +65,8 @@
>    *
>    *   - emits a "bpf_kfunc" decl tag, and "bpf_fastcall" when KF_FASTCALL is set;
>    *   - wraps the return value and/or arguments flagged KF_ARENA_RET,
> - *     KF_ARENA_ARG1 or KF_ARENA_ARG2 with the "address_space(1)" type attribute;
> + *     KF_ARENA_ARG1 or KF_ARENA_ARG2, or identified by an arena parameter
> + *     suffix, with the "address_space(1)" type attribute;

nit: Documentation/bpf/kfuncs.rst:512-517 says the same thing and
still lists only the flags. I don't know if it's useful to enumerate
all the ways address_space(1) can be emitted. Maybe just say
"resolve_btfids also handles arena BTF tags" or smth.

>    *   - rewrites the prototype of KF_IMPLICIT_ARGS kfuncs.
>    *
>    * These kfunc annotations were historically produced by pahole.
> @@ -182,6 +183,8 @@ struct object {
>   #define KF_IMPLICIT_ARGS (1 << 16)
>   #define KF_IMPL_SUFFIX "_impl"
>   #define TYPE_ATTR_ARENA "address_space(1)"
> +#define PARAM_SUFFIX_ARENA "__arena"
> +#define PARAM_SUFFIX_ARENA_NULLABLE "__arena__nullable"
>   
>   struct kfunc {
>   	struct rb_node rb_node;
> @@ -1067,6 +1070,22 @@ static int collect_decl_tags(struct btf2btf_context *ctx)
>   	return 0;
>   }
>   
> +static bool param_name_has_suffix(const char *name, const char *suffix)
> +{
> +	size_t name_len = strlen(name);
> +	size_t suffix_len = strlen(suffix);
> +
> +	return name_len >= suffix_len && !strcmp(name + name_len - suffix_len, suffix);
> +}
> +
> +static bool is_arena_param(const struct btf *btf, const struct btf_param *param)
> +{
> +	const char *name = btf__name_by_offset(btf, param->name_off);
> +
> +	return param_name_has_suffix(name, PARAM_SUFFIX_ARENA) ||
> +	       param_name_has_suffix(name, PARAM_SUFFIX_ARENA_NULLABLE);
> +}
> +
>   static int collect_kfuncs(struct object *obj, struct btf2btf_context *ctx)
>   {
>   	Elf_Data *idlist = obj->efile.idlist;
> @@ -1299,8 +1318,12 @@ static int process_kfunc_with_implicit_args(struct btf2btf_context *ctx, struct
>   	return 0;
>   }
>   
> -static bool is_arena_arg(struct kfunc *kfunc, u32 idx)
> +static bool is_arena_arg(const struct btf *btf, const struct kfunc *kfunc,
> +			 const struct btf_param *param, u32 idx)
>   {
> +	if (is_arena_param(btf, param))
> +		return true;
> +
>   	switch (idx) {
>   	case 0:
>   		return kfunc->flags & KF_ARENA_ARG1;
> @@ -1311,6 +1334,30 @@ static bool is_arena_arg(struct kfunc *kfunc, u32 idx)
>   	}
>   }
>   
> +static bool kfunc_has_arena_arg(const struct btf *btf, const struct kfunc *kfunc)
> +{
> +	const struct btf_type *func, *proto;
> +	const struct btf_param *params;
> +	u32 nr_params;
> +
> +	func = btf__type_by_id(btf, kfunc->btf_id);
> +	if (!func || !btf_is_func(func))
> +		return false;
> +
> +	proto = btf__type_by_id(btf, func->type);
> +	if (!proto || !btf_is_func_proto(proto))
> +		return false;

I don't like this helper: we walk the params only to decide whether to
walk them again.

What if we change add_arena_tagged_proto() to return original proto if
nothing was tagged? I think one of my early revisions worked like
that. Then the helper could be dropped.

And returning false on bad BTF seems to silently skip the tagging,
and so far we've been failing hard on errors like that.

> +
> +	params = btf_params(proto);
> +	nr_params = btf_vlen(proto);
> +	for (u32 i = 0; i < nr_params; i++) {
> +		if (is_arena_arg(btf, kfunc, &params[i], i))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   static s32 arena_tag_ptr(struct btf *btf, u32 ptr_id, struct kfunc *kfunc)
>   {
>   	const struct btf_type *ptr = btf__type_by_id(btf, ptr_id);
> @@ -1383,11 +1430,10 @@ static s32 add_arena_tagged_proto(struct btf *btf, struct kfunc *kfunc)
>   	}
>   
>   	for (i = 0; i < nr_params; i++) {
> -		if (!is_arena_arg(kfunc, i))
> -			continue;
> -
>   		t = btf__type_by_id(btf, new_proto_id);
>   		params = btf_params(t);
> +		if (!is_arena_arg(btf, kfunc, &params[i], i))
> +			continue;
>   
>   		id = arena_tag_ptr(btf, params[i].type, kfunc);
>   		if (id < 0)
> @@ -1403,7 +1449,7 @@ static s32 add_arena_tagged_proto(struct btf *btf, struct kfunc *kfunc)
>   	return new_proto_id;
>   }
>   
> -static int process_kfunc_with_arena_flags(struct btf2btf_context *ctx,
> +static int process_kfunc_with_arena_attrs(struct btf2btf_context *ctx,
>   					  struct kfunc *kfunc)
>   {
>   	struct btf_type *t;
> @@ -1463,8 +1509,8 @@ static int btf2btf(struct object *obj)
>   				goto out;
>   		}
>   
> -		if (kfunc->flags & (KF_ARENA_RET | KF_ARENA_ARG1 | KF_ARENA_ARG2)) {
> -			err = process_kfunc_with_arena_flags(&ctx, kfunc);
> +		if ((kfunc->flags & KF_ARENA_RET) || kfunc_has_arena_arg(ctx.btf, kfunc)) {

nit: KF_ARENA_RET case short-circuits the arena_arg check here.
Seems benign, but made me pause.

> +			err = process_kfunc_with_arena_attrs(&ctx, kfunc);
>   			if (err)
>   				goto out;
>   		}


  parent reply	other threads:[~2026-08-11  0:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-09  8:51 [PATCH bpf-next v1 0/2] Add resolve_btfids support for __arena kfunc suffix Kumar Kartikeya Dwivedi
2026-08-09  8:51 ` [PATCH bpf-next v1 1/2] resolve_btfids: Emit arena attributes from kfunc parameter suffixes Kumar Kartikeya Dwivedi
2026-08-09 10:10   ` bot+bpf-ci
2026-08-09 14:08     ` Kumar Kartikeya Dwivedi
2026-08-11  0:11   ` Ihor Solodrai [this message]
2026-08-09  8:51 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test resolve_btfids arena argument suffixes Kumar Kartikeya Dwivedi
2026-08-09  9:57   ` bot+bpf-ci
2026-08-09 14:07     ` Kumar Kartikeya Dwivedi
2026-08-11  0:16   ` Ihor Solodrai

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=b6c33651-05d1-485b-9a4b-f0135723c05f@linux.dev \
    --to=ihor.solodrai@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=kernel-team@meta.com \
    --cc=kkd@meta.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.