Re: [PATCH dwarves] btf_encoder: Infer arena kfunc arguments from suffixes

On Tue, 2026-08-04 at 14:55 -0700, Ihor Solodrai wrote:
> On 8/4/26 2:34 PM, Eduard Zingerman wrote:
> > On Tue, 2026-08-04 at 14:19 -0700, Ihor Solodrai wrote:
> > > On 8/4/26 1:22 PM, Eduard Zingerman wrote:
> > > > On Tue, 2026-08-04 at 21:27 +0200, Kumar Kartikeya Dwivedi wrote:
> > > > > On Tue Aug 4, 2026 at 8:43 PM CEST, Ihor Solodrai wrote:
> > > > > > On 8/3/26 5:55 AM, Kumar Kartikeya Dwivedi wrote:
> > > > > > > [...]
> > > > > 
> > > > > > 
> > > > > > 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.
> > > > > 
> > > > > I think one major difference is that KF_ARENA_ARG* things were mostly for
> > > > > annotating the vmlinux.h with the right address space label before, but didn't
> > > > > carry any semantic meaning for the kfunc's type checks.
> > > > > 
> > > > > That changes with these suffixes though. The pointer will be translated when
> > > > > passed into the kfunc. IMO it would be odd to diverge for this particular case,
> > > > > since we use suffixes for every other case where we constrain the input type of
> > > > > the kfunc argument or give it special meaning.
> > > > > 
> > > > > We also want to have similar annotation on struct_ops callbacks, where we also
> > > > > use suffixes, so it seemed better to keep it consistent.
> > > > 
> > > > I agree that we should follow the principle of least surprise here and
> > > > use suffixes, as everything else uses suffixes as well.
> > > 
> > > Ok, I understand the motivation. Let's say we use the suffixes.
> > > 
> > > Should this enable getting rid of KF_ARENA* flags then? For the
> > > purposes of generating address_space(1), we can also just check the
> > > name suffix, no?
> > 
> > That would be ideal, yes.
> > 
> > > > 
> > > > And yes, the __arena and KF_ARENA_ARG* annotations have different
> > > > semantics:
> > > > - __arena means that user space arena address is passed as is
> > > > - KF_ARENA_ARG* means that a user space address is converted
> > > >   to a kernel space address before passing.
> > > 
> > > Also I am a little confused about whether we *need* to be able to
> > > express two distinct meanings of "arena pointer" or not?
> > > 
> > > My understanding is that "arena pointer" is a feature of an arg type
> > > that has a single meaning: the pointer has one base in BPF world, and
> > > a different base when executed in the kernel.
> > > 
> > > The things that are missing is auto-conversion (Tejun's RFC [1]) and more
> > > comprehensive support of PTR_TO_ARENA in the verifier.
> > > 
> > > This is still only one "arena" annotation per arg. Do we actually need
> > > the proliferation of __arena, __arena__nullable and/or __arena_kern,
> > > __arena_user? Can't we have a single defined semantics of how arena
> > > pointers are supposed to work?
> > > 
> > > I can imagine something like follows:
> > >   * arena pointers can not be null, check for nulls
> > >     before passing from BPF prog to the kernel
> > 
> > We are deliberately lax when handling arena and don't do any kind of
> > value tracking there. So e.g. the following won't work:
> > 
> >   if (foo->ptr) {
> >     ...
> >     kfunc(foo->ptr);
> >   }
> > 
> > Unless compiler decides to keep foo->ptr in a register. I'm not sure
> > whether enforcing non-null here from the verifier side is the right
> > call.
> 
> I don't argue for this particular semantics, it's just an example.
> 
> My point is to have unified defined rules for arena pointers, to allow
> making safe assumptions everywhere when working with them. Both as a user
> and in the kernel.
> 
> > 
> > >   * arena pointers are converted to the kernel space for
> > >     kfunc/struct_ops callback by the verifier
> > > 
> > > With the documented and enforced semantics like this one way of
> > > annotating and one annotation should be enough.
> > > 
> > > What am I missing?
> > 
> > At the moment we have two consumers:
> > - Planned sched_ext related kfuncs that need kernel space pointers.
> > - Existing kfuncs with KF_ARENA_ARG:
> >   - bpf_arena_alloc_pages
> >   - bpf_arena_free_pages
> >   - bpf_arena_reserve_pages
> >   They, take a user space address. Looking at the code is appears that
> >   all three can be changed to handle kernel space address.
> >   On the other hand, neither of these *needs* the passed pointer to be
> >   converted to a kernel side arena pointer. So that would be just some
> >   useless work.
> > 
> > So there are two valid use cases.
> 
> I think having more than one way of how to pass an arena pointer to
> the kernel will create more confusion than bring value.

That might be the case.

> It seems to me the existing bpf_arena_* kfuncs accept user space
> address for historical reasons (we just tried something that worked),
> not by design exactly.

What makes you think so?

> btw, Eduard, I get very confused by how you say "user space".. you
> mean the 32bit value representing the BPF arena pointer, right?

Nope:

  static long compute_pgoff(struct bpf_arena *arena, long uaddr)
  {
	return (u32)(uaddr - (u32)arena->user_vm_start) >> PAGE_SHIFT;
  }

  static int arena_reserve_pages(struct bpf_arena *arena, long uaddr, u32 page_cnt)
  {
	...
	if (uaddr & ~PAGE_MASK)
		return 0;

	pgoff = compute_pgoff(arena, uaddr);
	if (pgoff + page_cnt > page_cnt_max)
		return -EINVAL;
	...
   }

  __bpf_kfunc int bpf_arena_reserve_pages(void *p__map, void *ptr__ign, u32 page_cnt)
  {
	...
	return arena_reserve_pages(arena, (long)ptr__ign, page_cnt);
  }

ptr__ign/uaddr is a 64-bit user space address.

> 
> Anyways, I think we should converge on the approach to arena pointers
> handling before landing anything.
> 
> Let's use __arena suffix as annotation mechanism, fine. But
> I really wouldn't like to end up with N annotations for each
> permutation of (non-)nullable and kern/user...
> 
> I'll submit the resolve_btfids patches asap to not block on that.
> 
> 
> > 
> > ...