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

Darrick J. Wong
2025-10-27T21:56:50Z

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