* Luis Chamberlain <mcgrof@xxxxxxxxxx> [240423 14:05]:
> While testing lib/test_xarray in userspace I've noticed we can fail with:
>
> make -C tools/testing/radix-tree
> ./tools/testing/radix-tree/xarray
>
> BUG at check_xa_multi_store_adv_add:749
> xarray: 0x55905fb21a00x head 0x55905fa1d8e0x flags 0 marks 0 0 0
> 0: 0x55905fa1d8e0x
> xarray: ../../../lib/test_xarray.c:749: check_xa_multi_store_adv_add: Assertion `0' failed.
> Aborted
>
> We get a failure with a BUG_ON(), and that is because we actually can
> fail due to -ENOMEM, the check in xas_nomem() will fix this for us so
> it makes no sense to expect no failure inside the loop. So modify the
> check and since this is also useful for instructional purposes clarify
> the situation.
The default behaviour in the testing framework is to test the error
path, which is what you are seeing with the less likely return of
-ENOMEM.
>
> The check for XA_BUG_ON(xa, xa_load(xa, index) != p) is already done
> at the end of the loop so just remove the bogus on inside the loop.
>
> With this we now pass the test in both kernel and userspace:
>
> In userspace:
>
> ./tools/testing/radix-tree/xarray
> XArray: 149092856 of 149092856 tests passed
>
> In kernel space:
>
> XArray: 148257077 of 148257077 tests passed
>
> Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
Reviewed-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> ---
> lib/test_xarray.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/lib/test_xarray.c b/lib/test_xarray.c
> index ebe2af2e072d..5ab35190aae3 100644
> --- a/lib/test_xarray.c
> +++ b/lib/test_xarray.c
> @@ -744,15 +744,20 @@ static noinline void check_xa_multi_store_adv_add(struct xarray *xa,
>
> do {
> xas_lock_irq(&xas);
> -
> xas_store(&xas, p);
> - XA_BUG_ON(xa, xas_error(&xas));
> - XA_BUG_ON(xa, xa_load(xa, index) != p);
> -
> xas_unlock_irq(&xas);
> + /*
> + * In our selftest case the only failure we can expect is for
> + * there not to be enough memory as we're not mimicking the
> + * entire page cache, so verify that's the only error we can run
> + * into here. The xas_nomem() which follows will ensure to fix
> + * that condition for us so to chug on on the loop.
> + */
> + XA_BUG_ON(xa, xas_error(&xas) && xas_error(&xas) != -ENOMEM);
> } while (xas_nomem(&xas, GFP_KERNEL));
>
> XA_BUG_ON(xa, xas_error(&xas));
> + XA_BUG_ON(xa, xa_load(xa, index) != p);
> }
>
> /* mimics page_cache_delete() */
> --
> 2.43.0
>
