[v9,5/8] block: Replace BIO_NO_PAGE_REF with BIO_PAGE_REFFED with inverted logic
Commit Message
From: Christoph Hellwig <hch@lst.de>
Replace BIO_NO_PAGE_REF with a BIO_PAGE_REFFED flag that has the inverted
meaning is only set when a page reference has been acquired that needs to
be released by bio_release_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: Matthew Wilcox <willy@infradead.org>
cc: Logan Gunthorpe <logang@deltatee.com>
cc: linux-block@vger.kernel.org
---
Notes:
ver #8)
- Don't default to BIO_PAGE_REFFED [hch].
ver #5)
- Split from patch that uses iov_iter_extract_pages().
block/bio.c | 2 +-
block/blk-map.c | 1 +
fs/direct-io.c | 2 ++
fs/iomap/direct-io.c | 1 -
include/linux/bio.h | 2 +-
include/linux/blk_types.h | 2 +-
6 files changed, 6 insertions(+), 4 deletions(-)
Comments
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 1/24/23 09:01, David Howells wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Replace BIO_NO_PAGE_REF with a BIO_PAGE_REFFED flag that has the inverted
> meaning is only set when a page reference has been acquired that needs to
> be released by bio_release_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: Matthew Wilcox <willy@infradead.org>
> cc: Logan Gunthorpe <logang@deltatee.com>
> cc: linux-block@vger.kernel.org
> ---
>
> Notes:
> ver #8)
> - Don't default to BIO_PAGE_REFFED [hch].
>
> ver #5)
> - Split from patch that uses iov_iter_extract_pages().
>
> block/bio.c | 2 +-
> block/blk-map.c | 1 +
> fs/direct-io.c | 2 ++
> fs/iomap/direct-io.c | 1 -
> include/linux/bio.h | 2 +-
> include/linux/blk_types.h | 2 +-
> 6 files changed, 6 insertions(+), 4 deletions(-)
One documentation nit below, but either way,
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
>
> diff --git a/block/bio.c b/block/bio.c
> index 683444e6b711..851c23641a0d 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1198,7 +1198,6 @@ 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_set_flag(bio, BIO_CLONED);
> }
>
> @@ -1343,6 +1342,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> return 0;
> }
>
> + bio_set_flag(bio, BIO_PAGE_REFFED);
> do {
> ret = __bio_iov_iter_get_pages(bio, iter);
> } while (!ret && iov_iter_count(iter) && !bio_full(bio, 0));
> diff --git a/block/blk-map.c b/block/blk-map.c
> index 7db52ad5b2d0..0e2b0a861ba3 100644
> --- a/block/blk-map.c
> +++ b/block/blk-map.c
> @@ -282,6 +282,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
> if (blk_queue_pci_p2pdma(rq->q))
> extraction_flags |= ITER_ALLOW_P2PDMA;
>
> + bio_set_flag(bio, BIO_PAGE_REFFED);
> while (iov_iter_count(iter)) {
> struct page **pages, *stack_pages[UIO_FASTIOV];
> ssize_t bytes;
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 03d381377ae1..07810465fc9d 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -403,6 +403,8 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
> bio->bi_end_io = dio_bio_end_aio;
> else
> bio->bi_end_io = dio_bio_end_io;
> + /* for now require references for all pages */
Maybe just delete this comment?
thanks,
John Hubbard <jhubbard@nvidia.com> wrote:
> > + /* for now require references for all pages */
>
> Maybe just delete this comment?
Christoph added that. Presumably because this really should move to pinning
or be replaced with iomap, but it's not straightforward either way. Christoph?
David
On Tue, Jan 24, 2023 at 09:17:57PM +0000, David Howells wrote:
> John Hubbard <jhubbard@nvidia.com> wrote:
>
> > > + /* for now require references for all pages */
> >
> > Maybe just delete this comment?
>
> Christoph added that. Presumably because this really should move to pinning
> or be replaced with iomap, but it's not straightforward either way. Christoph?
Mostly because it adds the flag when allocating the bio, and not where
doing the gup. If John thinks it adds more confusion than it helps, we
can drop the comment.
That being said you had a conversion in an earlier version of the
series, and once the current batch of patches is in we should
resurrected it ASAP as that will allow us to kill BIO_FLAG_REFFED.
@@ -1198,7 +1198,6 @@ 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_set_flag(bio, BIO_CLONED);
}
@@ -1343,6 +1342,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
return 0;
}
+ bio_set_flag(bio, BIO_PAGE_REFFED);
do {
ret = __bio_iov_iter_get_pages(bio, iter);
} while (!ret && iov_iter_count(iter) && !bio_full(bio, 0));
@@ -282,6 +282,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
if (blk_queue_pci_p2pdma(rq->q))
extraction_flags |= ITER_ALLOW_P2PDMA;
+ bio_set_flag(bio, BIO_PAGE_REFFED);
while (iov_iter_count(iter)) {
struct page **pages, *stack_pages[UIO_FASTIOV];
ssize_t bytes;
@@ -403,6 +403,8 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
bio->bi_end_io = dio_bio_end_aio;
else
bio->bi_end_io = dio_bio_end_io;
+ /* for now require references for all pages */
+ bio_set_flag(bio, BIO_PAGE_REFFED);
sdio->bio = bio;
sdio->logical_offset_in_bio = sdio->cur_page_fs_offset;
}
@@ -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;
- bio_set_flag(bio, BIO_NO_PAGE_REF);
__bio_add_page(bio, page, len, 0);
iomap_dio_submit_bio(iter, dio, bio, pos);
}
@@ -484,7 +484,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, /* put pages in bio_release_pages() */
BIO_CLONED, /* doesn't own data */
BIO_BOUNCED, /* bio is a bounce bio */
BIO_QUIET, /* Make BIO Quiet */