On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
<allison.henderson@xxxxxxxxxx> 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 | 66 +++++++++++++++++++++++++++++-------------------
> 1 file changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 4b788f2..30a16fe 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1366,6 +1366,43 @@ 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;
> +
> + args->dac.flags |= XFS_DAC_FINISH_TRANS;
Why did xfs_defer_finish(&args->trans); turn into the above?
Are you testing reviewers alertness? ;-)
Please keep logic preserving patches separate from logic change patches.
Thanks,
Amir.
> + } 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
> @@ -1383,8 +1420,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);
> @@ -1493,30 +1529,8 @@ 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 ((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;
> + if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
> + error = xfs_attr_node_shrink(args, state);
>
> out:
> if (state)
> --
> 2.7.4
>
