john.hubbard at gmail.com
2019-Jul-24 04:25 UTC
[PATCH 03/12] block: bio_release_pages: use flags arg instead of bool
From: John Hubbard <jhubbard at nvidia.com> In commit d241a95f3514 ("block: optionally mark pages dirty in bio_release_pages"), new "bool mark_dirty" argument was added to bio_release_pages. In upcoming work, another bool argument (to indicate that the pages came from get_user_pages) is going to be added. That's one bool too many, because it's not desirable have calls of the form: foo(true, false, true, etc); In order to prepare for that, change the argument from a bool, to a typesafe (enum-based) flags argument. Cc: Christoph Hellwig <hch at infradead.org> Cc: J?r?me Glisse <jglisse at redhat.com> Cc: Minwoo Im <minwoo.im.dev at gmail.com> Cc: Jens Axboe <axboe at kernel.dk> Signed-off-by: John Hubbard <jhubbard at nvidia.com> --- block/bio.c | 12 ++++++------ fs/block_dev.c | 4 ++-- fs/direct-io.c | 2 +- include/linux/bio.h | 13 ++++++++++++- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/block/bio.c b/block/bio.c index 299a0e7651ec..7675e2de509d 100644 --- a/block/bio.c +++ b/block/bio.c @@ -833,7 +833,7 @@ int bio_add_page(struct bio *bio, struct page *page, } EXPORT_SYMBOL(bio_add_page); -void bio_release_pages(struct bio *bio, bool mark_dirty) +void bio_release_pages(struct bio *bio, enum bio_rp_flags_t flags) { struct bvec_iter_all iter_all; struct bio_vec *bvec; @@ -842,7 +842,7 @@ void bio_release_pages(struct bio *bio, bool mark_dirty) return; bio_for_each_segment_all(bvec, bio, iter_all) { - if (mark_dirty && !PageCompound(bvec->bv_page)) + if ((flags & BIO_RP_MARK_DIRTY) && !PageCompound(bvec->bv_page)) set_page_dirty_lock(bvec->bv_page); put_page(bvec->bv_page); } @@ -1421,7 +1421,7 @@ struct bio *bio_map_user_iov(struct request_queue *q, return bio; out_unmap: - bio_release_pages(bio, false); + bio_release_pages(bio, BIO_RP_NORMAL); bio_put(bio); return ERR_PTR(ret); } @@ -1437,7 +1437,7 @@ struct bio *bio_map_user_iov(struct request_queue *q, */ void bio_unmap_user(struct bio *bio) { - bio_release_pages(bio, bio_data_dir(bio) == READ); + bio_release_pages(bio, bio_rp_dirty_flag(bio_data_dir(bio) == READ)); bio_put(bio); bio_put(bio); } @@ -1683,7 +1683,7 @@ static void bio_dirty_fn(struct work_struct *work) while ((bio = next) != NULL) { next = bio->bi_private; - bio_release_pages(bio, true); + bio_release_pages(bio, BIO_RP_MARK_DIRTY); bio_put(bio); } } @@ -1699,7 +1699,7 @@ void bio_check_pages_dirty(struct bio *bio) goto defer; } - bio_release_pages(bio, false); + bio_release_pages(bio, BIO_RP_NORMAL); bio_put(bio); return; defer: diff --git a/fs/block_dev.c b/fs/block_dev.c index 4707dfff991b..9fe6616f8788 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -259,7 +259,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, } __set_current_state(TASK_RUNNING); - bio_release_pages(&bio, should_dirty); + bio_release_pages(&bio, bio_rp_dirty_flag(should_dirty)); if (unlikely(bio.bi_status)) ret = blk_status_to_errno(bio.bi_status); @@ -329,7 +329,7 @@ static void blkdev_bio_end_io(struct bio *bio) if (should_dirty) { bio_check_pages_dirty(bio); } else { - bio_release_pages(bio, false); + bio_release_pages(bio, BIO_RP_NORMAL); bio_put(bio); } } diff --git a/fs/direct-io.c b/fs/direct-io.c index ae196784f487..423ef431ddda 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -551,7 +551,7 @@ static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio) if (dio->is_async && should_dirty) { bio_check_pages_dirty(bio); /* transfers ownership */ } else { - bio_release_pages(bio, should_dirty); + bio_release_pages(bio, bio_rp_dirty_flag(should_dirty)); bio_put(bio); } return err; diff --git a/include/linux/bio.h b/include/linux/bio.h index 3cdb84cdc488..2715e55679c1 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -440,7 +440,18 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page, void __bio_add_page(struct bio *bio, struct page *page, unsigned int len, unsigned int off); int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter); -void bio_release_pages(struct bio *bio, bool mark_dirty); + +enum bio_rp_flags_t { + BIO_RP_NORMAL = 0, + BIO_RP_MARK_DIRTY = 1, +}; + +static inline enum bio_rp_flags_t bio_rp_dirty_flag(bool mark_dirty) +{ + return mark_dirty ? BIO_RP_MARK_DIRTY : BIO_RP_NORMAL; +} + +void bio_release_pages(struct bio *bio, enum bio_rp_flags_t flags); struct rq_map_data; extern struct bio *bio_map_user_iov(struct request_queue *, struct iov_iter *, gfp_t); -- 2.22.0
Jerome Glisse
2019-Jul-29 20:57 UTC
[PATCH 03/12] block: bio_release_pages: use flags arg instead of bool
On Tue, Jul 23, 2019 at 10:30:53PM -0700, Christoph Hellwig wrote:> On Tue, Jul 23, 2019 at 09:25:09PM -0700, john.hubbard at gmail.com wrote: > > From: John Hubbard <jhubbard at nvidia.com> > > > > In commit d241a95f3514 ("block: optionally mark pages dirty in > > bio_release_pages"), new "bool mark_dirty" argument was added to > > bio_release_pages. > > > > In upcoming work, another bool argument (to indicate that the pages came > > from get_user_pages) is going to be added. That's one bool too many, > > because it's not desirable have calls of the form: > > All pages releases by bio_release_pages should come from > get_get_user_pages, so I don't really see the point here.No they do not all comes from GUP for see various callers of bio_check_pages_dirty() for instance iomap_dio_zero() I have carefully tracked down all this and i did not do anyconvertion just for the fun of it :) Cheers, J?r?me
Possibly Parallel Threads
- [PATCH 03/12] block: bio_release_pages: use flags arg instead of bool
- [PATCH 03/12] block: bio_release_pages: use flags arg instead of bool
- [PATCH 03/12] block: bio_release_pages: use flags arg instead of bool
- [PATCH 03/12] block: bio_release_pages: use flags arg instead of bool
- [PATCH 03/12] block: bio_release_pages: use flags arg instead of bool