by Liam R. Howlett

[permalink] [raw]

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

* Liam R. Howlett <[email protected]> [230707 15:28]:
> * Liam R. Howlett <[email protected]> [230704 11:22]:
> > * Peng Zhang <[email protected]> [230704 11:11]:
> > >
> > >
> > > 在 2023/7/3 02:20, Geert Uytterhoeven 写道:
> > > > Hi Liam,
> > > >
> > > > On Thu, May 18, 2023 at 9:37 PM Liam R. Howlett <[email protected]> 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 <[email protected]>
> > > >
> > > > 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

Subject: [PATCH v4 00/35] Maple tree mas_{next,prev}_range() and cleanup

Subject: [PATCH v4 08/35] maple_tree: Change RCU checks to WARN_ON() instead of BUG_ON()

Subject: [PATCH v4 12/35] maple_tree: Use MAS_BUG_ON() from mas_topiary_range()

Subject: [PATCH v4 17/35] mm: Update validate_mm() to use vma iterator

Subject: [PATCH v4 22/35] mm/mmap: Change do_vmi_align_munmap() for maple tree iterator changes

Subject: [PATCH v4 11/35] maple_tree: Use MAS_BUG_ON() in mas_set_height()

Subject: [PATCH v4 10/35] maple_tree: Use MAS_BUG_ON() when setting a leaf node as a parent

Subject: [PATCH v4 27/35] maple_tree: Introduce mas_next_slot() interface

Subject: [PATCH v4 24/35] maple_tree: Try harder to keep active node with mas_prev()

Subject: [PATCH v4 26/35] maple_tree: Fix testing mas_empty_area()

Subject: [PATCH v4 31/35] maple_tree: Add mas_prev_range() and mas_find_range_rev interface

Subject: [PATCH v4 01/35] maple_tree: Fix static analyser cppcheck issue

Subject: [PATCH v4 02/35] maple_tree: Clean up mas_parent_enum() and rename to mas_parent_type()

Subject: [PATCH v4 04/35] maple_tree: Clean up mas_dfs_postorder()

Subject: [PATCH v4 09/35] maple_tree: Convert debug code to use MT_WARN_ON() and MAS_WARN_ON()

Subject: [PATCH v4 34/35] mm: Add vma_iter_{next,prev}_range() to vma iterator

Subject: [PATCH v4 32/35] maple_tree: Clear up index and last setting in single entry tree

Subject: [PATCH v4 23/35] maple_tree: Try harder to keep active node after mas_next()

Subject: [PATCH v4 18/35] mm: Update vma_iter_store() to use MAS_WARN_ON()

Subject: [PATCH v4 05/35] maple_tree: Add format option to mt_dump()

Subject: [PATCH v4 07/35] maple_tree: Convert BUG_ON() to MT_BUG_ON()

Subject: [PATCH v4 30/35] maple_tree: Introduce mas_prev_slot() interface

Subject: [PATCH v4 29/35] maple_tree: Relocate mas_rewalk() and mas_rewalk_if_dead()

Subject: [PATCH v4 19/35] maple_tree: Add __init and __exit to test module

Subject: [PATCH v4 35/35] mm: Avoid rewalk in mmap_region

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

Subject: [PATCH v4 13/35] maple_tree: Use MAS_WR_BUG_ON() in mas_store_prealloc()

Subject: [PATCH v4 06/35] maple_tree: Add debug BUG_ON and WARN_ON variants

Subject: [PATCH v4 25/35] maple_tree: Revise limit checks in mas_empty_area{_rev}()

Subject: [PATCH v4 28/35] maple_tree: Add mas_next_range() and mas_find_range() interfaces

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

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

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

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

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

Subject: [PATCH v4 19/35] maple_tree: Add init and exit to test module