Re: [PATCH 2/9] block: optionally mark pages dirty in bio_release_pages

Chaitanya Kulkarni
2019-06-27T00:21:32Z

﻿On 6/26/19, 1:52 PM, "linux-block-owner@vger.kernel.org on behalf of Minwoo Im" <linux-block-owner@vger.kernel.org on behalf of minwoo.im.dev@gmail.com> wrote:

On 19-06-26 15:49:21, Christoph Hellwig wrote:
> A lot of callers of bio_release_pages also want to mark the released
> pages as dirty. Add a mark_dirty parameter to avoid a second
> relatively expensive bio_for_each_segment_all loop.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> block/bio.c | 12 +++++++-----
> include/linux/bio.h | 2 +-
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 9bc7d28ae997..7f3920b6baca 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -845,7 +845,7 @@ static void bio_get_pages(struct bio *bio)
> get_page(bvec->bv_page);
> }
>
> -void bio_release_pages(struct bio *bio)
> +void bio_release_pages(struct bio *bio, bool mark_dirty)
> {
> struct bvec_iter_all iter_all;
> struct bio_vec *bvec;
> @@ -853,8 +853,11 @@ void bio_release_pages(struct bio *bio)
> if (bio_flagged(bio, BIO_NO_PAGE_REF))
> return;
>
> - bio_for_each_segment_all(bvec, bio, iter_all)
> + bio_for_each_segment_all(bvec, bio, iter_all) {
> + if (mark_dirty && !PageCompound(bvec->bv_page))
> + set_page_dirty_lock(bvec->bv_page);

Christoph,

Could you please explain a bit why we should not make it dirty in case
of compound page?

Maybe because of [PATCH 7/9] block_dev: use bio_release_pages in bio_unmap_user :-

@@ -259,13 +258,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
}
__set_current_state(TASK_RUNNING);
- bio_for_each_segment_all(bvec, &bio, iter_all) {
- if (should_dirty && !PageCompound(bvec->bv_page))
- set_page_dirty_lock(bvec->bv_page);
- if (!bio_flagged(&bio, BIO_NO_PAGE_REF))
- put_page(bvec->bv_page);
- }
-
+ bio_release_pages(&bio, should_dirty);

I'll let Christoph confirm that.