[PATCH 0/2] test_xarray: couple of fixes for v6-9-rc6

[PATCH 0/2] test_xarray: couple of fixes for v6-9-rc6

[Luis Chamberlain]

Andrew,

Provided there are no issues by folks with these two patches, here are a couple
of fixes which should be merged into the queue for v6.9-rc6.  The first one was
reported by Liam, after fixing that I noticed an issue which a test, and a fix
for that is on the second patch.

I wasn't even aware that you can test xarray in userspace, cool beans.

Luis Chamberlain (2):
  tools: fix userspace compilation with new test_xarray changes
  lib/test_xarray.c: fix error assumptions on
    check_xa_multi_store_adv_add()

 lib/test_xarray.c                       | 13 +++++++++----
 tools/testing/radix-tree/linux/kernel.h |  2 ++
 2 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH 1/2] tools: fix userspace compilation with new test_xarray changes

[Luis Chamberlain]

Liam reported that compiling the test_xarray on userspace was broken.
I was not even aware that was possible but you can via and you can
run these tests in userspace with:

make -C tools/testing/radix-tree
./tools/testing/radix-tree/xarray

Add the two helpers we need to fix compilation. We don't need a
userspace schedule() so just make it do nothing.

Reported-by: "Liam R. Howlett" <Liam.Howlett@oracle.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tools/testing/radix-tree/linux/kernel.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index c5c9d05f29da..c0a2bb785b92 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -18,6 +18,8 @@
 #define pr_info printk
 #define pr_debug printk
 #define pr_cont printk
+#define schedule()
+#define PAGE_SHIFT	12
 
 #define __acquires(x)
 #define __releases(x)
-- 
2.43.0

Re: [PATCH 1/2] tools: fix userspace compilation with new test_xarray changes

[Liam R. Howlett]

* Luis Chamberlain <mcgrof@kernel.org> [240423 14:05]:
> Liam reported that compiling the test_xarray on userspace was broken.
> I was not even aware that was possible but you can via and you can
> run these tests in userspace with:
> 
> make -C tools/testing/radix-tree
> ./tools/testing/radix-tree/xarray
> 
> Add the two helpers we need to fix compilation. We don't need a
> userspace schedule() so just make it do nothing.
> 
> Reported-by: "Liam R. Howlett" <Liam.Howlett@oracle.com>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Tested-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  tools/testing/radix-tree/linux/kernel.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
> index c5c9d05f29da..c0a2bb785b92 100644
> --- a/tools/testing/radix-tree/linux/kernel.h
> +++ b/tools/testing/radix-tree/linux/kernel.h
> @@ -18,6 +18,8 @@
>  #define pr_info printk
>  #define pr_debug printk
>  #define pr_cont printk
> +#define schedule()
> +#define PAGE_SHIFT	12
>  
>  #define __acquires(x)
>  #define __releases(x)
> -- 
> 2.43.0
> 

[PATCH 2/2] lib/test_xarray.c: fix error assumptions on check_xa_multi_store_adv_add()

[Luis Chamberlain]

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 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@kernel.org>
---
 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

Re: [PATCH 2/2] lib/test_xarray.c: fix error assumptions on check_xa_multi_store_adv_add()

[Liam R. Howlett]

* Luis Chamberlain <mcgrof@kernel.org> [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@kernel.org>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  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
>