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@xxxxxxxxx> 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@xxxxxxxxxx>
> > >>
> > >> 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@xxxxxxxxxx>
> > >> ---
> > >> .../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
