* don't build bios/contexts over multiple iomaps @ 2026-06-19 5:00 Christoph Hellwig 2026-06-19 5:00 ` [PATCH] iomap: submit read bio after each extent Christoph Hellwig 0 siblings, 1 reply; 7+ messages in thread From: Christoph Hellwig @ 2026-06-19 5:00 UTC (permalink / raw) To: Christian Brauner, Darrick J. Wong Cc: Kelu Ye, Yifan Zhao, Ritesh Harjani, Joanne Koong, linux-erofs, linux-xfs, linux-fsdevel Hi all, this patch changes how iomap submits bios for reads. The old behavior to build up bios across iomap was already considered problematic for a while, but we now ran into a erofs bug because of it, so it's time to finally fix it. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] iomap: submit read bio after each extent 2026-06-19 5:00 don't build bios/contexts over multiple iomaps Christoph Hellwig @ 2026-06-19 5:00 ` Christoph Hellwig 2026-06-19 8:15 ` Gao Xiang ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Christoph Hellwig @ 2026-06-19 5:00 UTC (permalink / raw) To: Christian Brauner, Darrick J. Wong Cc: Kelu Ye, Yifan Zhao, Ritesh Harjani, Joanne Koong, linux-erofs, linux-xfs, linux-fsdevel Currently the iomap buffered read path tries to build up read context (i.e. bios for the typical block based case) over multiple iomaps as long as the sector matches. This does not take into account files that can map to multiple different devices. While this could be fixed by a bdev check in iomap_bio_read_folio_range, the building up of I/O over iomaps actually was a problem for the not yet merged ext2 iomap port, as that does want to send out I/O at the end of an indirect block mapped range. So instead of adding more checks move over to a model where a bio only spans a single iomap. File systems can still create iomap that span more than an extent if they want to build larger I/O. Reported-by: Kelu Ye <yekelu1@huawei.com> Reported-by: Yifan Zhao <zhaoyifan28@huawei.com> Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/iomap/buffered-io.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 8d4806dc46d4..7449cfd995d5 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -524,6 +524,14 @@ static void iomap_read_end(struct folio *folio, size_t bytes_submitted) } } +static void iomap_read_submit(struct iomap_iter *iter, + struct iomap_read_folio_ctx *ctx) +{ + if (ctx->read_ctx && ctx->ops->submit_read) + ctx->ops->submit_read(iter, ctx); + ctx->read_ctx = NULL; +} + static int iomap_read_folio_iter(struct iomap_iter *iter, struct iomap_read_folio_ctx *ctx, size_t *bytes_submitted) { @@ -642,12 +650,11 @@ void iomap_read_folio(const struct iomap_ops *ops, fsverity_readahead(ctx->vi, folio->index, folio_nr_pages(folio)); - while ((ret = iomap_iter(&iter, ops)) > 0) + while ((ret = iomap_iter(&iter, ops)) > 0) { iter.status = iomap_read_folio_iter(&iter, ctx, &bytes_submitted); - - if (ctx->read_ctx && ctx->ops->submit_read) - ctx->ops->submit_read(&iter, ctx); + iomap_read_submit(&iter, ctx); + } if (ctx->cur_folio) iomap_read_end(ctx->cur_folio, bytes_submitted); @@ -718,12 +725,11 @@ void iomap_readahead(const struct iomap_ops *ops, fsverity_readahead(ctx->vi, readahead_index(rac), readahead_count(rac)); - while (iomap_iter(&iter, ops) > 0) + while (iomap_iter(&iter, ops) > 0) { iter.status = iomap_readahead_iter(&iter, ctx, &cur_bytes_submitted); - - if (ctx->read_ctx && ctx->ops->submit_read) - ctx->ops->submit_read(&iter, ctx); + iomap_read_submit(&iter, ctx); + } if (ctx->cur_folio) iomap_read_end(ctx->cur_folio, cur_bytes_submitted); -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] iomap: submit read bio after each extent 2026-06-19 5:00 ` [PATCH] iomap: submit read bio after each extent Christoph Hellwig @ 2026-06-19 8:15 ` Gao Xiang 2026-06-19 10:15 ` Christian Brauner 2026-06-22 21:51 ` Joanne Koong 2 siblings, 0 replies; 7+ messages in thread From: Gao Xiang @ 2026-06-19 8:15 UTC (permalink / raw) To: Christoph Hellwig, Christian Brauner, Darrick J. Wong Cc: Kelu Ye, Yifan Zhao, Ritesh Harjani, Joanne Koong, linux-erofs, linux-xfs, linux-fsdevel On 2026/6/19 13:00, Christoph Hellwig wrote: > Currently the iomap buffered read path tries to build up read context > (i.e. bios for the typical block based case) over multiple iomaps as > long as the sector matches. This does not take into account files > that can map to multiple different devices. While this could be fixed > by a bdev check in iomap_bio_read_folio_range, the building up of I/O > over iomaps actually was a problem for the not yet merged ext2 iomap > port, as that does want to send out I/O at the end of an indirect > block mapped range. > > So instead of adding more checks move over to a model where a bio > only spans a single iomap. File systems can still create iomap > that span more than an extent if they want to build larger I/O. > > Reported-by: Kelu Ye <yekelu1@huawei.com> > Reported-by: Yifan Zhao <zhaoyifan28@huawei.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> Thanks, Gao Xiang ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iomap: submit read bio after each extent 2026-06-19 5:00 ` [PATCH] iomap: submit read bio after each extent Christoph Hellwig 2026-06-19 8:15 ` Gao Xiang @ 2026-06-19 10:15 ` Christian Brauner 2026-06-22 5:51 ` Gao Xiang 2026-06-22 21:51 ` Joanne Koong 2 siblings, 1 reply; 7+ messages in thread From: Christian Brauner @ 2026-06-19 10:15 UTC (permalink / raw) To: Darrick J. Wong, Christoph Hellwig Cc: Christian Brauner, Kelu Ye, Yifan Zhao, Ritesh Harjani, Joanne Koong, linux-erofs, linux-xfs, linux-fsdevel On Fri, 19 Jun 2026 07:00:53 +0200, Christoph Hellwig wrote: > Currently the iomap buffered read path tries to build up read context > (i.e. bios for the typical block based case) over multiple iomaps as > long as the sector matches. This does not take into account files > that can map to multiple different devices. While this could be fixed > by a bdev check in iomap_bio_read_folio_range, the building up of I/O > over iomaps actually was a problem for the not yet merged ext2 iomap > port, as that does want to send out I/O at the end of an indirect > block mapped range. > > [...] Applied to the vfs-7.3.iomap branch of the vfs/vfs.git tree. Patches in the vfs-7.3.iomap branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs-7.3.iomap [1/1] iomap: submit read bio after each extent https://git.kernel.org/vfs/vfs/c/dce5617fca3a ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iomap: submit read bio after each extent 2026-06-19 10:15 ` Christian Brauner @ 2026-06-22 5:51 ` Gao Xiang 0 siblings, 0 replies; 7+ messages in thread From: Gao Xiang @ 2026-06-22 5:51 UTC (permalink / raw) To: Christian Brauner, Darrick J. Wong, Christoph Hellwig Cc: Kelu Ye, Yifan Zhao, Ritesh Harjani, Joanne Koong, linux-erofs, linux-xfs, linux-fsdevel Hi Christian, On 2026/6/19 18:15, Christian Brauner wrote: > On Fri, 19 Jun 2026 07:00:53 +0200, Christoph Hellwig wrote: >> Currently the iomap buffered read path tries to build up read context >> (i.e. bios for the typical block based case) over multiple iomaps as >> long as the sector matches. This does not take into account files >> that can map to multiple different devices. While this could be fixed >> by a bdev check in iomap_bio_read_folio_range, the building up of I/O >> over iomaps actually was a problem for the not yet merged ext2 iomap >> port, as that does want to send out I/O at the end of an indirect >> block mapped range. >> >> [...] > > Applied to the vfs-7.3.iomap branch of the vfs/vfs.git tree. > Patches in the vfs-7.3.iomap branch should appear in linux-next soon. btw, could we address this issue in the 7.2 cycle? Not because that is a quite common case and needs a rush fix, but Just because that can be reproducible under given circumstances and users can get corrupted data. Or if we need to hear thoughts from others, I think we could delay one or two weeks though. Thanks, Gao Xiang ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iomap: submit read bio after each extent 2026-06-19 5:00 ` [PATCH] iomap: submit read bio after each extent Christoph Hellwig 2026-06-19 8:15 ` Gao Xiang 2026-06-19 10:15 ` Christian Brauner @ 2026-06-22 21:51 ` Joanne Koong 2026-06-23 13:52 ` Christoph Hellwig 2 siblings, 1 reply; 7+ messages in thread From: Joanne Koong @ 2026-06-22 21:51 UTC (permalink / raw) To: Christoph Hellwig Cc: Christian Brauner, Darrick J. Wong, Kelu Ye, Yifan Zhao, Ritesh Harjani, linux-erofs, linux-xfs, linux-fsdevel On Thu, Jun 18, 2026 at 10:01 PM Christoph Hellwig <hch@lst.de> wrote: > > Currently the iomap buffered read path tries to build up read context > (i.e. bios for the typical block based case) over multiple iomaps as > long as the sector matches. This does not take into account files > that can map to multiple different devices. While this could be fixed > by a bdev check in iomap_bio_read_folio_range, the building up of I/O > over iomaps actually was a problem for the not yet merged ext2 iomap > port, as that does want to send out I/O at the end of an indirect > block mapped range. > > So instead of adding more checks move over to a model where a bio > only spans a single iomap. File systems can still create iomap > that span more than an extent if they want to build larger I/O. > > Reported-by: Kelu Ye <yekelu1@huawei.com> > Reported-by: Yifan Zhao <zhaoyifan28@huawei.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/iomap/buffered-io.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 8d4806dc46d4..7449cfd995d5 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -524,6 +524,14 @@ static void iomap_read_end(struct folio *folio, size_t bytes_submitted) > } > } > > +static void iomap_read_submit(struct iomap_iter *iter, > + struct iomap_read_folio_ctx *ctx) > +{ > + if (ctx->read_ctx && ctx->ops->submit_read) > + ctx->ops->submit_read(iter, ctx); > + ctx->read_ctx = NULL; Does it make sense to move this line to the bio submit_read callback instead of unconditionally clearing it here? If we clear it here, I think this makes read_ctx only able to hold per-mapping state instead of also being able to hold persistent state across mappings. fuse currently uses read_ctx to hold per-request state (though this patch wouldn't break anything since fuse only ever returns one mapping covering the whole request). Thanks, Joanne ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iomap: submit read bio after each extent 2026-06-22 21:51 ` Joanne Koong @ 2026-06-23 13:52 ` Christoph Hellwig 0 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2026-06-23 13:52 UTC (permalink / raw) To: Joanne Koong Cc: Christoph Hellwig, Christian Brauner, Darrick J. Wong, Kelu Ye, Yifan Zhao, Ritesh Harjani, linux-erofs, linux-xfs, linux-fsdevel On Mon, Jun 22, 2026 at 02:51:36PM -0700, Joanne Koong wrote: > Does it make sense to move this line to the bio submit_read callback > instead of unconditionally clearing it here? If we clear it here, I > think this makes read_ctx only able to hold per-mapping state instead > of also being able to hold persistent state across mappings. fuse > currently uses read_ctx to hold per-request state (though this patch > wouldn't break anything since fuse only ever returns one mapping > covering the whole request). I've done that for v2, also we also need a new argument to ->submit_read for that to be useful. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-06-23 13:53 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-19 5:00 don't build bios/contexts over multiple iomaps Christoph Hellwig 2026-06-19 5:00 ` [PATCH] iomap: submit read bio after each extent Christoph Hellwig 2026-06-19 8:15 ` Gao Xiang 2026-06-19 10:15 ` Christian Brauner 2026-06-22 5:51 ` Gao Xiang 2026-06-22 21:51 ` Joanne Koong 2026-06-23 13:52 ` Christoph Hellwig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.