Re: [PATCH 3/6] xfs: distinguish extra split from real ENOSPC from xfs_attr3_leaf_split

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

On Tue, Aug 20, 2024 at 07:04:54PM +0200, Christoph Hellwig wrote:
> xfs_attr3_leaf_split propagates the need for an extra btree split as
> -ENOSPC to it's only caller, but the same return value can also be
> returned from xfs_da_grow_inode when it fails to find free space.
> 
> Distinguish the two cases by returning 1 for the extra split case instead
> of overloading -ENOSPC.
> 
> This can be triggered relatively easily with the pending realtime group
> support and a file system with a lot of small zones that use metadata
> space on the main device.  In this case every about 5-10th run of
> xfs/538 runs into the following assert:
> 
> 	ASSERT(oldblk->magic == XFS_ATTR_LEAF_MAGIC);
> 
> in xfs_attr3_leaf_split caused by an allocation failure.  Note that
> the allocation failure is caused by another bug that will be fixed
> subsequently, but this commit at least sorts out the error handling.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Nice cleanup
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 5 ++++-
>  fs/xfs/libxfs/xfs_da_btree.c  | 5 +++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index bcaf28732bfcae..92acef51876e4b 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -1334,6 +1334,9 @@ xfs_attr3_leaf_create(
>  
>  /*
>   * Split the leaf node, rebalance, then add the new entry.
> + *
> + * Returns 0 if the entry was added, 1 if a further split is needed or a
> + * negative error number otherwise.
>   */
>  int
>  xfs_attr3_leaf_split(
> @@ -1390,7 +1393,7 @@ xfs_attr3_leaf_split(
>  	oldblk->hashval = xfs_attr_leaf_lasthash(oldblk->bp, NULL);
>  	newblk->hashval = xfs_attr_leaf_lasthash(newblk->bp, NULL);
>  	if (!added)
> -		return -ENOSPC;
> +		return 1;
>  	return 0;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 16a529a8878083..17d9e6154f1978 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -593,9 +593,8 @@ xfs_da3_split(
>  		switch (oldblk->magic) {
>  		case XFS_ATTR_LEAF_MAGIC:
>  			error = xfs_attr3_leaf_split(state, oldblk, newblk);
> -			if ((error != 0) && (error != -ENOSPC)) {
> +			if (error < 0)
>  				return error;	/* GROT: attr is inconsistent */
> -			}
>  			if (!error) {
>  				addblk = newblk;
>  				break;
> @@ -617,6 +616,8 @@ xfs_da3_split(
>  				error = xfs_attr3_leaf_split(state, newblk,
>  							    &state->extrablk);
>  			}
> +			if (error == 1)
> +				return -ENOSPC;
>  			if (error)
>  				return error;	/* GROT: attr inconsistent */
>  			addblk = newblk;
> -- 
> 2.43.0
> 
>