Re: [PATCH 3/3] selftests/bpf: Test large allocations for both sleepable/nonsleepable arena users

> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_large.c b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
> index 6ab8730d4878..e388a67d9129 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_arena_large.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_large.c

[ ... ]

> @@ -284,6 +284,7 @@ int big_alloc2(void *ctx)
>  	return 0;
>  }
>
> +/* Nonsleepable because it binds to a socket program. */
>  SEC("socket")
>  __success __retval(0)
>  int big_alloc3(void *ctx)
> @@ -300,7 +301,31 @@ int big_alloc3(void *ctx)
>  	 */
>  	pages = bpf_arena_alloc_pages(&arena, NULL, 2051, NUMA_NO_NODE, 0);
>  	if (!pages)
> -		return 0;
> +		return 1;

Is it safe to make big_alloc3 fail the test when allocation fails?

The commit message says "add extra testing for that case," but the pre-existing
big_alloc3 deliberately tolerated allocation failure with 'return 0'. This
matters because big_alloc3 is SEC("socket"), and BPF_PROG_TEST_RUN executes
socket programs with local_bh_disable():

net/bpf/test_run.c:bpf_prog_test_run_skb()
    local_bh_disable();
    ...
    ret = bpf_test_run(skb, repeat, &retval, &duration, false);
    ...
    local_bh_enable();

That makes preempt_count() > 0, so is_bpf_alloc_nonsleepable() returns true
and bpf_arena_alloc_pages() selects arena_alloc_pages_non_sleepable() (as
intended, matching the commit message "different code paths ... depending on
whether the caller is sleepable or not").

The non-sleepable path can legitimately return NULL for reasons that aren't
kernel bugs. From d1f68a079731 in this same series:

    "This forces the use of alloc_pages_nolock() in the BPF allocator ...
     This in turn causes allocation failures even in cases where falling
     into the allocator slow path and possibly sleeping would eventually
     succeed."

So a 2051-page nolock allocation can fail when ZONE_NORMAL is below the min
watermark, or memory is fragmented, or the test runs on a busy CI VM. With
'return 1', big_alloc3 would report a false regression in those cases, even
though the allocator is working as designed.

Would it make sense to keep the tolerant 'return 0' for big_alloc3 (the
non-sleepable variant that uses alloc_pages_nolock), and reserve the strict
'return 1' for the new big_alloc4 (the sleepable variant that can use the
slow path)?

[ ... ]


---
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/32709208315