Re: [PATCH bpf-next v6 0/4] bpf: Add bpf_call_rcu() and bpf_call_rcu_tasks_trace()

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Wed Sep 23, 2026 at 6:59 PM CEST, Andrii Nakryiko wrote:
> On Wed, Sep 23, 2026 at 9:53 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
>>
>> On Wed Sep 23, 2026 at 6:45 PM CEST, Andrii Nakryiko wrote:
>> > On Wed, Sep 23, 2026 at 4:29 AM Kumar Kartikeya Dwivedi
>> > <memxor@gmail.com> wrote:
>> >>
>> >> On Wed Sep 23, 2026 at 12:37 PM CEST, Puranjay Mohan wrote:
>> >> > On Wed, Sep 23, 2026 at 12:41 AM Andrii Nakryiko
>> >> > <andrii.nakryiko@gmail.com> wrote:
>> >> >>
>> >> >> On Tue, Sep 22, 2026 at 1:02 PM Puranjay Mohan <puranjay@kernel.org> wrote:
>> >> >> >
>> >> >> > Changelog:
>> >> >> > v5: https://lore.kernel.org/all/20260921191407.1742386-1-puranjay@kernel.org/
>> >> >> > Changes in v6:
>> >> >> > - Size struct bpf_rcu_head at 48 bytes rather than 64, matching what the
>> >> >> >   inline state uses (Alexei)
>> >> >> > - Trim patch 1's changelog: drop the reasoning about possible future
>> >> >> >   layouts and about what the other async kfuncs return
>> >> >> > - Rebase on bpf-next/master
>> >> >> >
>> >> >> > v4: https://lore.kernel.org/all/20260915154248.3612028-1-puranjay@kernel.org/
>> >> >> > Changes in v5:
>> >> >> > - Rename the "hash_map" subtest to "bad_map" so that it matches its
>> >> >> >   helper test_call_rcu_bad_map() (bpf-ci)
>> >> >> > - Bump the callback counter after chain_err in the selftest callback.
>> >> >> >   Userspace polls that counter and then reads chain_err, so it could
>> >> >> >   still see the initial value before the re-arm had stored one, which
>> >> >> >   let the chain subtest's assertion pass without checking anything
>> >> >> > - Say in patch 1 why embedding struct rcu_head in a uapi struct is
>> >> >> >   acceptable here (Mykyta, Paul, Alexei)
>> >> >> > - Rebase on bpf-next/master
>> >> >> > v3: https://lore.kernel.org/all/20260915143640.36292-1-puranjay@kernel.org/
>> >> >> > Changes in v4:
>> >> >> > - Drop an unrelated hunk in bpf_async_update_prog_callback() that turned
>> >> >> >   PTR_ERR(prog) into -EBADF.  That is the shared bpf_timer/bpf_wq path
>> >> >> >   and bpf_prog_inc_not_zero() returns -ENOENT, so it would have changed
>> >> >> >   the errno bpf_timer_set_callback() and bpf_wq_set_callback() report to
>> >> >> >   userspace (Sashiko).  No other changes from v3
>> >> >> > v2: https://lore.kernel.org/all/20260915114240.3269184-1-puranjay@kernel.org/
>> >> >> > Changes in v3:
>> >> >> > - Move the bpf_call_rcu_tasks_trace() verifier bits from patch 1 to
>> >> >> >   patch 3; patch 1 alone emitted "resolve_btfids: unresolved symbol
>> >> >> >   bpf_call_rcu_tasks_trace" (Sashiko)
>> >> >> > - Poll the callback counter with an acquire load (Sashiko)
>> >> >> > - teardown: v2 only checked that the program was eventually freed, which
>> >> >> >   passes even if nothing was ever armed.  Also assert that the chain ran,
>> >> >> >   and read the -EPERM back through an independent .bss fd
>> >> >> > - Use kern_sync_rcu() instead of open coding the grace-period wait
>> >> >> > - Return -EBADF rather than -ENOENT when the calling program is going
>> >> >> >   away, matching bpf_task_work_schedule()
>> >> >> > - mismatch_map now pins the bpf_rcu_head label the new code emits; it
>> >> >> >   passed with that branch removed.  Add a two_heads test, and wait for
>> >> >> >   each grace period separately in the chain test
>> >> >> > - Commit messages: correct the -EPERM parity claim, explain the inline
>> >> >> >   callback state and the struct size, motivate the tasks trace flavour
>> >> >> > v1: https://lore.kernel.org/all/20260907134552.1772405-1-puranjay@kernel.org/
>> >> >> > Changes in v2:
>> >> >> > - Rebase on bpf-next/master
>> >> >> > - Use rcu_read_lock_dont_migrate() over open coding (Alexei)
>> >> >> > - Improve re-arming selftest to detect failure (Sashiko)
>> >> >> >
>> >> >> > BPF programs that manage their own objects have no way to run their own
>> >> >> > logic once an RCU grace period has elapsed.  bpf_obj_drop() defers a
>> >> >> > free, but returning an index to an allocator or unpinning a resource
>> >> >> > once readers are done has no equivalent.  sched_ext's BPF library works
>> >> >> > around this today by pushing freed nodes onto a list and having a
>> >> >> > userspace thread call membarrier(MEMBARRIER_CMD_GLOBAL) and then run a
>> >> >> > BPF program to reclaim them; it is the first intended user.
>> >> >> >
>> >> >> > Add:
>> >> >> >
>> >> >> >         int bpf_call_rcu(struct bpf_rcu_head *rh, void *map,
>> >> >> >                          int (*callback)(struct bpf_map *map, void *key,
>> >> >> >                                          void *value));
>> >> >> >
>> >> >> > and bpf_call_rcu_tasks_trace(), same signature, which also waits for
>> >> >> > sleepable programs.
>> >> >> >
>> >> >> > @rh is a struct bpf_rcu_head embedded in a value of @map, so the
>> >> >> > callback runs as callback(map, key, value) for the element it lives in
>> >> >> > and needs no cookie.  The field is only accepted in BPF_MAP_TYPE_ARRAY,
>> >> >>
>> >> >> Isn't restricting this to BPF_MAP_TYPE_ARRAY unnecessarily restrictive
>> >> >> in practice? Do other APIs of similar kind have this limitation? E.g.,
>> >> >> bpf_task_work_schedule() or timer, I don't think they limit user to
>> >> >> just ARRAY maps, it's way too inflexible.
>> >> >
>> >> > Right, bpf_timer and bpf_task_work take HASH/LRU_HASH/ARRAY. They can
>> >> > because they can cancel: on element delete bpf_obj_free_fields() calls
>> >>
>> >> bpf_obj_cancel_fields(). bpf_obj_free_fields() is no longer called except when
>> >> the object is being freed finally.
>> >>
>> >> > bpf_task_work_cancel_and_free(), which reaches task_work_cancel(), so
>> >> > a pending callback never runs against a recycled element.
>> >> > A queued RCU callback cannot be cancelled. rcu_barrier() is the only
>> >> > thing that waits for one, and it sleeps, so it is not callable from an
>> >> > element delete. Array elements are never freed individually, which is
>> >> > what makes it safe.
>> >>
>> >> If cancel is a noop, can you not simply skip it on map update?
>> >>
>> >> It is a bit unfortunate we can't provide cancel semantics, it will be yet
>> >> another divergence from how all other async callbacks work...sigh.
>> >>
>> >> >
>> >> > I can think of a complicated way to support hash maps but that would
>> >> > need making the state dynamically allocated and finding a way to
>> >> > cancel the callbacks. But I would do it as a follow-up if there is a
>> >> > real use case.
>> >>
>> >> That would mean allocation and frees, it is already quite expensive as is for a
>> >> call_rcu() primitive (with the refcount bumps and atomics).
>> >
>> > maybe so, but as is ARRAY is a severe limitation and makes a lot of
>> > cases either unusable or requiring extra sequential ID allocation
>> > logic to remap some HASH map entry to index in an ARRAY map, just so
>> > you can call rcu callback for a given kernel object stored in HASH
>> > map.
>> >
>> > E.g., think about keep as set of sockets, or tasks, or whatnot in HASH
>> > or RHASHTABLE (especially the latter one with support for resizing).
>> > How would you do bpf_call_rcu() for them with unduly complications,
>> > limitations and a lot of waste (unlike HASH you can't have lazy memory
>> > allocation).
>>
>> Yeah, I don't disagree. We would also need to support storing them in more map
>> types to make it woth with arenas (which was one of the motivations for the
>> feature).
>>
>> Perhaps it is better to make cancel a noop / unsupported and skip it when a map
>> element is deleted. That will be the easiest path to enabling it, but it does
>> create (perhaps surprising) divergence from some of the other async callback
>> primitives.
>
> I don't see much need for cancellation, but a more useful/sane
> behavior would be rearming on subsequent calls to bpf_call_rcu(). This
> would also handle update/delete/reuse of entries. Not sure how hard it
> is to support that in kernel's call_rcu() implementation, though, but
> that would solve the problem, because it should always be OK to delay
> call_rcu() callback, but not the other way around (which is what would
> happen today because we will ignore subsequent bpf_call_rcu() calls).
>

Hm. One of the design points for this was maintaining our own lists and using
more lower level RCU primitives to poll for the grace period. I wonder if some
of this could be made easier that way. I will think a bit more about this.

We could probably also do the waiting_for_gp amortization that memalloc.c on top.

>>
>> That said, I do think performance should be a consideration for this API; maybe
>> unlike other ones, I would expect that this could be called very frequently. I
>
> I'm not sure why this has to be super high frequency API to use, tbh.
> You'd normally use this when cleaning up when some kernel object is
> freed, no? Sure that can be relatively frequent, but not to the point
> where we should be *that* concerned with refcount or atomics overhead
> per se (multi-cpu cache bouncing of refcounting is a concern, but not
> sure what you can do about that).
>

Yeah it depends, but I think you can make the other case as well.  Going by
optimizations made in memalloc.c for hashtab, imagine implementing an arena hash
table using this stuff. You'd definitely want it to be as cheap as possible and
approach the kernel implementation.

If multiple CPUs dispatching it leads to constant cache line bouncing, it will
fail to scale with number of CPUs. The user then does their own batching to
amortize the cost and pace the calls, but it's just more complexity pushed down
on the callers, and it might build up memory pressure because items are now not
being freed as quickly and hit locks in the allocator (one of the main reasons
BPF maps have memory reuse semantics, to avoid exhausting caches and hitting
allocator locks).

Anyway, I know we kicked the can down the road for now on prog refcounts, and
can cache allocations etc. to amortize the cost there, but I hope that I could
illustrate why I think it might be more sensitive to performance differences
than some of the other primitives.

>> had concerns about prog refcount increment as well, but didn't really bring it
>> up since it is something that can be addressed after the fact (maybe using pcpu
>> refs). But once we promise supporting cancellation, it is hard to walk that
>> back. I think it's less of a concern for other async cb types. In practice,
>> except to support map semantics I don't know if anyone will use cancellation API
>> either, the kernel side never grew such support.
>>
>> Anyway, overall the best path to me seems to be just enabling it as is and
>
> you mean enabling for HASH or keeping it for ARRAY only? If the
> latter, my concern is that to support HASH we might need to change the
> internal structure and break that 48-byte size, so we should probably
> decide all this before we get this into the next Linux release.

I did mean enabling it in other maps, yes, sorry, I think I wrote it in a
confusing manner. I was just speculating that we can bail on cancelling and see
whether we can make it work. I probably need to spend a little more time
thinking it through.

All of that said, it seems there's a more discussion to be had about this, and
it probably landed too early. I would prefer if we could resolve these questions
without operating under some time pressure just because it might go out in the
next release.