On Mon, Apr 08, 2024 at 04:54:51PM +0200, Christoph Hellwig wrote:
> xfs_bmapi_allocate currently overwrites offset and len when converting
> delayed allocations, and duplicates the length cap done for non-delalloc
> allocations. Move all that logic into the callers to avoid duplication
> and to make the calling conventions more obvious.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 32 ++++++++++++++++++--------------
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index f2e934c2fb423c..aa182937de4641 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4194,21 +4194,11 @@ xfs_bmapi_allocate(
> int error;
>
> ASSERT(bma->length > 0);
> + ASSERT(bma->length <= XFS_MAX_BMBT_EXTLEN);
>
> - /*
> - * For the wasdelay case, we could also just allocate the stuff asked
> - * for in this bmap call but that wouldn't be as good.
> - */
> if (bma->wasdel) {
> - bma->length = (xfs_extlen_t)bma->got.br_blockcount;
> - bma->offset = bma->got.br_startoff;
> if (!xfs_iext_peek_prev_extent(ifp, &bma->icur, &bma->prev))
> bma->prev.br_startoff = NULLFILEOFF;
> - } else {
> - bma->length = XFS_FILBLKS_MIN(bma->length, XFS_MAX_BMBT_EXTLEN);
> - if (!bma->eof)
> - bma->length = XFS_FILBLKS_MIN(bma->length,
> - bma->got.br_startoff - bma->offset);
> }
>
> if (bma->flags & XFS_BMAPI_CONTIG)
> @@ -4542,6 +4532,15 @@ xfs_bmapi_write(
> */
> bma.length = XFS_FILBLKS_MIN(len, XFS_MAX_BMBT_EXTLEN);
>
> + if (wasdelay) {
> + bma.offset = bma.got.br_startoff;
> + bma.length = bma.got.br_blockcount;
This read funny since we'd previously set bma.{offset,length} above, but
I guess that preserves the "convert all the delalloc" behavior; and you
turn it off in patch 8, right?
--D
> + } else {
> + if (!eof)
> + bma.length = XFS_FILBLKS_MIN(bma.length,
> + bma.got.br_startoff - bno);
> + }
> +
> ASSERT(bma.length > 0);
> error = xfs_bmapi_allocate(&bma);
> if (error) {
> @@ -4694,11 +4693,16 @@ xfs_bmapi_convert_delalloc(
> bma.tp = tp;
> bma.ip = ip;
> bma.wasdel = true;
> - bma.offset = bma.got.br_startoff;
> - bma.length = max_t(xfs_filblks_t, bma.got.br_blockcount,
> - XFS_MAX_BMBT_EXTLEN);
> bma.minleft = xfs_bmapi_minleft(tp, ip, whichfork);
>
> + /*
> + * Always allocate convert from the start of the delalloc extent even if
> + * that is outside the passed in range to create large contiguous
> + * extents on disk.
> + */
> + bma.offset = bma.got.br_startoff;
> + bma.length = bma.got.br_blockcount;
> +
> /*
> * When we're converting the delalloc reservations backing dirty pages
> * in the page cache, we must be careful about how we create the new
> --
> 2.39.2
>
>
