Re: [PATCH v3 REBASE] xfs: Use xarray to track SB UUIDs instead of plain array.

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Mar 10, 2026 at 01:30:22PM +0100, Lukas Herbolt wrote:
> Removing the plain array to track the UUIDs and switch
> to xarray makes it more readable.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
>  changes v3:
> 	fix locking/unlocking mistakes
> 	removing wrapper function xfs_uuid_insert()
> 	rebase xfs/for-next
> 
>  fs/xfs/xfs_mount.c | 76 ++++++++++++++++++++++------------------------
>  fs/xfs/xfs_mount.h |  3 ++
>  2 files changed, 40 insertions(+), 39 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 9c295abd0a0a..b183d04da998 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -44,17 +44,36 @@
>  #include "xfs_healthmon.h"
>  
>  static DEFINE_MUTEX(xfs_uuid_table_mutex);
> -static int xfs_uuid_table_size;
> -static uuid_t *xfs_uuid_table;
> +static DEFINE_XARRAY_ALLOC(xfs_uuid_table);
> +
> +static uuid_t *
> +xfs_uuid_search(
> +	uuid_t		*new_uuid)
> +{
> +	unsigned long	index = 0;
> +	uuid_t		*uuid;
> +
> +	xa_for_each(&xfs_uuid_table, index, uuid) {
> +		if (uuid_equal(uuid, new_uuid))
> +			return uuid;
> +	}
> +	return NULL;
> +}
> +
> +static void
> +xfs_uuid_delete(
> +	uuid_t		*uuid,
> +	unsigned int	index)
> +{
> +	ASSERT(uuid_equal(xa_load(&xfs_uuid_table, index), uuid));
> +	xa_erase(&xfs_uuid_table, index);
> +}
>  
>  void
>  xfs_uuid_table_free(void)
>  {
> -	if (xfs_uuid_table_size == 0)
> -		return;
> -	kfree(xfs_uuid_table);
> -	xfs_uuid_table = NULL;
> -	xfs_uuid_table_size = 0;
> +	ASSERT(xa_empty(&xfs_uuid_table));
> +	xa_destroy(&xfs_uuid_table);
>  }
>  
>  /*
> @@ -66,7 +85,7 @@ xfs_uuid_mount(
>  	struct xfs_mount	*mp)
>  {
>  	uuid_t			*uuid = &mp->m_sb.sb_uuid;
> -	int			hole, i;
> +	int			ret;
>  
>  	/* Publish UUID in struct super_block */
>  	super_set_uuid(mp->m_super, uuid->b, sizeof(*uuid));
> @@ -80,30 +99,17 @@ xfs_uuid_mount(
>  	}
>  
>  	mutex_lock(&xfs_uuid_table_mutex);
> -	for (i = 0, hole = -1; i < xfs_uuid_table_size; i++) {
> -		if (uuid_is_null(&xfs_uuid_table[i])) {
> -			hole = i;
> -			continue;
> -		}
> -		if (uuid_equal(uuid, &xfs_uuid_table[i]))
> -			goto out_duplicate;
> -	}
> -
> -	if (hole < 0) {
> -		xfs_uuid_table = krealloc(xfs_uuid_table,
> -			(xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table),
> -			GFP_KERNEL | __GFP_NOFAIL);
> -		hole = xfs_uuid_table_size++;
> +	if (unlikely(xfs_uuid_search(uuid))) {
> +		xfs_warn(mp, "Filesystem has duplicate UUID %pU - can't mount",
> +				uuid);
> +		mutex_unlock(&xfs_uuid_table_mutex);
> +		return -EINVAL;
>  	}
> -	xfs_uuid_table[hole] = *uuid;
> -	mutex_unlock(&xfs_uuid_table_mutex);
>  
> -	return 0;
> -
> - out_duplicate:
> +	ret = xa_alloc(&xfs_uuid_table, &mp->m_uuid_table_index, uuid,
> +				xa_limit_32b, GFP_KERNEL);
>  	mutex_unlock(&xfs_uuid_table_mutex);
> -	xfs_warn(mp, "Filesystem has duplicate UUID %pU - can't mount", uuid);
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  STATIC void
> @@ -111,22 +117,14 @@ xfs_uuid_unmount(
>  	struct xfs_mount	*mp)
>  {
>  	uuid_t			*uuid = &mp->m_sb.sb_uuid;
> -	int			i;
>  
>  	if (xfs_has_nouuid(mp))
>  		return;
>  
>  	mutex_lock(&xfs_uuid_table_mutex);
> -	for (i = 0; i < xfs_uuid_table_size; i++) {
> -		if (uuid_is_null(&xfs_uuid_table[i]))
> -			continue;
> -		if (!uuid_equal(uuid, &xfs_uuid_table[i]))
> -			continue;
> -		memset(&xfs_uuid_table[i], 0, sizeof(uuid_t));
> -		break;
> -	}
> -	ASSERT(i < xfs_uuid_table_size);
> +	xfs_uuid_delete(uuid, mp->m_uuid_table_index);
>  	mutex_unlock(&xfs_uuid_table_mutex);
> +	return;

Nit: no need to add a return; at the end of a void function.

With that fixed,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index ddd4028be8d6..d964bae096ef 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -346,6 +346,9 @@ typedef struct xfs_mount {
>  
>  	/* Private data referring to a health monitor object. */
>  	struct xfs_healthmon __rcu	*m_healthmon;
> +
> +	/* Index of uuid record in the uuid xarray. */
> +	unsigned int		m_uuid_table_index;
>  } xfs_mount_t;
>  
>  #define M_IGEO(mp)		(&(mp)->m_ino_geo)
> 
> base-commit: 54fcd2f95f8d216183965a370ec69e1aab14f5da
> -- 
> 2.53.0
> 
>