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

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/22/26 5:06 PM, bot+bpf-ci@kernel.org wrote:
>> 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);
>> +		}
> 

Yep, this one should be inverted.
I'll send v13, if Alexei does not mind.

> 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