On Saturday, April 4, 2020 3:42 AM Allison Collins wrote:
> This patch adds a new helper function xfs_attr_node_shrink used to
> shrink an attr name into an inode if it is small enough. This helps to
> modularize the greater calling function xfs_attr_node_removename.
>
> Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 67 ++++++++++++++++++++++++++++++------------------
> 1 file changed, 42 insertions(+), 25 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index db5a99c..27a9bb5 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1103,6 +1103,45 @@ xfs_attr_node_addname(
> }
>
> /*
> + * Shrink an attribute from leaf to shortform
> + */
> +STATIC int
> +xfs_attr_node_shrink(
> + struct xfs_da_args *args,
> + struct xfs_da_state *state)
> +{
> + struct xfs_inode *dp = args->dp;
> + int error, forkoff;
> + struct xfs_buf *bp;
> +
> + /*
> + * Have to get rid of the copy of this dabuf in the state.
> + */
> + ASSERT(state->path.active == 1);
> + ASSERT(state->path.blk[0].bp);
> + state->path.blk[0].bp = NULL;
> +
> + error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
> + if (error)
> + return error;
> +
> + forkoff = xfs_attr_shortform_allfit(bp, dp);
> + if (forkoff) {
> + error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
> + /* bp is gone due to xfs_da_shrink_inode */
> + if (error)
> + return error;
> +
> + error = xfs_defer_finish(&args->trans);
> + if (error)
> + return error;
> + } else
> + xfs_trans_brelse(args->trans, bp);
> +
> + return 0;
> +}
> +
> +/*
> * Remove a name from a B-tree attribute list.
> *
> * This will involve walking down the Btree, and may involve joining
> @@ -1115,8 +1154,7 @@ xfs_attr_node_removename(
> {
> struct xfs_da_state *state;
> struct xfs_da_state_blk *blk;
> - struct xfs_buf *bp;
> - int retval, error, forkoff;
> + int retval, error;
> struct xfs_inode *dp = args->dp;
>
> trace_xfs_attr_node_removename(args);
> @@ -1197,31 +1235,10 @@ xfs_attr_node_removename(
> /*
> * If the result is small enough, push it all into the inode.
> */
> - if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> - /*
> - * Have to get rid of the copy of this dabuf in the state.
> - */
> - ASSERT(state->path.active == 1);
> - ASSERT(state->path.blk[0].bp);
> - state->path.blk[0].bp = NULL;
> -
> - error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
> - if (error)
> - goto out;
> + if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
> + error = xfs_attr_node_shrink(args, state);
If a non-zero error value is returned by the above statement, the following
statement i.e. "error = 0" will overwrite it.
Apart from that everything else looks fine.
Reviewed-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx>
>
> - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
> - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
> - /* bp is gone due to xfs_da_shrink_inode */
> - if (error)
> - goto out;
> - error = xfs_defer_finish(&args->trans);
> - if (error)
> - goto out;
> - } else
> - xfs_trans_brelse(args->trans, bp);
> - }
> error = 0;
> -
> out:
> if (state)
> xfs_da_state_free(state);
>
--
chandan
