Re: [PATCH 1/2] iomap: consolidate bio submission

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jun 25, 2026 at 02:07:56PM +0200, Christoph Hellwig wrote:
> Add a iomap_bio_submit_read_endio helper factored out of
> iomap_bio_submit_read to that all ->submit_read implementations for
> iomap_read_ops that use iomap_bio_read_folio_range can shared the
> logic.
> 
> Right now that logic is mostly trivial, but already has a bug for XFS
> because the XFS version is too trivial:  file system integrity validation
> needs a workqueue context and thus can't happen from the default iomap
> bi_end_io I/O handler.  Unfortunately the iomap refactoring just before
> fs integrity landed moved code around here and the call go misplaced,
> meaning it never got called.  The PI information still is verified by
> the block layer, but the offloading is less efficient (and the future
> userspace interface can't get at it).
> 
> Fixes: 0b10a370529c ("iomap: support T10 protection information")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/exfat/iomap.c      |  5 +----
>  fs/iomap/bio.c        | 13 ++++++++++---
>  fs/ntfs/aops.c        |  6 ++----
>  fs/ntfs3/inode.c      |  5 +----
>  fs/xfs/xfs_aops.c     |  3 +--
>  include/linux/iomap.h |  2 ++
>  6 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/exfat/iomap.c b/fs/exfat/iomap.c
> index 1aac38e63fe6..190fc6471f84 100644
> --- a/fs/exfat/iomap.c
> +++ b/fs/exfat/iomap.c
> @@ -253,10 +253,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 bio *bio = ctx->read_ctx;
> -
> -	bio->bi_end_io = exfat_iomap_read_end_io;
> -	submit_bio(bio);
> +	iomap_bio_submit_read_endio(iter, ctx, exfat_iomap_read_end_io);
>  }
>  
>  const struct iomap_read_ops exfat_iomap_bio_read_ops = {
> diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
> index 4504f4633f17..0f31e35567b4 100644
> --- a/fs/iomap/bio.c
> +++ b/fs/iomap/bio.c
> @@ -78,15 +78,23 @@ u32 iomap_finish_ioend_buffered_read(struct iomap_ioend *ioend)
>  	return __iomap_read_end_io(&ioend->io_bio, ioend->io_error);
>  }
>  
> -static void iomap_bio_submit_read(const struct iomap_iter *iter,
> -		struct iomap_read_folio_ctx *ctx)
> +void iomap_bio_submit_read_endio(const struct iomap_iter *iter,
> +		struct iomap_read_folio_ctx *ctx, bio_end_io_t end_io)
>  {
>  	struct bio *bio = ctx->read_ctx;
>  
> +	bio->bi_end_io = end_io;
>  	if (iter->iomap.flags & IOMAP_F_INTEGRITY)
>  		fs_bio_integrity_alloc(bio);

Ah, so the bug here is that all the pagecache readers should have been
allocating integrity information for the bio before submitting it?  And
because it doesn't, iomap_finish_ioend won't do the read verification?
So the block layer does it for us, and that's why we don't use the ioend
chaining?  And (I guess) the future userspace interface won't have any
means to get at the integrity data?

If the answers to all four questions is "yes" then I've understood this
fix well enough to declare

Cc: <stable@vger.kernel.org> # v7.1
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>


--D

>  	submit_bio(bio);
>  }
> +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)
> +{
> +	return iomap_bio_submit_read_endio(iter, ctx, iomap_read_end_io);
> +}
>  
>  static struct bio_set *iomap_read_bio_set(struct iomap_read_folio_ctx *ctx)
>  {
> @@ -127,7 +135,6 @@ static void iomap_read_alloc_bio(const struct iomap_iter *iter,
>  	if (ctx->rac)
>  		bio->bi_opf |= REQ_RAHEAD;
>  	bio->bi_iter.bi_sector = iomap_sector(iomap, iter->pos);
> -	bio->bi_end_io = iomap_read_end_io;
>  	bio_add_folio_nofail(bio, folio, plen,
>  			offset_in_folio(folio, iter->pos));
>  	ctx->read_ctx = bio;
> diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
> index 1fbf832ad165..f2bb56506046 100644
> --- a/fs/ntfs/aops.c
> +++ b/fs/ntfs/aops.c
> @@ -38,11 +38,9 @@ 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)
>  {
> -	struct bio *bio = ctx->read_ctx;
> -	bio->bi_end_io = ntfs_iomap_read_end_io;
> -	submit_bio(bio);
> +	iomap_bio_submit_read_endio(iter, ctx, ntfs_iomap_read_end_io);
>  }
>  
>  static const struct iomap_read_ops ntfs_iomap_bio_read_ops = {
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index 42af1abe17f8..f9600aba1548 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -609,10 +609,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 bio *bio = ctx->read_ctx;
> -
> -	bio->bi_end_io = ntfs_iomap_read_end_io;
> -	submit_bio(bio);
> +	iomap_bio_submit_read_endio(iter, ctx, ntfs_iomap_read_end_io);
>  }
>  
>  static const struct iomap_read_ops ntfs_iomap_bio_read_ops = {
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 2a0c54256e93..51293b6f331f 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -764,8 +764,7 @@ xfs_bio_submit_read(
>  
>  	/* defer read completions to the ioend workqueue */
>  	iomap_init_ioend(iter->inode, bio, ctx->read_ctx_file_offset, 0);
> -	bio->bi_end_io = xfs_end_bio;
> -	submit_bio(bio);
> +	iomap_bio_submit_read_endio(iter, ctx, xfs_end_bio);
>  }
>  
>  static const struct iomap_read_ops xfs_iomap_read_ops = {
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 3582ed1fe236..56b43d594e6e 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -622,6 +622,8 @@ extern struct bio_set iomap_ioend_bioset;
>  #ifdef CONFIG_BLOCK
>  int iomap_bio_read_folio_range(const struct iomap_iter *iter,
>  		struct iomap_read_folio_ctx *ctx, size_t plen);
> +void iomap_bio_submit_read_endio(const struct iomap_iter *iter,
> +		struct iomap_read_folio_ctx *ctx, bio_end_io_t end_io);
>  
>  extern const struct iomap_read_ops iomap_bio_read_ops;
>  
> -- 
> 2.53.0
> 
>