alloc misaligned vectors for zoned XFS

alloc misaligned vectors for zoned XFS

[Christoph Hellwig]

Hi all,

this series enables the new block layer support for misaligned
individual vectors for zoned XFS.

The first patch is the from Qu and supposedly already applied to
the vfs iomap 6.19 branch, but I can't find it there.  The next
two are small fixups for it, and the last one makes use of this
new functionality in XFS.

Diffstat:
 fs/iomap/direct-io.c  |   17 +++++++++++++++--
 fs/xfs/xfs_file.c     |   21 +++++++++++----------
 include/linux/iomap.h |    8 ++++++++
 3 files changed, 34 insertions(+), 12 deletions(-)

[PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Christoph Hellwig]

From: Qu Wenruo <wqu@suse.com>

Btrfs requires all of its bios to be fs block aligned, normally it's
totally fine but with the incoming block size larger than page size
(bs > ps) support, the requirement is no longer met for direct IOs.

Because iomap_dio_bio_iter() calls bio_iov_iter_get_pages(), only
requiring alignment to be bdev_logical_block_size().

In the real world that value is either 512 or 4K, on 4K page sized
systems it means bio_iov_iter_get_pages() can break the bio at any page
boundary, breaking btrfs' requirement for bs > ps cases.

To address this problem, introduce a new public iomap dio flag,
IOMAP_DIO_FSBLOCK_ALIGNED.

When calling __iomap_dio_rw() with that new flag, iomap_dio::flags will
inherit that new flag, and iomap_dio_bio_iter() will take fs block size
into the calculation of the alignment, and pass the alignment to
bio_iov_iter_get_pages(), respecting the fs block size requirement.

The initial user of this flag will be btrfs, which needs to calculate the
checksum for direct read and thus requires the biovec to be fs block
aligned for the incoming bs > ps support.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c  | 13 ++++++++++++-
 include/linux/iomap.h |  8 ++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 5d5d63efbd57..ce9cbd2bace0 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -336,10 +336,18 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	int nr_pages, ret = 0;
 	u64 copied = 0;
 	size_t orig_count;
+	unsigned int alignment = bdev_logical_block_size(iomap->bdev);
 
 	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
 		return -EINVAL;
 
+	/*
+	 * Align to the larger one of bdev and fs block size, to meet the
+	 * alignment requirement of both layers.
+	 */
+	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
+		alignment = max(alignment, fs_block_size);
+
 	if (dio->flags & IOMAP_DIO_WRITE) {
 		bio_opf |= REQ_OP_WRITE;
 
@@ -434,7 +442,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 		bio->bi_end_io = iomap_dio_bio_end_io;
 
 		ret = bio_iov_iter_get_pages(bio, dio->submit.iter,
-				bdev_logical_block_size(iomap->bdev) - 1);
+					     alignment - 1);
 		if (unlikely(ret)) {
 			/*
 			 * We have to stop part way through an IO. We must fall
@@ -639,6 +647,9 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 	if (iocb->ki_flags & IOCB_NOWAIT)
 		iomi.flags |= IOMAP_NOWAIT;
 
+	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
+		dio->flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
+
 	if (iov_iter_rw(iter) == READ) {
 		/* reads can always complete inline */
 		dio->flags |= IOMAP_DIO_INLINE_COMP;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 73dceabc21c8..4da13fe24ce8 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -518,6 +518,14 @@ struct iomap_dio_ops {
  */
 #define IOMAP_DIO_PARTIAL		(1 << 2)
 
+/*
+ * Ensure each bio is aligned to fs block size.
+ *
+ * For filesystems which need to calculate/verify the checksum of each fs
+ * block. Otherwise they may not be able to handle unaligned bios.
+ */
+#define IOMAP_DIO_FSBLOCK_ALIGNED	(1 << 3)
+
 ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
 		unsigned int dio_flags, void *private, size_t done_before);
-- 
2.47.3

[PATCH 2/4] FIXUP: iomap: aligning to larger than fs block size doesn't make sense

[Christoph Hellwig]

All of the VFS and helpers assume that the file system block size must
be larger or equal to the device block size.  So doing a max of both
doesn't really make much sense.  Siplify the code in iomap_dio_bio_iter
to do a simple if/else.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index ce9cbd2bace0..8d094d6f5f3e 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -336,17 +336,19 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	int nr_pages, ret = 0;
 	u64 copied = 0;
 	size_t orig_count;
-	unsigned int alignment = bdev_logical_block_size(iomap->bdev);
+	unsigned int alignment;
 
 	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
 		return -EINVAL;
 
 	/*
-	 * Align to the larger one of bdev and fs block size, to meet the
-	 * alignment requirement of both layers.
+	 * File systems that write out of place and always allocate new blocks
+	 * need each bio to be block aligned as that's the unit of allocation.
 	 */
 	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
-		alignment = max(alignment, fs_block_size);
+		alignment = fs_block_size;
+	else
+		alignment = bdev_logical_block_size(iomap->bdev);
 
 	if (dio->flags & IOMAP_DIO_WRITE) {
 		bio_opf |= REQ_OP_WRITE;
-- 
2.47.3

[PATCH 3/4] FIXUP: iomap: also apply IOMAP_DIO_FSBLOCK_ALIGNED to the iomap range

[Christoph Hellwig]

Not just the bios created by bio_iov_iter_get_pages need to be aligned
to the file system block size when IOMAP_DIO_FSBLOCK_ALIGNED is used,
but also the iomap mapped region.  Use the local alignment variable
for this check as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 8d094d6f5f3e..13def8418659 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -338,9 +338,6 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	size_t orig_count;
 	unsigned int alignment;
 
-	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
-		return -EINVAL;
-
 	/*
 	 * File systems that write out of place and always allocate new blocks
 	 * need each bio to be block aligned as that's the unit of allocation.
@@ -350,6 +347,9 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	else
 		alignment = bdev_logical_block_size(iomap->bdev);
 
+	if ((pos | length) & (alignment - 1))
+		return -EINVAL;
+
 	if (dio->flags & IOMAP_DIO_WRITE) {
 		bio_opf |= REQ_OP_WRITE;
 
-- 
2.47.3

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

[Christoph Hellwig]

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>
---
 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

Re: [PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Darrick J. Wong]

On Thu, Oct 23, 2025 at 03:55:42PM +0200, Christoph Hellwig wrote:
> From: Qu Wenruo <wqu@suse.com>
> 
> Btrfs requires all of its bios to be fs block aligned, normally it's
> totally fine but with the incoming block size larger than page size
> (bs > ps) support, the requirement is no longer met for direct IOs.
> 
> Because iomap_dio_bio_iter() calls bio_iov_iter_get_pages(), only
> requiring alignment to be bdev_logical_block_size().
> 
> In the real world that value is either 512 or 4K, on 4K page sized
> systems it means bio_iov_iter_get_pages() can break the bio at any page
> boundary, breaking btrfs' requirement for bs > ps cases.
> 
> To address this problem, introduce a new public iomap dio flag,
> IOMAP_DIO_FSBLOCK_ALIGNED.
> 
> When calling __iomap_dio_rw() with that new flag, iomap_dio::flags will
> inherit that new flag, and iomap_dio_bio_iter() will take fs block size
> into the calculation of the alignment, and pass the alignment to
> bio_iov_iter_get_pages(), respecting the fs block size requirement.
> 
> The initial user of this flag will be btrfs, which needs to calculate the
> checksum for direct read and thus requires the biovec to be fs block
> aligned for the incoming bs > ps support.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/direct-io.c  | 13 ++++++++++++-
>  include/linux/iomap.h |  8 ++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 5d5d63efbd57..ce9cbd2bace0 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -336,10 +336,18 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  	int nr_pages, ret = 0;
>  	u64 copied = 0;
>  	size_t orig_count;
> +	unsigned int alignment = bdev_logical_block_size(iomap->bdev);
>  
>  	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
>  		return -EINVAL;
>  
> +	/*
> +	 * Align to the larger one of bdev and fs block size, to meet the
> +	 * alignment requirement of both layers.
> +	 */
> +	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
> +		alignment = max(alignment, fs_block_size);
> +
>  	if (dio->flags & IOMAP_DIO_WRITE) {
>  		bio_opf |= REQ_OP_WRITE;
>  
> @@ -434,7 +442,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  		bio->bi_end_io = iomap_dio_bio_end_io;
>  
>  		ret = bio_iov_iter_get_pages(bio, dio->submit.iter,
> -				bdev_logical_block_size(iomap->bdev) - 1);
> +					     alignment - 1);
>  		if (unlikely(ret)) {
>  			/*
>  			 * We have to stop part way through an IO. We must fall
> @@ -639,6 +647,9 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  	if (iocb->ki_flags & IOCB_NOWAIT)
>  		iomi.flags |= IOMAP_NOWAIT;
>  
> +	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
> +		dio->flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
> +
>  	if (iov_iter_rw(iter) == READ) {
>  		/* reads can always complete inline */
>  		dio->flags |= IOMAP_DIO_INLINE_COMP;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 73dceabc21c8..4da13fe24ce8 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -518,6 +518,14 @@ struct iomap_dio_ops {
>   */
>  #define IOMAP_DIO_PARTIAL		(1 << 2)
>  
> +/*
> + * Ensure each bio is aligned to fs block size.
> + *
> + * For filesystems which need to calculate/verify the checksum of each fs
> + * block. Otherwise they may not be able to handle unaligned bios.
> + */
> +#define IOMAP_DIO_FSBLOCK_ALIGNED	(1 << 3)

A new flag requires an update to IOMAP_F_FLAGS_STRINGS in trace.h for
tracepoint decoding.

The rest of the changes look ok to me, modulo hch's subsequent fixups.

--D

>  ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
>  		unsigned int dio_flags, void *private, size_t done_before);
> -- 
> 2.47.3
> 
> 

Re: [PATCH 2/4] FIXUP: iomap: aligning to larger than fs block size doesn't make sense

[Darrick J. Wong]

On Thu, Oct 23, 2025 at 03:55:43PM +0200, Christoph Hellwig wrote:
> All of the VFS and helpers assume that the file system block size must
> be larger or equal to the device block size.  So doing a max of both
> doesn't really make much sense.  Siplify the code in iomap_dio_bio_iter

                                   Simplify

> to do a simple if/else.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

With that fixed,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index ce9cbd2bace0..8d094d6f5f3e 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -336,17 +336,19 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  	int nr_pages, ret = 0;
>  	u64 copied = 0;
>  	size_t orig_count;
> -	unsigned int alignment = bdev_logical_block_size(iomap->bdev);
> +	unsigned int alignment;
>  
>  	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
>  		return -EINVAL;
>  
>  	/*
> -	 * Align to the larger one of bdev and fs block size, to meet the
> -	 * alignment requirement of both layers.
> +	 * File systems that write out of place and always allocate new blocks
> +	 * need each bio to be block aligned as that's the unit of allocation.
>  	 */
>  	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
> -		alignment = max(alignment, fs_block_size);
> +		alignment = fs_block_size;
> +	else
> +		alignment = bdev_logical_block_size(iomap->bdev);
>  
>  	if (dio->flags & IOMAP_DIO_WRITE) {
>  		bio_opf |= REQ_OP_WRITE;
> -- 
> 2.47.3
> 
> 

Re: [PATCH 3/4] FIXUP: iomap: also apply IOMAP_DIO_FSBLOCK_ALIGNED to the iomap range

[Darrick J. Wong]

On Thu, Oct 23, 2025 at 03:55:44PM +0200, Christoph Hellwig wrote:
> Not just the bios created by bio_iov_iter_get_pages need to be aligned
> to the file system block size when IOMAP_DIO_FSBLOCK_ALIGNED is used,
> but also the iomap mapped region.  Use the local alignment variable
> for this check as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Heh, silly diff for eliminating context to save a few lines :P
This makes sense to me.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 8d094d6f5f3e..13def8418659 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -338,9 +338,6 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  	size_t orig_count;
>  	unsigned int alignment;
>  
> -	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
> -		return -EINVAL;
> -
>  	/*
>  	 * File systems that write out of place and always allocate new blocks
>  	 * need each bio to be block aligned as that's the unit of allocation.
> @@ -350,6 +347,9 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  	else
>  		alignment = bdev_logical_block_size(iomap->bdev);
>  
> +	if ((pos | length) & (alignment - 1))
> +		return -EINVAL;
> +
>  	if (dio->flags & IOMAP_DIO_WRITE) {
>  		bio_opf |= REQ_OP_WRITE;
>  
> -- 
> 2.47.3
> 
> 

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

[Darrick J. Wong]

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
> 
> 

Re: [PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Qu Wenruo]

在 2025/10/28 02:40, Darrick J. Wong 写道:
> On Thu, Oct 23, 2025 at 03:55:42PM +0200, Christoph Hellwig wrote:
>> From: Qu Wenruo <wqu@suse.com>
>>
>> Btrfs requires all of its bios to be fs block aligned, normally it's
>> totally fine but with the incoming block size larger than page size
>> (bs > ps) support, the requirement is no longer met for direct IOs.
>>
>> Because iomap_dio_bio_iter() calls bio_iov_iter_get_pages(), only
>> requiring alignment to be bdev_logical_block_size().
>>
>> In the real world that value is either 512 or 4K, on 4K page sized
>> systems it means bio_iov_iter_get_pages() can break the bio at any page
>> boundary, breaking btrfs' requirement for bs > ps cases.
>>
>> To address this problem, introduce a new public iomap dio flag,
>> IOMAP_DIO_FSBLOCK_ALIGNED.
>>
>> When calling __iomap_dio_rw() with that new flag, iomap_dio::flags will
>> inherit that new flag, and iomap_dio_bio_iter() will take fs block size
>> into the calculation of the alignment, and pass the alignment to
>> bio_iov_iter_get_pages(), respecting the fs block size requirement.
>>
>> The initial user of this flag will be btrfs, which needs to calculate the
>> checksum for direct read and thus requires the biovec to be fs block
>> aligned for the incoming bs > ps support.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>   fs/iomap/direct-io.c  | 13 ++++++++++++-
>>   include/linux/iomap.h |  8 ++++++++
>>   2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
>> index 5d5d63efbd57..ce9cbd2bace0 100644
>> --- a/fs/iomap/direct-io.c
>> +++ b/fs/iomap/direct-io.c
>> @@ -336,10 +336,18 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>>   	int nr_pages, ret = 0;
>>   	u64 copied = 0;
>>   	size_t orig_count;
>> +	unsigned int alignment = bdev_logical_block_size(iomap->bdev);
>>   
>>   	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
>>   		return -EINVAL;
>>   
>> +	/*
>> +	 * Align to the larger one of bdev and fs block size, to meet the
>> +	 * alignment requirement of both layers.
>> +	 */
>> +	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
>> +		alignment = max(alignment, fs_block_size);
>> +
>>   	if (dio->flags & IOMAP_DIO_WRITE) {
>>   		bio_opf |= REQ_OP_WRITE;
>>   
>> @@ -434,7 +442,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>>   		bio->bi_end_io = iomap_dio_bio_end_io;
>>   
>>   		ret = bio_iov_iter_get_pages(bio, dio->submit.iter,
>> -				bdev_logical_block_size(iomap->bdev) - 1);
>> +					     alignment - 1);
>>   		if (unlikely(ret)) {
>>   			/*
>>   			 * We have to stop part way through an IO. We must fall
>> @@ -639,6 +647,9 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>>   	if (iocb->ki_flags & IOCB_NOWAIT)
>>   		iomi.flags |= IOMAP_NOWAIT;
>>   
>> +	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
>> +		dio->flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
>> +
>>   	if (iov_iter_rw(iter) == READ) {
>>   		/* reads can always complete inline */
>>   		dio->flags |= IOMAP_DIO_INLINE_COMP;
>> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
>> index 73dceabc21c8..4da13fe24ce8 100644
>> --- a/include/linux/iomap.h
>> +++ b/include/linux/iomap.h
>> @@ -518,6 +518,14 @@ struct iomap_dio_ops {
>>    */
>>   #define IOMAP_DIO_PARTIAL		(1 << 2)
>>   
>> +/*
>> + * Ensure each bio is aligned to fs block size.
>> + *
>> + * For filesystems which need to calculate/verify the checksum of each fs
>> + * block. Otherwise they may not be able to handle unaligned bios.
>> + */
>> +#define IOMAP_DIO_FSBLOCK_ALIGNED	(1 << 3)
> 
> A new flag requires an update to IOMAP_F_FLAGS_STRINGS in trace.h for
> tracepoint decoding.

I'm wondering who should fix this part.

The original author (myself) or Christoph?

Thanks,
Qu

> 
> The rest of the changes look ok to me, modulo hch's subsequent fixups.
> 
> --D
> 
>>   ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>>   		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
>>   		unsigned int dio_flags, void *private, size_t done_before);
>> -- 
>> 2.47.3
>>
>>

Re: [PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Darrick J. Wong]

On Tue, Oct 28, 2025 at 08:11:21AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2025/10/28 02:40, Darrick J. Wong 写道:
> > On Thu, Oct 23, 2025 at 03:55:42PM +0200, Christoph Hellwig wrote:
> > > From: Qu Wenruo <wqu@suse.com>
> > > 
> > > Btrfs requires all of its bios to be fs block aligned, normally it's
> > > totally fine but with the incoming block size larger than page size
> > > (bs > ps) support, the requirement is no longer met for direct IOs.
> > > 
> > > Because iomap_dio_bio_iter() calls bio_iov_iter_get_pages(), only
> > > requiring alignment to be bdev_logical_block_size().
> > > 
> > > In the real world that value is either 512 or 4K, on 4K page sized
> > > systems it means bio_iov_iter_get_pages() can break the bio at any page
> > > boundary, breaking btrfs' requirement for bs > ps cases.
> > > 
> > > To address this problem, introduce a new public iomap dio flag,
> > > IOMAP_DIO_FSBLOCK_ALIGNED.
> > > 
> > > When calling __iomap_dio_rw() with that new flag, iomap_dio::flags will
> > > inherit that new flag, and iomap_dio_bio_iter() will take fs block size
> > > into the calculation of the alignment, and pass the alignment to
> > > bio_iov_iter_get_pages(), respecting the fs block size requirement.
> > > 
> > > The initial user of this flag will be btrfs, which needs to calculate the
> > > checksum for direct read and thus requires the biovec to be fs block
> > > aligned for the incoming bs > ps support.
> > > 
> > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > > Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >   fs/iomap/direct-io.c  | 13 ++++++++++++-
> > >   include/linux/iomap.h |  8 ++++++++
> > >   2 files changed, 20 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> > > index 5d5d63efbd57..ce9cbd2bace0 100644
> > > --- a/fs/iomap/direct-io.c
> > > +++ b/fs/iomap/direct-io.c
> > > @@ -336,10 +336,18 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
> > >   	int nr_pages, ret = 0;
> > >   	u64 copied = 0;
> > >   	size_t orig_count;
> > > +	unsigned int alignment = bdev_logical_block_size(iomap->bdev);
> > >   	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1))
> > >   		return -EINVAL;
> > > +	/*
> > > +	 * Align to the larger one of bdev and fs block size, to meet the
> > > +	 * alignment requirement of both layers.
> > > +	 */
> > > +	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
> > > +		alignment = max(alignment, fs_block_size);
> > > +
> > >   	if (dio->flags & IOMAP_DIO_WRITE) {
> > >   		bio_opf |= REQ_OP_WRITE;
> > > @@ -434,7 +442,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
> > >   		bio->bi_end_io = iomap_dio_bio_end_io;
> > >   		ret = bio_iov_iter_get_pages(bio, dio->submit.iter,
> > > -				bdev_logical_block_size(iomap->bdev) - 1);
> > > +					     alignment - 1);
> > >   		if (unlikely(ret)) {
> > >   			/*
> > >   			 * We have to stop part way through an IO. We must fall
> > > @@ -639,6 +647,9 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> > >   	if (iocb->ki_flags & IOCB_NOWAIT)
> > >   		iomi.flags |= IOMAP_NOWAIT;
> > > +	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
> > > +		dio->flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
> > > +
> > >   	if (iov_iter_rw(iter) == READ) {
> > >   		/* reads can always complete inline */
> > >   		dio->flags |= IOMAP_DIO_INLINE_COMP;
> > > diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> > > index 73dceabc21c8..4da13fe24ce8 100644
> > > --- a/include/linux/iomap.h
> > > +++ b/include/linux/iomap.h
> > > @@ -518,6 +518,14 @@ struct iomap_dio_ops {
> > >    */
> > >   #define IOMAP_DIO_PARTIAL		(1 << 2)
> > > +/*
> > > + * Ensure each bio is aligned to fs block size.
> > > + *
> > > + * For filesystems which need to calculate/verify the checksum of each fs
> > > + * block. Otherwise they may not be able to handle unaligned bios.
> > > + */
> > > +#define IOMAP_DIO_FSBLOCK_ALIGNED	(1 << 3)
> > 
> > A new flag requires an update to IOMAP_F_FLAGS_STRINGS in trace.h for
> > tracepoint decoding.
> 
> I'm wondering who should fix this part.
> 
> The original author (myself) or Christoph?

Or the maintainer?

Here's the relevant patch for whomever actually commits it.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

diff --git i/fs/iomap/trace.h w/fs/iomap/trace.h
index a61c1dae474270..eae9c1e5c674c8 100644
--- i/fs/iomap/trace.h
+++ w/fs/iomap/trace.h
@@ -121,13 +121,14 @@ DEFINE_RANGE_EVENT(iomap_zero_iter);
        { IOMAP_F_STALE,        "STALE" }
 
 
 #define IOMAP_DIO_STRINGS \
        {IOMAP_DIO_FORCE_WAIT,  "DIO_FORCE_WAIT" }, \
        {IOMAP_DIO_OVERWRITE_ONLY, "DIO_OVERWRITE_ONLY" }, \
-       {IOMAP_DIO_PARTIAL,     "DIO_PARTIAL" }
+       {IOMAP_DIO_PARTIAL,     "DIO_PARTIAL" }, \
+       {IOMAP_DIO_FSBLOCK_ALIGNED,     "DIO_FSBLOCK" }
 
 DECLARE_EVENT_CLASS(iomap_class,
        TP_PROTO(struct inode *inode, struct iomap *iomap),
        TP_ARGS(inode, iomap),
        TP_STRUCT__entry(
                __field(dev_t, dev)

> Thanks,
> Qu
> 
> > 
> > The rest of the changes look ok to me, modulo hch's subsequent fixups.
> > 
> > --D
> > 
> > >   ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> > >   		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
> > >   		unsigned int dio_flags, void *private, size_t done_before);
> > > -- 
> > > 2.47.3
> > > 
> > > 
> 

Re: [PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Christoph Hellwig]

On Tue, Oct 28, 2025 at 08:11:21AM +1030, Qu Wenruo wrote:
> I'm wondering who should fix this part.
>
> The original author (myself) or Christoph?

Whoever resend it next :)

Unless you want to, I'll happily take care of it.

Any comments on my tweaks in the meantime?

Re: [PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Qu Wenruo]

在 2025/10/29 18:14, Christoph Hellwig 写道:
> On Tue, Oct 28, 2025 at 08:11:21AM +1030, Qu Wenruo wrote:
>> I'm wondering who should fix this part.
>>
>> The original author (myself) or Christoph?
> 
> Whoever resend it next :)
> 
> Unless you want to, I'll happily take care of it.

I'm totally fine if you can take care of it.

> 
> Any comments on my tweaks in the meantime?
> 

Patch 2 and 3 look good to me. Thank for catching the missing pos/length 
checks.

If you like you can even fold the fixes into this one.

Thanks,
Qu

Re: [PATCH 1/4] iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag

[Christoph Hellwig]

On Wed, Oct 29, 2025 at 06:30:47PM +1030, Qu Wenruo wrote:
> Patch 2 and 3 look good to me. Thank for catching the missing pos/length 
> checks.
>
> If you like you can even fold the fixes into this one.

That would be best.  I just want to hear from Christian if he's fine
with rebasing, as he said he already applied your original patch.
Although it still hasn't shown up in the tree anyway.