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.
Changes
=======
ver #5)
- Split from patch that uses iov_iter_extract_pages().
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Al Viro <viro@zeniv.linux.org.uk>
cc: Jens Axboe <axboe@kernel.dk>
cc: Jan Kara <jack@suse.cz>
cc: Christoph Hellwig <hch@lst.de>
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
---
block/bio.c | 9 ++++++++-
include/linux/bio.h | 2 +-
include/linux/blk_types.h | 2 +-
3 files changed, 10 insertions(+), 3 deletions(-)
Actually, should I make it so that the bottom two bits of bi_flags are a
four-state variable and make it such that bio_release_page() gives a warning
if the state is 0 - ie. unset?
The states would then be, say:
0 WARN(), do no cleanup
1 FOLL_GET
2 FOLL_PUT
3 do no cleanup
This should help debug any places, such as iomap_dio_zero() that I just found,
that add pages with refs without calling iov_iter_extract_pages().
David
On Wed, Jan 18, 2023 at 02:00:54PM +0000, David Howells wrote:
> Actually, should I make it so that the bottom two bits of bi_flags are a
> four-state variable and make it such that bio_release_page() gives a warning
> if the state is 0 - ie. unset?
>
> The states would then be, say:
>
> 0 WARN(), do no cleanup
> 1 FOLL_GET
> 2 FOLL_PUT
> 3 do no cleanup
>
> This should help debug any places, such as iomap_dio_zero() that I just found,
> that add pages with refs without calling iov_iter_extract_pages().
I don't really see a point. The fundamental use case of the bio itself
isn't really to this at all. So we're stealing one, or in the future
two bits mostly to optimize some direct I/O use cases. In fact I
wonder if instead we should just drop this micro-optimization entirely
an just add a member for the foll flags to the direct I/O container
structures (struct blkdev_dio, strut iomap_dio, struct dio, or just on
stack for __blkdev_direct_IO_simple and zonefs_file_dio_append) and
pass that to bio_release_pages.
@@ -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);
}
@@ -496,7 +496,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 */