Re: [PATCH v2 0/4] hwspinlock: add summary in debugfs

[Wolfram Sang]

Hi Matthew,

> > Because the radix tree seems to have no dedicated tree nor maintainer, I
> > suggest that all these patches go in via hwspinlock. This also keeps the
> > dependencies zero.
> 
> The radix tree is deprecated.  I don't want to add any new functionality
> to it.  Here's a replacement patch to convert hwspinlock to use an
> XArray instead of a radix tree.  Compile tested only.

Okay, seems to work so far. Thank you again! Will merge your patch into
my series with your credits. Now I just need to wrap XArray into struct
seq_operations. Seems no one has needed that in the kernel so far.

Some comments and questions to get a better understanding.

> From 8ec88ed466e8153f546f7e8e69193cd5389488ee Mon Sep 17 00:00:00 2001
> From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
> Date: Mon, 22 Jun 2026 09:38:30 -0400
> Subject: [PATCH] hwspinlock: Convert to XArray
> 
> The radix tree is deprecated.  The XArray uses the same data structure
> with a nicer interface.  The hwspinlock_tree_lock is not needed as the
> spinlock built into the XArray is sufficient for all these cases.
> 
> hwspin_lock_register_single() used to always return 0.  Its caller
> thinks it can return an errno, so I believe this to be a bug and so I
> have restored its ability to return an error.

I sent a patch for that previously and would rebase your patch on mine
to keep the one patch per issue ration.

> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> ---
>  drivers/hwspinlock/hwspinlock_core.c | 133 ++++++++++-----------------
>  1 file changed, 50 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
> index cc8e952a6772..1dd68b8410dd 100644
> --- a/drivers/hwspinlock/hwspinlock_core.c
> +++ b/drivers/hwspinlock/hwspinlock_core.c
> @@ -16,7 +16,7 @@
>  #include <linux/types.h>
>  #include <linux/err.h>
>  #include <linux/jiffies.h>
> -#include <linux/radix-tree.h>
> +#include <linux/xarray.h>

According to some quick grepping, there are 102 users of XArray
including this header and 423 users which are not including this header.
Do you think this is a useful improvement to add the header directly
(per subsystem to keep the number of patches limited)?

> -	void **slot;

Great, this obsoletes a fix concerning RCU annotations I have sent
previously!

> @@ -389,15 +375,9 @@ int of_hwspin_lock_get_id(struct device_node *np, int index)
>  	/* Find the hwspinlock device: we need its base_id */
>  	ret = -EPROBE_DEFER;
>  	rcu_read_lock();
> -	radix_tree_for_each_slot(slot, &hwspinlock_tree, &iter, 0) {
> -		hwlock = radix_tree_deref_slot(slot);
> -		if (unlikely(!hwlock))
> -			continue;
> -		if (radix_tree_deref_retry(hwlock)) {
> -			slot = radix_tree_iter_retry(&iter);
> +	xas_for_each(&xas, hwlock, ULONG_MAX) {
> +		if (xas_retry(&xas, hwlock))

So, the unlikely(!hwlock) case cannot happen with XArray?

> -	ret = radix_tree_tag_get(&hwspinlock_tree, id, HWSPINLOCK_UNUSED);
> +	ret = xas_get_mark(&xas, HWSPINLOCK_UNUSED);

xas_get_mark() returns bool, so I will update the code to match that.
Makes it more readable, too, IMO.

The rest I could understand, I think. Looks much leaner, in deed. Will
keep you in the loop once my next iteration is ready.

Happy hacking,

   Wolfram