Re: [PATCH 8/8] xfs: do not allocate the entire delalloc extent in xfs_bmapi_write

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Apr 08, 2024 at 04:54:54PM +0200, Christoph Hellwig wrote:
> While trying to convert the entire delalloc extent is a good decision
> for regular writeback as it leads to larger contigous on-disk extents,
> but for other callers of xfs_bmapi_write is is rather questionable as
> it forced them to loop creating new transactions just in case there
> is no large enough contiguous extent to cover the whole delalloc
> reservation.
> 
> Change xfs_bmapi_write to only allocate the passed in range instead.

Looking at this... I guess xfs_map_blocks -> xfs_convert_blocks ->
xfs_bmapi_convert_delalloc -> xfs_bmapi_allocate is now how writeback
converts delalloc extents before scheduling writeout.  This is how the
mass-conversions of large da reservations got done before this series,
and that's still how it works, right?

Whereas xfs_bmapi_write is for targeted conversions only?

> Signed-off-by: Christoph Hellwig <hch@lst.de>

If yes and yes, then:
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 7700a48e013d5a..748809b13113ab 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4533,8 +4533,9 @@ 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;
> +				bma.length = XFS_FILBLKS_MIN(bma.length,
> +					bma.got.br_blockcount -
> +					(bno - bma.got.br_startoff));
>  			} else {
>  				if (!eof)
>  					bma.length = XFS_FILBLKS_MIN(bma.length,
> -- 
> 2.39.2
> 
>