| Message ID | 20230124170108.1070389-6-dhowells@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iov_iter: Improve page extraction (pin or just list) | expand |
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.
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 */ + bio_set_flag(bio, BIO_PAGE_REFFED); sdio->bio = bio; sdio->logical_offset_in_bio = sdio->cur_page_fs_offset; } diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 47db4ead1e74..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; - bio_set_flag(bio, BIO_NO_PAGE_REF); __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 10366b8bdb13..805957c99147 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -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); } diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 99be590f952f..7daa261f4f98 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, /* put pages in bio_release_pages() */ BIO_CLONED, /* doesn't own data */ BIO_BOUNCED, /* bio is a bounce bio */ BIO_QUIET, /* Make BIO Quiet */