[v7,4/8] block: Rename BIO_NO_PAGE_REF to BIO_PAGE_REFFED and invert the meaning
Commit Message
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
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.
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
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.
@@ -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);
}
@@ -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);
}
@@ -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);
}
@@ -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 */