On Fri, 16 Feb 2024 11:43:29 -0800 Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote: > The new adanced API tests So this is a fix against the mm-unstable series "test_xarray: advanced API multi-index tests", v2. > want to vet the xarray API is doing what it > promises by manually iterating over a set of possible indexes on its > own, and using a query operation which holds the RCU lock and then > releases it. So it is not using the helper loop options which xarray > provides on purpose. Any loop which iterates over 1 million entries > (which is possible with order 20, so emulating say a 4 GiB block size) > to just to rcu lock and unlock will eventually end up triggering a soft > lockup on systems which don't preempt, and have lock provin and RCU > prooving enabled. > > xarray users already use XA_CHECK_SCHED for loops which may take a long > time, in our case we don't want to RCU unlock and lock as the caller > does that already, but rather just force a schedule every XA_CHECK_SCHED > iterations since the test is trying to not trust and rather test that > xarray is doing the right thing. > > [0] https://lkml.kernel.org/r/202402071613.70f28243-lkp@xxxxxxxxx > > Reported-by: kernel test robot <oliver.sang@xxxxxxxxx> As the above links shows, this should be Reported-by: kernel test robot <oliver.sang@xxxxxxxxx> Closes: https://lore.kernel.org/oe-lkp/202402071613.70f28243-lkp@xxxxxxxxx > --- a/lib/test_xarray.c > +++ b/lib/test_xarray.c > @@ -781,6 +781,7 @@ static noinline void *test_get_entry(struct xarray *xa, unsigned long index) > { > XA_STATE(xas, xa, index); > void *p; > + static unsigned int i = 0; I don't think this needs static storage. PetPeeve: it is unexpected that `i' has unsigned type. Can a more communicative identifier be used? I shall queue your patch as a fixup patch against test_xarray-add-tests-for-advanced-multi-index-use and shall add the below on top. Pleae check. --- a/lib/test_xarray.c~test_xarray-fix-soft-lockup-for-advanced-api-tests-fix +++ a/lib/test_xarray.c @@ -728,7 +728,7 @@ static noinline void *test_get_entry(str { XA_STATE(xas, xa, index); void *p; - static unsigned int i = 0; + unsigned int loops = 0; rcu_read_lock(); repeat: @@ -746,7 +746,7 @@ repeat: * APIs won't be stupid, proper page cache APIs loop over the proper * order so when using a larger order we skip shared entries. */ - if (++i % XA_CHECK_SCHED == 0) + if (++loops % XA_CHECK_SCHED == 0) schedule(); return p; _
