On Tue, Aug 20, 2024 at 07:04:53PM +0200, Christoph Hellwig wrote:
> xfs_attr3_leaf_add only has two potential return values, indicating if the
> entry could be added or not. Replace the errno return with a bool so that
> ENOSPC from it can't easily be confused with a real ENOSPC.
>
> Remove the return value from the xfs_attr3_leaf_add_work helper entirely,
> as it always return 0.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 12 ++++--------
> fs/xfs/libxfs/xfs_attr_leaf.c | 37 ++++++++++++++++++-----------------
> fs/xfs/libxfs/xfs_attr_leaf.h | 2 +-
> 3 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index b9df7a6b1f9d61..c3ddea016eac95 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -557,10 +557,7 @@ xfs_attr_leaf_addname(
> * or perform more xattr manipulations. Otherwise there is nothing more
> * to do and we can return success.
> */
> - error = xfs_attr3_leaf_add(bp, args);
> - if (error) {
> - if (error != -ENOSPC)
> - return error;
> + if (!xfs_attr3_leaf_add(bp, args)) {
> error = xfs_attr3_leaf_to_node(args);
> if (error)
> return error;
> @@ -574,7 +571,7 @@ xfs_attr_leaf_addname(
> }
>
> trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp);
> - return error;
> + return 0;
>
> out_brelse:
> xfs_trans_brelse(args->trans, bp);
> @@ -1399,15 +1396,14 @@ xfs_attr_node_try_addname(
> {
> struct xfs_da_state *state = attr->xattri_da_state;
> struct xfs_da_state_blk *blk;
> - int error;
> + int error = 0;
>
> trace_xfs_attr_node_addname(state->args);
>
> blk = &state->path.blk[state->path.active-1];
> ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
>
> - error = xfs_attr3_leaf_add(blk->bp, state->args);
> - if (error == -ENOSPC) {
> + if (!xfs_attr3_leaf_add(blk->bp, state->args)) {
> if (state->path.active == 1) {
> /*
> * Its really a single leaf node, but it had
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index b9e98950eb3d81..bcaf28732bfcae 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -47,7 +47,7 @@
> */
> STATIC int xfs_attr3_leaf_create(struct xfs_da_args *args,
> xfs_dablk_t which_block, struct xfs_buf **bpp);
> -STATIC int xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
> +STATIC void xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
> struct xfs_attr3_icleaf_hdr *ichdr,
> struct xfs_da_args *args, int freemap_index);
> STATIC void xfs_attr3_leaf_compact(struct xfs_da_args *args,
> @@ -995,10 +995,8 @@ xfs_attr_shortform_to_leaf(
> xfs_attr_sethash(&nargs);
> error = xfs_attr3_leaf_lookup_int(bp, &nargs); /* set a->index */
> ASSERT(error == -ENOATTR);
Huh. xfs_attr3_leaf_lookup_int can return -EFSCORRUPTED here, but we
just ignore it. I guess we've never seen the debug assert go off so
it's not a real issue, but it does make my eyes twitch. :P
> - error = xfs_attr3_leaf_add(bp, &nargs);
> - ASSERT(error != -ENOSPC);
> - if (error)
> - goto out;
> + if (!xfs_attr3_leaf_add(bp, &nargs))
> + ASSERT(0);
Cleaner callsites, all right!
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> sfe = xfs_attr_sf_nextentry(sfe);
> }
> error = 0;
> @@ -1343,8 +1341,9 @@ xfs_attr3_leaf_split(
> struct xfs_da_state_blk *oldblk,
> struct xfs_da_state_blk *newblk)
> {
> - xfs_dablk_t blkno;
> - int error;
> + bool added;
> + xfs_dablk_t blkno;
> + int error;
>
> trace_xfs_attr_leaf_split(state->args);
>
> @@ -1379,10 +1378,10 @@ xfs_attr3_leaf_split(
> */
> if (state->inleaf) {
> trace_xfs_attr_leaf_add_old(state->args);
> - error = xfs_attr3_leaf_add(oldblk->bp, state->args);
> + added = xfs_attr3_leaf_add(oldblk->bp, state->args);
> } else {
> trace_xfs_attr_leaf_add_new(state->args);
> - error = xfs_attr3_leaf_add(newblk->bp, state->args);
> + added = xfs_attr3_leaf_add(newblk->bp, state->args);
> }
>
> /*
> @@ -1390,13 +1389,15 @@ xfs_attr3_leaf_split(
> */
> oldblk->hashval = xfs_attr_leaf_lasthash(oldblk->bp, NULL);
> newblk->hashval = xfs_attr_leaf_lasthash(newblk->bp, NULL);
> - return error;
> + if (!added)
> + return -ENOSPC;
> + return 0;
> }
>
> /*
> * Add a name to the leaf attribute list structure.
> */
> -int
> +bool
> xfs_attr3_leaf_add(
> struct xfs_buf *bp,
> struct xfs_da_args *args)
> @@ -1405,6 +1406,7 @@ xfs_attr3_leaf_add(
> struct xfs_attr3_icleaf_hdr ichdr;
> int tablesize;
> int entsize;
> + bool added = true;
> int sum;
> int tmp;
> int i;
> @@ -1433,7 +1435,7 @@ xfs_attr3_leaf_add(
> if (ichdr.freemap[i].base < ichdr.firstused)
> tmp += sizeof(xfs_attr_leaf_entry_t);
> if (ichdr.freemap[i].size >= tmp) {
> - tmp = xfs_attr3_leaf_add_work(bp, &ichdr, args, i);
> + xfs_attr3_leaf_add_work(bp, &ichdr, args, i);
> goto out_log_hdr;
> }
> sum += ichdr.freemap[i].size;
> @@ -1445,7 +1447,7 @@ xfs_attr3_leaf_add(
> * no good and we should just give up.
> */
> if (!ichdr.holes && sum < entsize)
> - return -ENOSPC;
> + return false;
>
> /*
> * Compact the entries to coalesce free space.
> @@ -1458,24 +1460,24 @@ xfs_attr3_leaf_add(
> * free region, in freemap[0]. If it is not big enough, give up.
> */
> if (ichdr.freemap[0].size < (entsize + sizeof(xfs_attr_leaf_entry_t))) {
> - tmp = -ENOSPC;
> + added = false;
> goto out_log_hdr;
> }
>
> - tmp = xfs_attr3_leaf_add_work(bp, &ichdr, args, 0);
> + xfs_attr3_leaf_add_work(bp, &ichdr, args, 0);
>
> out_log_hdr:
> xfs_attr3_leaf_hdr_to_disk(args->geo, leaf, &ichdr);
> xfs_trans_log_buf(args->trans, bp,
> XFS_DA_LOGRANGE(leaf, &leaf->hdr,
> xfs_attr3_leaf_hdr_size(leaf)));
> - return tmp;
> + return added;
> }
>
> /*
> * Add a name to a leaf attribute list structure.
> */
> -STATIC int
> +STATIC void
> xfs_attr3_leaf_add_work(
> struct xfs_buf *bp,
> struct xfs_attr3_icleaf_hdr *ichdr,
> @@ -1593,7 +1595,6 @@ xfs_attr3_leaf_add_work(
> }
> }
> ichdr->usedbytes += xfs_attr_leaf_entsize(leaf, args->index);
> - return 0;
> }
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
> index bac219589896ad..589f810eedc0d8 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.h
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.h
> @@ -76,7 +76,7 @@ int xfs_attr3_leaf_split(struct xfs_da_state *state,
> int xfs_attr3_leaf_lookup_int(struct xfs_buf *leaf,
> struct xfs_da_args *args);
> int xfs_attr3_leaf_getvalue(struct xfs_buf *bp, struct xfs_da_args *args);
> -int xfs_attr3_leaf_add(struct xfs_buf *leaf_buffer,
> +bool xfs_attr3_leaf_add(struct xfs_buf *leaf_buffer,
> struct xfs_da_args *args);
> int xfs_attr3_leaf_remove(struct xfs_buf *leaf_buffer,
> struct xfs_da_args *args);
> --
> 2.43.0
>
>
