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@xxxxxx>
Nice cleanup
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--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
>
>
