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

[Liam R. Howlett]

* Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> [230707 15:28]:
> * 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.

The check_empty_area_window issue did not show up on i386.  I was able
to reproduce it with qemu m68k nommu build.  It seems that there is a
bug in mas_rev_awalk() where mas_logical_pivot() should be used instead
of mas_safe_pivot().

It's a simple one line fix, but Peng already has a change in-flight that
will fix it by always setting the pivot and render mas_logical_pivot()
useless.

I'm going to reply to Peng's patch to amend the change log and add a Cc
stable.

The remainder of the failures you reported here will be fixed through
akpm's branch as fixes to the maple tree testing.

Thanks again, Geert for catching these issues.

Thanks,
Liam