Messages in this thread | | | Subject | Re: [PATCH] iomap: Fix the write_count in iomap_add_to_ioend(). | From | Ritesh Harjani <> | Date | Fri, 21 Aug 2020 14:23:55 +0530 |
| |
On 8/21/20 11:37 AM, Christoph Hellwig wrote: > On Wed, Aug 19, 2020 at 03:58:41PM +0530, Anju T Sudhakar wrote: >> From: Ritesh Harjani <riteshh@linux.ibm.com> >> >> __bio_try_merge_page() may return same_page = 1 and merged = 0. >> This could happen when bio->bi_iter.bi_size + len > UINT_MAX. >> Handle this case in iomap_add_to_ioend() by incrementing write_count. >> This scenario mostly happens where we have too much dirty data accumulated. >> >> w/o the patch we hit below kernel warning, > > I think this is better fixed in the block layer rather than working > around the problem in the callers. Something like this: > > diff --git a/block/bio.c b/block/bio.c > index c63ba04bd62967..ef321cd1072e4e 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -879,8 +879,10 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page, > struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1]; > > if (page_is_mergeable(bv, page, len, off, same_page)) { > - if (bio->bi_iter.bi_size > UINT_MAX - len) > + if (bio->bi_iter.bi_size > UINT_MAX - len) { > + *same_page = false; > return false; > + } > bv->bv_len += len; > bio->bi_iter.bi_size += len; > return true; >
Ya, we had think of that. But what we then thought was, maybe the API does return the right thing. Meaning, what API says is, same_page is true, but the page couldn't be merged hence it returned ret = false. With that thought, we fixed this in the caller.
But agree with you that with ret = false, there is no meaning of same_page being true. Ok, so let linux-block comment on whether above also looks good. If yes, I can spin out an official patch with all details.
-ritesh
| |