[PATCH 03/12] iomap: add bioset in iomap_read_folio_ops for filesystems to use own bioset

Goldwyn Rodrigues posted 12 patches 1 month, 3 weeks ago
[PATCH 03/12] iomap: add bioset in iomap_read_folio_ops for filesystems to use own bioset
Posted by Goldwyn Rodrigues 1 month, 3 weeks ago
From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Allocate the bio from the bioset provided in iomap_read_folio_ops.
If no bioset is provided, fs_bio_set is used which is the standard
bioset for filesystems.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/iomap/buffered-io.c | 20 ++++++++++++++------
 include/linux/iomap.h  |  6 ++++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 71370bbe466c..d007b4a8307c 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -421,6 +421,7 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
 	if (!ctx->bio ||
 	    bio_end_sector(ctx->bio) != sector ||
 	    !bio_add_folio(ctx->bio, folio, plen, poff)) {
+		struct bio_set *bioset;
 		gfp_t gfp = mapping_gfp_constraint(folio->mapping, GFP_KERNEL);
 		gfp_t orig_gfp = gfp;
 		unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);
@@ -434,17 +435,24 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
 
 		if (ctx->rac) /* same as readahead_gfp_mask */
 			gfp |= __GFP_NORETRY | __GFP_NOWARN;
-		ctx->bio = bio_alloc(iomap->bdev, bio_max_segs(nr_vecs),
-				     REQ_OP_READ, gfp);
+
+		if (ctx->ops && ctx->ops->bio_set)
+			bioset = ctx->ops->bio_set;
+		else
+			bioset = &fs_bio_set;
+
+		ctx->bio = bio_alloc_bioset(iomap->bdev, bio_max_segs(nr_vecs),
+				REQ_OP_READ, gfp, bioset);
+
 		/*
 		 * If the bio_alloc fails, try it again for a single page to
 		 * avoid having to deal with partial page reads.  This emulates
 		 * what do_mpage_read_folio does.
 		 */
-		if (!ctx->bio) {
-			ctx->bio = bio_alloc(iomap->bdev, 1, REQ_OP_READ,
-					     orig_gfp);
-		}
+		if (!ctx->bio)
+			ctx->bio = bio_alloc_bioset(iomap->bdev, 1, REQ_OP_READ,
+					orig_gfp, bioset);
+
 		if (ctx->rac)
 			ctx->bio->bi_opf |= REQ_RAHEAD;
 		ctx->bio->bi_iter.bi_sector = sector;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 5b775213920e..f876d16353c6 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -262,6 +262,12 @@ struct iomap_read_folio_ops {
 	 * bio, such as csum calculations or multi-device bio split
 	 */
 	void (*submit_io)(struct inode *inode, struct bio *bio);
+
+	/*
+	 * Optional, allows filesystem to specify own bio_set, so new bio's
+	 * can be allocated from the provided bio_set.
+	 */
+	struct bio_set *bio_set;
 };
 
 ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
-- 
2.46.1
Re: [PATCH 03/12] iomap: add bioset in iomap_read_folio_ops for filesystems to use own bioset
Posted by Christoph Hellwig 1 month, 2 weeks ago
>  	    !bio_add_folio(ctx->bio, folio, plen, poff)) {
> +		struct bio_set *bioset;
>  		gfp_t gfp = mapping_gfp_constraint(folio->mapping, GFP_KERNEL);
>  		gfp_t orig_gfp = gfp;
>  		unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);

Nit: I try to keep variables just declared and not initialized after
those initialized at declaration time.

> +
> +		if (ctx->ops && ctx->ops->bio_set)
> +			bioset = ctx->ops->bio_set;
> +		else
> +			bioset = &fs_bio_set;
> +
> +		ctx->bio = bio_alloc_bioset(iomap->bdev, bio_max_segs(nr_vecs),
> +				REQ_OP_READ, gfp, bioset);
> +

But it would be nice to move this logic into a helper, similar to what
is done in the direct I/O code.  That should robably include
picking the gfp flags from the ctx.
Re: [PATCH 03/12] iomap: add bioset in iomap_read_folio_ops for filesystems to use own bioset
Posted by Goldwyn Rodrigues 1 month, 2 weeks ago
On  2:45 08/10, Christoph Hellwig wrote:
> >  	    !bio_add_folio(ctx->bio, folio, plen, poff)) {
> > +		struct bio_set *bioset;
> >  		gfp_t gfp = mapping_gfp_constraint(folio->mapping, GFP_KERNEL);
> >  		gfp_t orig_gfp = gfp;
> >  		unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);
> 
> Nit: I try to keep variables just declared and not initialized after
> those initialized at declaration time.
> 
> > +
> > +		if (ctx->ops && ctx->ops->bio_set)
> > +			bioset = ctx->ops->bio_set;
> > +		else
> > +			bioset = &fs_bio_set;
> > +
> > +		ctx->bio = bio_alloc_bioset(iomap->bdev, bio_max_segs(nr_vecs),
> > +				REQ_OP_READ, gfp, bioset);
> > +
> 
> But it would be nice to move this logic into a helper, similar to what
> is done in the direct I/O code.  That should robably include
> picking the gfp flags from the ctx.
> 

Agree. I will put this in the next version.

-- 
Goldwyn