Linux XFS filesystem development
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Christian Brauner <brauner@kernel.org>,
	Kelu Ye <yekelu1@huawei.com>, Yifan Zhao <zhaoyifan28@huawei.com>,
	Ritesh Harjani <ritesh.list@gmail.com>,
	Joanne Koong <joannelkoong@gmail.com>,
	Namjae Jeon <linkinjeon@kernel.org>,
	Sungjong Seo <sj1557.seo@samsung.com>,
	Hyunchul Lee <hyc.lee@gmail.com>,
	Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	fuse-devel@lists.linux.dev, ntfs3@lists.linux.dev,
	linux-erofs@lists.ozlabs.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 2/2] iomap: submit read bio after each extent
Date: Thu, 25 Jun 2026 10:47:58 -0700	[thread overview]
Message-ID: <20260625174758.GE6078@frogsfrogsfrogs> (raw)
In-Reply-To: <20260625120803.2462291-3-hch@lst.de>

On Thu, Jun 25, 2026 at 02:07:57PM +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.

This really puts the onus on block-mapped filesystems (e.g. ext2) to
merge adjacent maps into extents.  Granted they *probably* already have
been doing that.

> So instead of adding more checks move over to a model where a bio only
> spans a single iomap.  Change ->submit_read to be called after each
> iteration, and pass a force argument to indicate that the bio must
> be submitted set on the last iteration.  Switch the bio based users
> to always submit, while keeping the single submit for fuse.

Is fuse the sole reason for the "force" parameter to exist?  I wonder if
fuse could drop its submit_read function and call fuse_send_readpages
after the iomap_read{ahead,folio} function returns?

--D

> Fixes: dfeab2e95a75 ("erofs: add multiple device support")
> Reported-by: Kelu Ye <yekelu1@huawei.com>
> Reported-by: Yifan Zhao <zhaoyifan28@huawei.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Yifan Zhao <zhaoyifan28@huawei.com>
> ---
>  fs/exfat/iomap.c       |  2 +-
>  fs/fuse/file.c         |  6 +++++-
>  fs/iomap/bio.c         |  6 ++++--
>  fs/iomap/buffered-io.c | 21 +++++++++++++--------
>  fs/ntfs/aops.c         |  2 +-
>  fs/ntfs3/inode.c       |  2 +-
>  fs/xfs/xfs_aops.c      |  3 ++-
>  include/linux/iomap.h  |  2 +-
>  8 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/exfat/iomap.c b/fs/exfat/iomap.c
> index 190fc6471f84..c428a949120e 100644
> --- a/fs/exfat/iomap.c
> +++ b/fs/exfat/iomap.c
> @@ -251,7 +251,7 @@ static void exfat_iomap_read_end_io(struct bio *bio)
>  }
>  
>  static void exfat_iomap_bio_submit_read(const struct iomap_iter *iter,
> -		struct iomap_read_folio_ctx *ctx)
> +		struct iomap_read_folio_ctx *ctx, bool force)
>  {
>  	iomap_bio_submit_read_endio(iter, ctx, exfat_iomap_read_end_io);
>  }
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index e052a0d44dee..6fa3b1f55c95 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -982,13 +982,17 @@ static int fuse_iomap_read_folio_range_async(const struct iomap_iter *iter,
>  }
>  
>  static void fuse_iomap_submit_read(const struct iomap_iter *iter,
> -		struct iomap_read_folio_ctx *ctx)
> +		struct iomap_read_folio_ctx *ctx, bool force)
>  {
>  	struct fuse_fill_read_data *data = ctx->read_ctx;
>  
> +	if (!force)
> +		return;
> +
>  	if (data->ia)
>  		fuse_send_readpages(data->ia, data->file, data->nr_bytes,
>  				    data->fc->async_read);
> +	ctx->read_ctx = NULL;
>  }
>  
>  static const struct iomap_read_ops fuse_iomap_read_ops = {
> diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
> index 0f31e35567b4..6aca1cd0622c 100644
> --- a/fs/iomap/bio.c
> +++ b/fs/iomap/bio.c
> @@ -87,11 +87,13 @@ void iomap_bio_submit_read_endio(const struct iomap_iter *iter,
>  	if (iter->iomap.flags & IOMAP_F_INTEGRITY)
>  		fs_bio_integrity_alloc(bio);
>  	submit_bio(bio);
> +
> +	ctx->read_ctx = NULL;
>  }
>  EXPORT_SYMBOL_GPL(iomap_bio_submit_read_endio);
>  
>  static void iomap_bio_submit_read(const struct iomap_iter *iter,
> -		struct iomap_read_folio_ctx *ctx)
> +		struct iomap_read_folio_ctx *ctx, bool force)
>  {
>  	return iomap_bio_submit_read_endio(iter, ctx, iomap_read_end_io);
>  }
> @@ -116,7 +118,7 @@ static void iomap_read_alloc_bio(const struct iomap_iter *iter,
>  
>  	/* Submit the existing range if there was one. */
>  	if (ctx->read_ctx)
> -		ctx->ops->submit_read(iter, ctx);
> +		ctx->ops->submit_read(iter, ctx, true);
>  
>  	/* Same as readahead_gfp_mask: */
>  	if (ctx->rac)
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 8d4806dc46d4..b1c3da8a97dc 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -524,6 +524,13 @@ static void iomap_read_end(struct folio *folio, size_t bytes_submitted)
>  	}
>  }
>  
> +static void iomap_submit_read(struct iomap_iter *iter,
> +		struct iomap_read_folio_ctx *ctx, bool force)
> +{
> +	if (ctx->read_ctx && ctx->ops->submit_read)
> +		ctx->ops->submit_read(iter, ctx, force);
> +}
> +
>  static int iomap_read_folio_iter(struct iomap_iter *iter,
>  		struct iomap_read_folio_ctx *ctx, size_t *bytes_submitted)
>  {
> @@ -642,12 +649,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_submit_read(&iter, ctx, !iter.iomap.length);
> +	}
>  
>  	if (ctx->cur_folio)
>  		iomap_read_end(ctx->cur_folio, bytes_submitted);
> @@ -718,12 +724,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_submit_read(&iter, ctx, !iter.iomap.length);
> +	}
>  
>  	if (ctx->cur_folio)
>  		iomap_read_end(ctx->cur_folio, cur_bytes_submitted);
> diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
> index f2bb56506046..f7bd55275d8c 100644
> --- a/fs/ntfs/aops.c
> +++ b/fs/ntfs/aops.c
> @@ -38,7 +38,7 @@ static void ntfs_iomap_read_end_io(struct bio *bio)
>  }
>  
>  static void ntfs_iomap_bio_submit_read(const struct iomap_iter *iter,
> -		struct iomap_read_folio_ctx *ctx)
> +		struct iomap_read_folio_ctx *ctx, bool force)
>  {
>  	iomap_bio_submit_read_endio(iter, ctx, ntfs_iomap_read_end_io);
>  }
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index f9600aba1548..cd05faebb806 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -607,7 +607,7 @@ static void ntfs_iomap_read_end_io(struct bio *bio)
>  }
>  
>  static void ntfs_iomap_bio_submit_read(const struct iomap_iter *iter,
> -		struct iomap_read_folio_ctx *ctx)
> +		struct iomap_read_folio_ctx *ctx, bool force)
>  {
>  	iomap_bio_submit_read_endio(iter, ctx, ntfs_iomap_read_end_io);
>  }
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 51293b6f331f..1b9a55e2f4a7 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -758,7 +758,8 @@ xfs_vm_bmap(
>  static void
>  xfs_bio_submit_read(
>  	const struct iomap_iter		*iter,
> -	struct iomap_read_folio_ctx	*ctx)
> +	struct iomap_read_folio_ctx	*ctx,
> +	bool				force)
>  {
>  	struct bio			*bio = ctx->read_ctx;
>  
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 56b43d594e6e..4d8893b02aaf 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -528,7 +528,7 @@ struct iomap_read_ops {
>  	 * This is optional.
>  	 */
>  	void (*submit_read)(const struct iomap_iter *iter,
> -			struct iomap_read_folio_ctx *ctx);
> +			struct iomap_read_folio_ctx *ctx, bool force);
>  
>  	/*
>  	 * Optional, allows filesystem to specify own bio_set, so new bio's
> -- 
> 2.53.0
> 
> 

  reply	other threads:[~2026-06-25 17:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25 12:07 don't build bios/contexts over multiple iomaps v3 Christoph Hellwig
2026-06-25 12:07 ` [PATCH 1/2] iomap: consolidate bio submission Christoph Hellwig
2026-06-25 17:27   ` Darrick J. Wong
2026-06-26  4:30     ` Christoph Hellwig
2026-06-25 12:07 ` [PATCH 2/2] iomap: submit read bio after each extent Christoph Hellwig
2026-06-25 17:47   ` Darrick J. Wong [this message]
2026-06-25 18:32     ` Joanne Koong
2026-06-26  4:33       ` Christoph Hellwig
2026-06-26  6:16         ` Christoph Hellwig
2026-06-26  6:20           ` Darrick J. Wong
2026-06-26 14:51           ` Joanne Koong
2026-06-26  4:31     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2026-06-23 13:51 don't build bios/contexts over multiple iomaps v2 Christoph Hellwig
2026-06-23 13:51 ` [PATCH 2/2] iomap: submit read bio after each extent Christoph Hellwig
2026-06-23 17:29   ` Joanne Koong
2026-06-24  7:34     ` Christoph Hellwig
2026-06-23 23:58   ` Namjae Jeon
2026-06-24  7:42   ` zhaoyifan (H)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260625174758.GE6078@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=brauner@kernel.org \
    --cc=fuse-devel@lists.linux.dev \
    --cc=hch@lst.de \
    --cc=hyc.lee@gmail.com \
    --cc=joannelkoong@gmail.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=ntfs3@lists.linux.dev \
    --cc=ritesh.list@gmail.com \
    --cc=sj1557.seo@samsung.com \
    --cc=yekelu1@huawei.com \
    --cc=zhaoyifan28@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox