Re: [PATCH bpf-next 2/2] selftests/bpf: Add a test for arena range tree algorithm

[Yonghong Song <yonghong.song@linux.dev>]

On 11/15/24 4:20 AM, Jiri Olsa wrote:
> On Thu, Nov 07, 2024 at 06:56:16PM -0800, Alexei Starovoitov wrote:
>> From: Alexei Starovoitov <ast@kernel.org>
>>
>> Add a test that verifies specific behavior of arena range tree
>> algorithm and just existing bif_alloc1 test due to use
>> of global data in arena.
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> ---
>>   .../bpf/progs/verifier_arena_large.c          | 110 +++++++++++++++++-
>>   1 file changed, 108 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_large.c b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
>> index 6065f862d964..8a9af79db884 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_arena_large.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
>> @@ -29,12 +29,12 @@ int big_alloc1(void *ctx)
>>   	if (!page1)
>>   		return 1;
>>   	*page1 = 1;
>> -	page2 = bpf_arena_alloc_pages(&arena, base + ARENA_SIZE - PAGE_SIZE,
>> +	page2 = bpf_arena_alloc_pages(&arena, base + ARENA_SIZE - PAGE_SIZE * 2,
>>   				      1, NUMA_NO_NODE, 0);
>>   	if (!page2)
>>   		return 2;
>>   	*page2 = 2;
>> -	no_page = bpf_arena_alloc_pages(&arena, base + ARENA_SIZE,
>> +	no_page = bpf_arena_alloc_pages(&arena, base + ARENA_SIZE - PAGE_SIZE,
>>   					1, NUMA_NO_NODE, 0);
>>   	if (no_page)
>>   		return 3;
>> @@ -66,4 +66,110 @@ int big_alloc1(void *ctx)
>>   #endif
>>   	return 0;
>>   }
>> +
>> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
>> +#define PAGE_CNT 100
>> +__u8 __arena * __arena page[PAGE_CNT]; /* occupies the first page */
>> +__u8 __arena *base;
>> +
>> +/*
>> + * Check that arena's range_tree algorithm allocates pages sequentially
>> + * on the first pass and then fills in all gaps on the second pass.
>> + */
>> +__noinline int alloc_pages(int page_cnt, int pages_atonce, bool first_pass,
>> +		int max_idx, int step)
>> +{
>> +	__u8 __arena *pg;
>> +	int i, pg_idx;
>> +
>> +	for (i = 0; i < page_cnt; i++) {
>> +		pg = bpf_arena_alloc_pages(&arena, NULL, pages_atonce,
>> +					   NUMA_NO_NODE, 0);
>> +		if (!pg)
>> +			return step;
>> +		pg_idx = (pg - base) / PAGE_SIZE;
> hi,
> I'm getting compile error below with clang 20.0.0:
>
>        CLNG-BPF [test_progs] verifier_arena_large.bpf.o
>      progs/verifier_arena_large.c:90:24: error: unsupported signed division, please convert to unsigned div/mod.
>         90 |                 pg_idx = (pg - base) / PAGE_SIZE;
>
> should we just convert it to unsigned div like below?
>
> also I saw recent llvm change [1] that might help, I'll give it a try

I am using latest clang 20 and compilation is successful due to the llvm change [1].

>
> jirka
>
>
> [1] 38a8000f30aa [BPF] Use mul for certain div/mod operations (#110712)
> ---
> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_large.c b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
> index 8a9af79db884..e743d008697e 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_arena_large.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
> @@ -87,7 +87,7 @@ __noinline int alloc_pages(int page_cnt, int pages_atonce, bool first_pass,
>   					   NUMA_NO_NODE, 0);
>   		if (!pg)
>   			return step;
> -		pg_idx = (pg - base) / PAGE_SIZE;
> +		pg_idx = (unsigned int) (pg - base) / PAGE_SIZE;
>   		if (first_pass) {
>   			/* Pages must be allocated sequentially */
>   			if (pg_idx != i)

I think this patch is still good.
Compiling the current verifier_arena_large.c will be okay for llvm <= 18 and >= 20.
But llvm19 will have compilation failure as you mentioned in the above.

So once bpf ci upgrades compiler to llvm19 we will see the above compilation failure.

Please verifify it as well. If this is the case in your side, please submit a patch.
Thanks!