Re: [PATCH v4 33/35] maple_tree: Update testing code for mas_{next,prev,walk}

[Liam R. Howlett]

* Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> [230704 11:22]:
> * Peng Zhang <zhangpeng.00@xxxxxxxxxxxxx> [230704 11:11]:
> > 
> > 
> > 在 2023/7/3 02:20, Geert Uytterhoeven 写道:
> > > Hi Liam,
> > > 
> > > On Thu, May 18, 2023 at 9:37 PM Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> wrote:
> > > > Now that the functions have changed the limits, update the testing of
> > > > the maple tree to test these new settings.
> > > > 
> > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> > > 
> > > Thanks for your patch, which is now commit eb2e817f38cafbf7
> > > ("maple_tree: update testing code for mas_{next,prev,walk}") in
> > > 
> > > > --- a/lib/test_maple_tree.c
> > > > +++ b/lib/test_maple_tree.c
> > > > @@ -2011,7 +2011,7 @@ static noinline void __init next_prev_test(struct maple_tree *mt)
> > > > 
> > > >          val = mas_next(&mas, ULONG_MAX);
> > > >          MT_BUG_ON(mt, val != NULL);
> > > > -       MT_BUG_ON(mt, mas.index != ULONG_MAX);
> > > > +       MT_BUG_ON(mt, mas.index != 0x7d6);
> > > 
> > > On m68k (ARAnyM):
> > > 
> > >      TEST STARTING
> > > 
> > >      BUG at next_prev_test:2014 (1)
> > >      Pass: 3749128 Run:3749129
> > > 
> > > And after that it seems to hang[*].
> > > 
> > > After adding a debug print (thus shifting all line numbers by +1):
> > > 
> > >      next_prev_test:mas.index = 0x138e
> > >      BUG at next_prev_test:2015 (1)
> > > 
> > > 0x138e = 5006, while the expected value is 0x7d6 = 2006.
> > I took a look. The return value 5006 is correct while the
> > expected value is wrong. This is a problem with the test,
> > it is not compatible with 32-bit systems.
> 
> Thanks.  There are a number of tests which deal with larger numbers that
> do not work for the 32 bit systems.  Those tests are put within an ifdef
> to avoid running.  I guess this one will either need to be altered to be
> 32 bit safe or added to that list.

This test should work on 32 bit systems.  The problem is that the test
sets up different size trees for 32 and 64 bit systems so that there are
at least two levels in the tree.  The test that fails checks what
happens when we shift off the end of a tree - which differs depending on
the number of entries needed to create a two level tree.

I have a fix for this test, but I will hold off until I test in a VM to
see the issue below.

> 
> > > 
> > > I guess converting this test to the KUnit framework would make it a
> > > bit easier to investigate failures...

I disagree, I can see the above failure in userspace on 64 bit systems
by running the following in tools/testing/radix-tree:

BUILD=32 CC=gcc make maple && LSAN_OPTIONS="report_objects=1" ./maple

In fact, that tests more than the module as it will run RCU testing as
well as using direct tree accesses for another set of tests, which
revealed another test is also failing for 32 bit around allocating
nodes.  I also have a fix for this now, but again, I'll hold off sending
them out until I see the below failures.

> > > 
> > > [*] Left the debug one running, and I got a few more:
> > > 
> > >      BUG at check_empty_area_window:2656 (1)
> > >      Pass: 3754275 Run:3754277
> > >      BUG at check_empty_area_window:2657 (1)
> > >      Pass: 3754275 Run:3754278
> > >      BUG at check_empty_area_window:2658 (1)
> > >      Pass: 3754275 Run:3754279
> > >      BUG at check_empty_area_window:2662 (1)
> > >      Pass: 3754275 Run:3754280
> > >      BUG at check_empty_area_window:2663 (1)
> > >      Pass: 3754275 Run:3754281
> > >      maple_tree: 3804518 of 3804524 tests passed
> > > 
> > > So the full test took more than 20 minutes...
> 
> There are a large number of test which are probably going to take a long
> time to run.  I'm not sure what should be limited to avoid testing
> taking a long time on old systems or even what would be acceptable?
> 

I'll look for these failures, perhaps on i386 so I can have them run at
a reasonable speed.

Thanks again for reporting this issue.  We should be more careful with
32 bit testing in future updates.

Regards,
Liam