Re: [PATCH bpf-next v12 2/6] bpf: Add bpf_prog_run_array_sleepable()

bot+bpf-ci
2026-04-22T16:06:12Z

[-- Attachment #1: Type: text/plain, Size: 3735 bytes --]

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3079,6 +3079,56 @@ void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr);
> void bpf_dynptr_set_rdonly(struct bpf_dynptr_kern *ptr);
> void bpf_prog_report_arena_violation(bool write, unsigned long addr, unsigned long fault_ip);
>
> +static __always_inline u32
> +bpf_prog_run_array_sleepable(const struct bpf_prog_array *array,
> + const void *ctx, bpf_prog_run_fn run_prog)
> +{
> + const struct bpf_prog_array_item *item;
> + struct bpf_prog *prog;
> + struct bpf_run_ctx *old_run_ctx;
> + struct bpf_trace_run_ctx run_ctx;
> + u32 ret = 1;
> +
> + if (unlikely(!array))
> + return ret;
> +
> + migrate_disable();
> +
> + run_ctx.is_uprobe = false;
> +
> + old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> + item = &array->items[0];
> + while ((prog = READ_ONCE(item->prog))) {
> + /* Skip dummy_bpf_prog placeholder (len == 0) */
> + if (unlikely(!prog->len)) {
> + item++;
> + continue;
> + }
> +
> + if (unlikely(!bpf_prog_get_recursion_context(prog))) {
> + bpf_prog_inc_misses_counter(prog);
> + bpf_prog_put_recursion_context(prog);
> + item++;
> + continue;
> + }
> +
> + run_ctx.bpf_cookie = item->bpf_cookie;
> +
> + if (prog->sleepable) {
> + guard(rcu)();
> + ret &= run_prog(prog, ctx);
> + } else {
> + ret &= run_prog(prog, ctx);
> + }

The sleepable/non-sleepable branches appear inverted relative to
bpf_prog_run_array_uprobe(). In the uprobe sibling at
include/linux/bpf.h:2491-2526, classic RCU is acquired only for
non-sleepable programs:

while ((prog = READ_ONCE(item->prog))) {
if (!prog->sleepable)
rcu_read_lock();
run_ctx.bpf_cookie = item->bpf_cookie;
ret &= run_prog(prog, ctx);
item++;
if (!prog->sleepable)
rcu_read_unlock();
}

Here, the condition takes classic RCU (guard(rcu)() expands to
rcu_read_lock()/rcu_read_unlock() via DEFINE_LOCK_GUARD_0 in
include/linux/rcupdate.h:1181) around sleepable programs while taking no
RCU read-side lock for non-sleepable ones.

Sleepable BPF programs are designed to fault and sleep. Executing them
inside a classic RCU read-side critical section violates the RCU
requirement that classic readers must not block. On CONFIG_PREEMPT_RCU=n
kernels this can cause RCU stalls.

Meanwhile, non-sleepable BPF programs often rely on classic-RCU-protected
structures (like classic RCU-protected map entries). Invoking them with no
classic RCU read-side critical section weakens protection. The caller added
in the subsequent commit 864dee5d55d1 holds only guard(rcu_tasks_trace)(),
so no outer classic RCU protects these accesses.

Should the condition be inverted to match the uprobe pattern?

This was raised by Alexei Starovoitov in v11 review:
https://lore.kernel.org/bpf/DHZ4I3RWVVCU.16XRZX1HPFI8K@gmail.com/

Alexei suggested in his April 21st comment:

"Why grab rcu_read_lock() and undo it?
imo it would be cleaner and faster to do
bpf_prog_get_recursion_context() here ...

... and then here:
if (!prog->sleepable) {
guard(rcu)();
ret &= run_prog(prog, ctx);
} else {
ret &= run_prog(prog, ctx);
}"

Was this feedback addressed in v12?

> +
> + bpf_prog_put_recursion_context(prog);
> + item++;
> + }
> + bpf_reset_run_ctx(old_run_ctx);
> + migrate_enable();
> + return ret;
> +}
> +
> #else /* !CONFIG_BPF_SYSCALL */
> static inline struct bpf_prog *bpf_prog_get(u32 ufd)
> {

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24787752194