All of lore.kernel.org
 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 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.