Re: [PATCH 4/4] xfs: support sub-block aligned vectors in always COW mode

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

On Thu, Oct 23, 2025 at 03:55:45PM +0200, Christoph Hellwig wrote:
> Now that the block layer and iomap have grown support to indicate
> the bio sector size explicitly instead of assuming the device sector
> size, we can ask for logical block size alignment and thus support
> direct I/O writes where the overall size is logical block size
> aligned, but the boundaries between vectors might not be.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_file.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 2702fef2c90c..f2ac4115c18b 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -674,8 +674,17 @@ xfs_file_dio_write_aligned(
>  	struct xfs_zone_alloc_ctx *ac)
>  {
>  	unsigned int		iolock = XFS_IOLOCK_SHARED;
> +	unsigned int		dio_flags = 0;
>  	ssize_t			ret;
>  
> +	/*
> +	 * For always COW inodes, each bio must be aligned to the file system
> +	 * block size and not just the device sector size because we need to
> +	 * allocate a block-aligned amount of space for each write.
> +	 */
> +	if (xfs_is_always_cow_inode(ip))
> +		dio_flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
> +
>  	ret = xfs_ilock_iocb_for_write(iocb, &iolock);
>  	if (ret)
>  		return ret;
> @@ -693,7 +702,7 @@ xfs_file_dio_write_aligned(
>  		iolock = XFS_IOLOCK_SHARED;
>  	}
>  	trace_xfs_file_direct_write(iocb, from);
> -	ret = iomap_dio_rw(iocb, from, ops, dops, 0, ac, 0);
> +	ret = iomap_dio_rw(iocb, from, ops, dops, dio_flags, ac, 0);
>  out_unlock:
>  	xfs_iunlock(ip, iolock);
>  	return ret;
> @@ -890,15 +899,7 @@ xfs_file_dio_write(
>  	if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
>  		return -EINVAL;
>  
> -	/*
> -	 * For always COW inodes we also must check the alignment of each
> -	 * individual iovec segment, as they could end up with different
> -	 * I/Os due to the way bio_iov_iter_get_pages works, and we'd
> -	 * then overwrite an already written block.
> -	 */
> -	if (((iocb->ki_pos | count) & ip->i_mount->m_blockmask) ||
> -	    (xfs_is_always_cow_inode(ip) &&
> -	     (iov_iter_alignment(from) & ip->i_mount->m_blockmask)))
> +	if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
>  		return xfs_file_dio_write_unaligned(ip, iocb, from);
>  	if (xfs_is_zoned_inode(ip))
>  		return xfs_file_dio_write_zoned(ip, iocb, from);
> -- 
> 2.47.3
> 
>