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 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).