On 19-06-27 00:21:27, Chaitanya Kulkarni wrote:
>
>
> On 6/26/19, 1:52 PM, "linux-block-owner@xxxxxxxxxxxxxxx on behalf of Minwoo Im" <linux-block-owner@xxxxxxxxxxxxxxx on behalf of minwoo.im.dev@xxxxxxxxx> 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@xxxxxx>
> > ---
> > 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.
Chaitanya,
Thanks for sharing this. Actually I have seen that commit log but
didn't catch up why the compound pages are so. Now I understood what
Christoph has mentioned.
Thanks for your help, again.
