Re: [PATCH V7 12/14] xfs: Compute bmap extent alignments in a separate function

Allison Henderson
2020-10-24T22:27:17Z

On 10/18/20 11:40 PM, Chandan Babu R wrote:
> This commit moves over the code which computes stripe alignment and
> extent size hint alignment into a separate function. Apart from
> xfs_bmap_btalloc(), the new function will be used by another function
> introduced in a future commit.
>
Ok, the hoist looks equivelent
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 88 +++++++++++++++++++++++-----------------
> 1 file changed, 51 insertions(+), 37 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 64c4d0e384a5..935f2d506748 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3463,13 +3463,58 @@ xfs_bmap_btalloc_accounting(
> args->len);
> }
>
> +static void
> +xfs_bmap_compute_alignments(
> + struct xfs_bmalloca *ap,
> + struct xfs_alloc_arg *args,
> + int *stripe_align)
> +{
> + struct xfs_mount *mp = args->mp;
> + xfs_extlen_t align = 0; /* minimum allocation alignment */
> + int error;
> +
> + /* stripe alignment for allocation is determined by mount parameters */
> + *stripe_align = 0;
> + if (mp->m_swidth && (mp->m_flags & XFS_MOUNT_SWALLOC))
> + *stripe_align = mp->m_swidth;
> + else if (mp->m_dalign)
> + *stripe_align = mp->m_dalign;
> +
> + if (ap->flags & XFS_BMAPI_COWFORK)
> + align = xfs_get_cowextsz_hint(ap->ip);
> + else if (ap->datatype & XFS_ALLOC_USERDATA)
> + align = xfs_get_extsz_hint(ap->ip);
> + if (align) {
> + error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
> + align, 0, ap->eof, 0, ap->conv,
> + &ap->offset, &ap->length);
> + ASSERT(!error);
> + ASSERT(ap->length);
> + }
> +
> + /* apply extent size hints if obtained earlier */
> + if (align) {
> + args->prod = align;
> + div_u64_rem(ap->offset, args->prod, &args->mod);
> + if (args->mod)
> + args->mod = args->prod - args->mod;
> + } else if (mp->m_sb.sb_blocksize >= PAGE_SIZE) {
> + args->prod = 1;
> + args->mod = 0;
> + } else {
> + args->prod = PAGE_SIZE >> mp->m_sb.sb_blocklog;
> + div_u64_rem(ap->offset, args->prod, &args->mod);
> + if (args->mod)
> + args->mod = args->prod - args->mod;
> + }
> +}
> +
> STATIC int
> xfs_bmap_btalloc(
> struct xfs_bmalloca *ap) /* bmap alloc argument struct */
> {
> xfs_mount_t *mp; /* mount point structure */
> xfs_alloctype_t atype = 0; /* type for allocation routines */
> - xfs_extlen_t align = 0; /* minimum allocation alignment */
> xfs_agnumber_t fb_agno; /* ag number of ap->firstblock */
> xfs_agnumber_t ag;
> xfs_alloc_arg_t args;
> @@ -3489,25 +3534,11 @@ xfs_bmap_btalloc(
>
> mp = ap->ip->i_mount;
>
> - /* stripe alignment for allocation is determined by mount parameters */
> - stripe_align = 0;
> - if (mp->m_swidth && (mp->m_flags & XFS_MOUNT_SWALLOC))
> - stripe_align = mp->m_swidth;
> - else if (mp->m_dalign)
> - stripe_align = mp->m_dalign;
> -
> - if (ap->flags & XFS_BMAPI_COWFORK)
> - align = xfs_get_cowextsz_hint(ap->ip);
> - else if (ap->datatype & XFS_ALLOC_USERDATA)
> - align = xfs_get_extsz_hint(ap->ip);
> - if (align) {
> - error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
> - align, 0, ap->eof, 0, ap->conv,
> - &ap->offset, &ap->length);
> - ASSERT(!error);
> - ASSERT(ap->length);
> - }
> + memset(&args, 0, sizeof(args));
> + args.tp = ap->tp;
> + args.mp = mp;
>
> + xfs_bmap_compute_alignments(ap, &args, &stripe_align);
>
> nullfb = ap->tp->t_firstblock == NULLFSBLOCK;
> fb_agno = nullfb ? NULLAGNUMBER : XFS_FSB_TO_AGNO(mp,
> @@ -3538,9 +3569,6 @@ xfs_bmap_btalloc(
> * Normal allocation, done through xfs_alloc_vextent.
> */
> tryagain = isaligned = 0;
> - memset(&args, 0, sizeof(args));
> - args.tp = ap->tp;
> - args.mp = mp;
> args.fsbno = ap->blkno;
> args.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
>
> @@ -3571,21 +3599,7 @@ xfs_bmap_btalloc(
> args.total = ap->total;
> args.minlen = ap->minlen;
> }
> - /* apply extent size hints if obtained earlier */
> - if (align) {
> - args.prod = align;
> - div_u64_rem(ap->offset, args.prod, &args.mod);
> - if (args.mod)
> - args.mod = args.prod - args.mod;
> - } else if (mp->m_sb.sb_blocksize >= PAGE_SIZE) {
> - args.prod = 1;
> - args.mod = 0;
> - } else {
> - args.prod = PAGE_SIZE >> mp->m_sb.sb_blocklog;
> - div_u64_rem(ap->offset, args.prod, &args.mod);
> - if (args.mod)
> - args.mod = args.prod - args.mod;
> - }
> +
> /*
> * If we are not low on available data blocks, and the underlying
> * logical volume manager is a stripe, and the file offset is zero then
>