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

[Christoph Hellwig <hch@infradead.org>]

> +STATIC int

We're phasing out the magic STATIC, so pleae just use plain static
here.

> +xfs_uuid_insert(uuid_t *uuid)

The somewhat unique XFS coding style uses separate lines for each
argument:

static int
xfs_uuid_insert(
	uuid_t			*uuid)

> +{
> +	uint32_t index = 0;

.. and aligns the variables the same way:

{
	uint32_t		index = 0;

Although this one will go away with Darrick's suggestion anyway.

> +
> +	return xa_alloc(&xfs_uuid_table, &index, uuid,
> +			xa_limit_32b, GFP_KERNEL);
> +}
> +
> +STATIC uuid_t
> +*xfs_uuid_search(uuid_t *new_uuid)

The * for pointers goes with the type:

static uuid_t *
xfs_uuid_search(
	uuid_t			*new_uuid)

> +	uuid_t *uuid = NULL;

no need to initialize the iterator to NULL before xa_for_each.

> +STATIC void
> +xfs_uuid_delete(uuid_t *uuid)
> +{
> +	unsigned long index = 0;
> +
> +	xa_for_each(&xfs_uuid_table, index, uuid) {
> +		xa_erase(&xfs_uuid_table, index);
> +	}

I don't think this works as expected, as it just erases all uuids in the
table.

> +}
>  
>  void
> -xfs_uuid_table_free(void)
> +xfs_uuid_table_destroy(void)

I'd drop this rename.  Free works just fine here as a name.

> +	if (!xfs_uuid_search(uuid))
> +		return xfs_uuid_insert(uuid);
>  
>  	xfs_warn(mp, "Filesystem has duplicate UUID %pU - can't mount", uuid);
>  	return -EINVAL;

Just return an error here if xfs_uuid_search finds something, and then
open code the insert in the straight line path.

> @@ -110,22 +119,12 @@ xfs_uuid_unmount(
>  	struct xfs_mount	*mp)
>  {
>  	uuid_t			*uuid = &mp->m_sb.sb_uuid;
>  
>  	if (xfs_has_nouuid(mp))
>  		return;
> +	xfs_uuid_delete(uuid);
> +	return;

No need for the last return.  Also I think you can just open code
xfs_uuid_delete here.