linux-kernel - Re: [PATCH v4 33/35] maple_tree: Update testing code for mas

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230710150343.el2dioaubrd2bmbr@revolver>
Date:   Mon, 10 Jul 2023 11:03:43 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Peng Zhang <zhangpeng.00@...edance.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        maple-tree@...ts.infradead.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v4 33/35] maple_tree: Update testing code for
 mas_{next,prev,walk}

* Liam R. Howlett <Liam.Howlett@...cle.com> [230707 15:28]:
> * Liam R. Howlett <Liam.Howlett@...cle.com> [230704 11:22]:
> > * Peng Zhang <zhangpeng.00@...edance.com> [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@...cle.com> 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@...cle.com>
> > > > 
> > > > 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