On Thu, Dec 14, 2023 at 07:34:36AM +0100, Christoph Hellwig wrote:
> xfs_bmap_rtalloc is a bit of a mess in terms of calculating the locally
> need variables. Reorder them a bit so that related code is located
> next to each other - the raminlen calculation moves up next to where
> the maximum len is calculated, and all the prod calculation is move
> into a single place and rearranged so that the real prod calculation
> only happens when it actually is needed.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
This looks like a simple enough change,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_rtalloc.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 24d74c8b532e5f..595740d18dc4c3 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1369,7 +1369,6 @@ xfs_bmap_rtalloc(
>
> align = xfs_get_extsz_hint(ap->ip);
> retry:
> - prod = xfs_extlen_to_rtxlen(mp, align);
> error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
> align, 1, ap->eof, 0,
> ap->conv, &ap->offset, &ap->length);
> @@ -1387,13 +1386,6 @@ xfs_bmap_rtalloc(
> if (ap->offset != orig_offset)
> minlen += orig_offset - ap->offset;
>
> - /*
> - * If the offset & length are not perfectly aligned
> - * then kill prod, it will just get us in trouble.
> - */
> - div_u64_rem(ap->offset, align, &mod);
> - if (mod || ap->length % align)
> - prod = 1;
> /*
> * Set ralen to be the actual requested length in rtextents.
> *
> @@ -1404,6 +1396,7 @@ xfs_bmap_rtalloc(
> * adjust the starting point to match it.
> */
> ralen = xfs_extlen_to_rtxlen(mp, min(ap->length, XFS_MAX_BMBT_EXTLEN));
> + raminlen = max_t(xfs_rtxlen_t, 1, xfs_extlen_to_rtxlen(mp, minlen));
>
> /*
> * Lock out modifications to both the RT bitmap and summary inodes
> @@ -1432,7 +1425,16 @@ xfs_bmap_rtalloc(
> start = 0;
> }
>
> - raminlen = max_t(xfs_rtxlen_t, 1, xfs_extlen_to_rtxlen(mp, minlen));
> + /*
> + * Only bother calculating a real prod factor if offset & length are
> + * perfectly aligned, otherwise it will just get us in trouble.
> + */
> + div_u64_rem(ap->offset, align, &mod);
> + if (mod || ap->length % align)
> + prod = 1;
> + else
> + prod = xfs_extlen_to_rtxlen(mp, align);
> +
> error = xfs_rtallocate_extent(ap->tp, start, raminlen, ralen, &ralen,
> ap->wasdel, prod, &rtx);
> if (error == -ENOSPC) {
> --
> 2.39.2
>
>
