On Mon, Feb 24, 2020 at 09:30:41AM +0530, Chandan Rajendra wrote:
> The intermediate numbers calculated by xfs_attr_calc_size() will be needed by
> a future commit to correctly calculate log reservation for xattr set
> operation. Towards this goal, this commit introduces 'struct
> xfs_attr_set_resv' to collect,
> 1. Number of dabtree blocks.
> 2. Number of remote blocks.
> 3. Number of Bmbt blocks.
> 4. Total number of blocks we need to reserve.
>
> This will be returned as an out argument by xfs_attr_calc_size().
>
> Signed-off-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 50 ++++++++++++++++++++++------------------
> fs/xfs/libxfs/xfs_attr.h | 13 +++++++++++
> 2 files changed, 40 insertions(+), 23 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index a708b142f69b6..921acac71e5d9 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -136,16 +136,14 @@ xfs_attr_get(
> /*
> * Calculate how many blocks we need for the new attribute,
> */
> -STATIC int
> +STATIC void
> xfs_attr_calc_size(
> - struct xfs_da_args *args,
> - int *local)
> + struct xfs_da_args *args,
> + struct xfs_attr_set_resv *resv,
> + int *local)
> {
> - struct xfs_mount *mp = args->dp->i_mount;
> - unsigned int total_dablks;
> - unsigned int bmbt_blks;
> - unsigned int rmt_blks;
> - int size;
> + struct xfs_mount *mp = args->dp->i_mount;
> + int size;
>
> /*
> * Determine space new attribute will use, and if it would be
> @@ -153,25 +151,27 @@ xfs_attr_calc_size(
> */
> size = xfs_attr_leaf_newentsize(args->geo, args->namelen,
> args->valuelen, local);
> - total_dablks = XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK);
> + resv->total_dablks = XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK);
> if (*local) {
> if (size > (args->geo->blksize / 2)) {
> /* Double split possible */
> - total_dablks *= 2;
> + resv->total_dablks *= 2;
> }
> - rmt_blks = 0;
> + resv->rmt_blks = 0;
> } else {
> /*
> * Out of line attribute, cannot double split, but
> * make room for the attribute value itself.
> */
> - rmt_blks = xfs_attr3_rmt_blocks(mp, args->valuelen);
> + resv->rmt_blks = xfs_attr3_rmt_blocks(mp, args->valuelen);
> }
>
> - bmbt_blks = XFS_NEXTENTADD_SPACE_RES(mp, total_dablks + rmt_blks,
> - XFS_ATTR_FORK);
> + resv->bmbt_blks = XFS_NEXTENTADD_SPACE_RES(mp,
> + resv->total_dablks + resv->rmt_blks,
> + XFS_ATTR_FORK);
>
> - return total_dablks + rmt_blks + bmbt_blks;
> + resv->alloc_blks = resv->total_dablks + resv->rmt_blks +
> + resv->bmbt_blks;
Do we really need a field to track the total of three other fields in
the same structure? I'd rather just let the caller add them up for
args.total if that's the only usage.
Brian
> }
>
> STATIC int
> @@ -295,14 +295,17 @@ xfs_attr_remove_args(
> */
> int
> xfs_attr_set(
> - struct xfs_da_args *args)
> + struct xfs_da_args *args)
> {
> - struct xfs_inode *dp = args->dp;
> - struct xfs_mount *mp = dp->i_mount;
> - struct xfs_trans_res tres;
> - bool rsvd = (args->attr_namespace & XFS_ATTR_ROOT);
> - int error, local;
> - unsigned int total;
> + struct xfs_inode *dp = args->dp;
> + struct xfs_mount *mp = dp->i_mount;
> + struct xfs_attr_set_resv resv;
> + struct xfs_trans_res tres;
> + bool rsvd;
> + int error, local;
> + unsigned int total;
> +
> + rsvd = (args->attr_namespace & XFS_ATTR_ROOT);
>
> if (XFS_FORCED_SHUTDOWN(dp->i_mount))
> return -EIO;
> @@ -326,7 +329,8 @@ xfs_attr_set(
> XFS_STATS_INC(mp, xs_attr_set);
>
> args->op_flags |= XFS_DA_OP_ADDNAME;
> - args->total = xfs_attr_calc_size(args, &local);
> + xfs_attr_calc_size(args, &resv, &local);
> + args->total = resv.alloc_blks;
>
> /*
> * If the inode doesn't have an attribute fork, add one.
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 861c81f9bb918..dc08bdfbc9615 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -73,6 +73,19 @@ struct xfs_attr_list_context {
> int index; /* index into output buffer */
> };
>
> +struct xfs_attr_set_resv {
> + /* Number of unlogged blocks needed to store the remote attr value. */
> + unsigned int rmt_blks;
> +
> + /* Number of filesystem blocks to allocate for the da btree. */
> + unsigned int total_dablks;
> +
> + /* Blocks we might need to create all the new attr fork mappings. */
> + unsigned int bmbt_blks;
> +
> + /* Total number of blocks we might have to allocate. */
> + unsigned int alloc_blks;
> +};
>
> /*========================================================================
> * Function prototypes for the kernel.
> --
> 2.19.1
>
