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

[Jiri Olsa <olsajiri@gmail.com>]

On Sat, Nov 16, 2024 at 11:00:21AM -0800, Alexei Starovoitov wrote:
> On Fri, Nov 15, 2024 at 8:30 AM Yonghong Song <yonghong.song@linux.dev> wrote:
> >
> >
> >
> >
> > 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.
> 
> The merge window is about to open, so I pushed the fix myself.

thanks, also the [1] llvm change fixes the build for me

jirka