diff mbox series

[v7,4/8] block: Rename BIO_NO_PAGE_REF to BIO_PAGE_REFFED and invert the meaning

Message ID 20230120175556.3556978-5-dhowells@redhat.com
State New
Headers
Series iov_iter: Improve page extraction (ref, pin or just list) |

Commit Message

David Howells Jan. 20, 2023, 5:55 p.m. UTC 
  Rename BIO_NO_PAGE_REF to BIO_PAGE_REFFED and invert the meaning.  In a
following patch I intend to add a BIO_PAGE_PINNED flag to indicate that the
page needs unpinning and this way both flags have the same logic.

Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
cc: Al Viro <viro@zeniv.linux.org.uk>
cc: Jens Axboe <axboe@kernel.dk>
cc: Jan Kara <jack@suse.cz>
cc: Matthew Wilcox <willy@infradead.org>
cc: Logan Gunthorpe <logang@deltatee.com>
cc: linux-block@vger.kernel.org
Link: https://lore.kernel.org/r/167305166150.1521586.10220949115402059720.stgit@warthog.procyon.org.uk/ # v4
Link: https://lore.kernel.org/r/167344730802.2425628.14034153595667416149.stgit@warthog.procyon.org.uk/ # v5
Link: https://lore.kernel.org/r/167391054631.2311931.7588488803802952158.stgit@warthog.procyon.org.uk/ # v6
---

Notes:
    ver #5)
     - Split from patch that uses iov_iter_extract_pages().

 block/bio.c               | 9 ++++++++-
 fs/iomap/direct-io.c      | 1 -
 include/linux/bio.h       | 2 +-
 include/linux/blk_types.h | 2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Jan. 21, 2023, 1:04 p.m. UTC | #1 
On Fri, Jan 20, 2023 at 05:55:52PM +0000, David Howells wrote:
> Rename BIO_NO_PAGE_REF to BIO_PAGE_REFFED and invert the meaning.  In a
> following patch I intend to add a BIO_PAGE_PINNED flag to indicate that the
> page needs unpinning and this way both flags have the same logic.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

As pointed out last time, we really should not set the flag by default.
When a bio is allocated there are no pages to be released, that only
happens when we add dio-like pages to the bio.  Instead of explaining
why I mean again, I've put together a version of this series that
implements this and my other suggestions here:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dio-pin-pages

This also tests fine with xfs and btrfs and nvme passthrough I/O.
David Howells Jan. 23, 2023, 9:38 a.m. UTC | #2 
Christoph Hellwig <hch@infradead.org> wrote:

> ... I've put together a version of this series that implements this and my
> other suggestions here:
> 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dio-pin-pages
> 
> This also tests fine with xfs and btrfs and nvme passthrough I/O.

That looks okay.  Do you need to put a Co-developed-by tag on the "block:
invert BIO_NO_PAGE_REF" patch?

Should I take that set of patches and send it on to Linus when the window
opens?  Or should it go through the block tree?

David
Christoph Hellwig Jan. 23, 2023, 9:56 a.m. UTC | #3 
On Mon, Jan 23, 2023 at 09:38:45AM +0000, David Howells wrote:
> That looks okay.  Do you need to put a Co-developed-by tag on the "block:
> invert BIO_NO_PAGE_REF" patch?

I can do if you want, although there's really not much left of the
original.

> Should I take that set of patches and send it on to Linus when the window
> opens?  Or should it go through the block tree?

I think the block tree would be better.
diff mbox series

Patch

diff --git a/block/bio.c b/block/bio.c
index 6a6c73d14bfd..cfe11f4799d1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -243,6 +243,10 @@  static void bio_free(struct bio *bio)
  * Users of this function have their own bio allocation. Subsequently,
  * they must remember to pair any call to bio_init() with bio_uninit()
  * when IO has completed, or when the bio is released.
+ *
+ * We set the initial assumption that pages attached to the bio will be
+ * released with put_page() by setting BIO_PAGE_REFFED; if the pages
+ * should not be put, this flag should be cleared.
  */
 void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
 	      unsigned short max_vecs, blk_opf_t opf)
@@ -274,6 +278,7 @@  void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 	bio->bi_integrity = NULL;
 #endif
+	bio_set_flag(bio, BIO_PAGE_REFFED);
 	bio->bi_vcnt = 0;
 
 	atomic_set(&bio->__bi_remaining, 1);
@@ -302,6 +307,7 @@  void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf)
 {
 	bio_uninit(bio);
 	memset(bio, 0, BIO_RESET_BYTES);
+	bio_set_flag(bio, BIO_PAGE_REFFED);
 	atomic_set(&bio->__bi_remaining, 1);
 	bio->bi_bdev = bdev;
 	if (bio->bi_bdev)
@@ -812,6 +818,7 @@  EXPORT_SYMBOL(bio_put);
 static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp)
 {
 	bio_set_flag(bio, BIO_CLONED);
+	bio_clear_flag(bio, BIO_PAGE_REFFED);
 	bio->bi_ioprio = bio_src->bi_ioprio;
 	bio->bi_iter = bio_src->bi_iter;
 
@@ -1198,7 +1205,7 @@  void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
 	bio->bi_io_vec = (struct bio_vec *)iter->bvec;
 	bio->bi_iter.bi_bvec_done = iter->iov_offset;
 	bio->bi_iter.bi_size = size;
-	bio_set_flag(bio, BIO_NO_PAGE_REF);
+	bio_clear_flag(bio, BIO_PAGE_REFFED);
 	bio_set_flag(bio, BIO_CLONED);
 }
 
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 9804714b1751..c0e75900e754 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -202,7 +202,6 @@  static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 	bio->bi_private = dio;
 	bio->bi_end_io = iomap_dio_bio_end_io;
 
-	get_page(page);
 	__bio_add_page(bio, page, len, 0);
 	iomap_dio_submit_bio(iter, dio, bio, pos);
 }
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 22078a28d7cb..63bfd91793f9 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -482,7 +482,7 @@  void zero_fill_bio(struct bio *bio);
 
 static inline void bio_release_pages(struct bio *bio, bool mark_dirty)
 {
-	if (!bio_flagged(bio, BIO_NO_PAGE_REF))
+	if (bio_flagged(bio, BIO_PAGE_REFFED))
 		__bio_release_pages(bio, mark_dirty);
 }
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 99be590f952f..86711fb0534a 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -318,7 +318,7 @@  struct bio {
  * bio flags
  */
 enum {
-	BIO_NO_PAGE_REF,	/* don't put release vec pages */
+	BIO_PAGE_REFFED,	/* Pages need refs putting (equivalent to FOLL_GET) */
 	BIO_CLONED,		/* doesn't own data */
 	BIO_BOUNCED,		/* bio is a bounce bio */
 	BIO_QUIET,		/* Make BIO Quiet */