Re: [PATCH 2/7] iomap: allow the file system to provide a bio_set for direct I/O

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

On Thu, May 05, 2022 at 03:11:10PM -0500, Christoph Hellwig wrote:
> Allow the file system to provide a specific bio_set for allocating
> direct I/O bios.  This will allow file systems that use the
> ->submit_io hook to stash away additional information for file system
> use.
> 
> To make use of this additional space for information in the completion
> path, the file system needs to override the ->bi_end_io callback and
> then call back into iomap, so export iomap_dio_bio_end_io for that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

LGTM,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c  | 18 ++++++++++++++----
>  include/linux/iomap.h | 11 +++++++++++
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index b08f5dc31780d..15929690d89e3 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -51,6 +51,15 @@ struct iomap_dio {
>  	};
>  };
>  
> +static struct bio *iomap_dio_alloc_bio(const struct iomap_iter *iter,
> +		struct iomap_dio *dio, unsigned short nr_vecs, unsigned int opf)
> +{
> +	if (dio->dops && dio->dops->bio_set)
> +		return bio_alloc_bioset(iter->iomap.bdev, nr_vecs, opf,
> +					GFP_KERNEL, dio->dops->bio_set);
> +	return bio_alloc(iter->iomap.bdev, nr_vecs, opf, GFP_KERNEL);
> +}
> +
>  static void iomap_dio_submit_bio(const struct iomap_iter *iter,
>  		struct iomap_dio *dio, struct bio *bio, loff_t pos)
>  {
> @@ -144,7 +153,7 @@ static inline void iomap_dio_set_error(struct iomap_dio *dio, int ret)
>  	cmpxchg(&dio->error, 0, ret);
>  }
>  
> -static void iomap_dio_bio_end_io(struct bio *bio)
> +void iomap_dio_bio_end_io(struct bio *bio)
>  {
>  	struct iomap_dio *dio = bio->bi_private;
>  	bool should_dirty = (dio->flags & IOMAP_DIO_DIRTY);
> @@ -176,16 +185,17 @@ static void iomap_dio_bio_end_io(struct bio *bio)
>  		bio_put(bio);
>  	}
>  }
> +EXPORT_SYMBOL_GPL(iomap_dio_bio_end_io);
>  
>  static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
>  		loff_t pos, unsigned len)
>  {
>  	struct inode *inode = file_inode(dio->iocb->ki_filp);
>  	struct page *page = ZERO_PAGE(0);
> -	int flags = REQ_SYNC | REQ_IDLE;
>  	struct bio *bio;
>  
> -	bio = bio_alloc(iter->iomap.bdev, 1, REQ_OP_WRITE | flags, GFP_KERNEL);
> +	bio = iomap_dio_alloc_bio(iter, dio, 1,
> +			REQ_OP_WRITE | REQ_SYNC | REQ_IDLE);
>  	fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
>  				  GFP_KERNEL);
>  	bio->bi_iter.bi_sector = iomap_sector(&iter->iomap, pos);
> @@ -311,7 +321,7 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
>  			goto out;
>  		}
>  
> -		bio = bio_alloc(iomap->bdev, nr_pages, bio_opf, GFP_KERNEL);
> +		bio = iomap_dio_alloc_bio(iter, dio, nr_pages, bio_opf);
>  		fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
>  					  GFP_KERNEL);
>  		bio->bi_iter.bi_sector = iomap_sector(iomap, pos);
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index b76f0dd149fb4..526c9e7f2eaf8 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -320,6 +320,16 @@ struct iomap_dio_ops {
>  		      unsigned flags);
>  	void (*submit_io)(const struct iomap_iter *iter, struct bio *bio,
>  		          loff_t file_offset);
> +
> +	/*
> +	 * Filesystems wishing to attach private information to a directio bio
> +	 * must provide a ->submit_io method that attaches the additional
> +	 * information to the bio and changes the ->bi_end_io callback to a
> +	 * custom function.  This function should, at a minimum, perform any
> +	 * relevant post-processing of the bio and end with a call to
> +	 * iomap_dio_bio_end_io.
> +	 */
> +	struct bio_set *bio_set;
>  };
>  
>  /*
> @@ -349,6 +359,7 @@ struct iomap_dio *__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, size_t done_before);
>  ssize_t iomap_dio_complete(struct iomap_dio *dio);
> +void iomap_dio_bio_end_io(struct bio *bio);
>  
>  #ifdef CONFIG_SWAP
>  struct file;
> -- 
> 2.30.2
>