Re: [PATCH v7 12/19] xfs: Add helper function xfs_attr_rmtval_unmap

Dave Chinner
2020-02-25T07:21:09Z

On Sat, Feb 22, 2020 at 07:06:04PM -0700, Allison Collins wrote:
> This function is similar to xfs_attr_rmtval_remove, but adapted to return EAGAIN for
> new transactions. We will use this later when we introduce delayed attributes

68-72 columns...

>
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr_remote.c | 28 ++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_attr_remote.h | 1 +
> 2 files changed, 29 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index 3de2eec..da40f85 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -711,3 +711,31 @@ xfs_attr_rmtval_remove(
> }
> return 0;
> }
> +
> +/*
> + * Remove the value associated with an attribute by deleting the out-of-line
> + * buffer that it is stored on. Returns EAGAIN for the caller to refresh the
> + * transaction and recall the function
> + */
> +int
> +xfs_attr_rmtval_unmap(
> + struct xfs_da_args *args)
> +{
> + int error, done;

Best to initialise done to zero here.

> +
> + /*
> + * Unmap value blocks for this attr. This is similar to
> + * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
> + * for new transactions
> + */
> + error = xfs_bunmapi(args->trans, args->dp,
> + args->rmtblkno, args->rmtblkcnt,
> + XFS_BMAPI_ATTRFORK, 1, &done);

Got 80 columns for code, please use them all :)

> + if (error)
> + return error;
> +
> + if (!done)
> + return -EAGAIN;

Hmmm, I guess I'm missing the context at this point: why not just pass the done
variable all the way back to the caller that will be looping on this function
call?

That turns this function into:

int
xfs_attr_rmtval_unmap(
struct xfs_da_args *args,
bool *done)
{
return xfs_bunmapi(args->trans, args->dp, args->rmtblkno,
args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, done);
}

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com