On Fri, Sep 11, 2020 at 12:47:04AM +0100, Matthew Wilcox (Oracle) wrote: > Instead of counting bio segments, count the number of bytes submitted. > This insulates us from the block layer's definition of what a 'same page' > is, which is not necessarily clear once THPs are involved. > > Signed-off-by: Matthew Wilcox (Oracle) <[email protected]> > --- > fs/iomap/buffered-io.c | 41 ++++++++++++----------------------------- > 1 file changed, 12 insertions(+), 29 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 9670c096b83e..1cf976a8e55c 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -26,7 +26,7 @@ > * to track sub-page uptodate status and I/O completions. > */ > struct iomap_page { > - atomic_t read_count; > + atomic_t read_bytes_pending; > atomic_t write_count; > spinlock_t uptodate_lock; > unsigned long uptodate[]; > @@ -72,7 +72,7 @@ iomap_page_release(struct page *page) > > if (!iop) > return; > - WARN_ON_ONCE(atomic_read(&iop->read_count)); > + WARN_ON_ONCE(atomic_read(&iop->read_bytes_pending)); > WARN_ON_ONCE(atomic_read(&iop->write_count)); > WARN_ON_ONCE(bitmap_full(iop->uptodate, nr_blocks) != > PageUptodate(page)); > @@ -167,13 +167,6 @@ iomap_set_range_uptodate(struct page *page, unsigned > off, unsigned len) > SetPageUptodate(page); > } > > -static void > -iomap_read_finish(struct iomap_page *iop, struct page *page) > -{ > - if (!iop || atomic_dec_and_test(&iop->read_count)) > - unlock_page(page); > -} > - > static void > iomap_read_page_end_io(struct bio_vec *bvec, int error) > { > @@ -187,7 +180,8 @@ iomap_read_page_end_io(struct bio_vec *bvec, int error) > iomap_set_range_uptodate(page, bvec->bv_offset, bvec->bv_len); > } > > - iomap_read_finish(iop, page); > + if (!iop || atomic_sub_and_test(bvec->bv_len, &iop->read_bytes_pending)) > + unlock_page(page); > } > > static void > @@ -267,30 +261,19 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, > loff_t length, void *data, > } > > ctx->cur_page_in_bio = true; > + if (iop) > + atomic_add(plen, &iop->read_bytes_pending); > > - /* > - * Try to merge into a previous segment if we can. > - */ > + /* Try to merge into a previous segment if we can */ > sector = iomap_sector(iomap, pos); > - if (ctx->bio && bio_end_sector(ctx->bio) == sector) > + if (ctx->bio && bio_end_sector(ctx->bio) == sector) { > + if (__bio_try_merge_page(ctx->bio, page, plen, poff, > + &same_page)) > + goto done; > is_contig = true; > - > - if (is_contig && > - __bio_try_merge_page(ctx->bio, page, plen, poff, &same_page)) { > - if (!same_page && iop) > - atomic_inc(&iop->read_count); > - goto done; > } > > - /* > - * If we start a new segment we need to increase the read count, and we > - * need to do so before submitting any previous full bio to make sure > - * that we don't prematurely unlock the page. > - */ > - if (iop) > - atomic_inc(&iop->read_count); > - > - if (!ctx->bio || !is_contig || bio_full(ctx->bio, plen)) { > + if (!is_contig || bio_full(ctx->bio, plen)) { > gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL); > gfp_t orig_gfp = gfp; > int nr_vecs = (length + PAGE_SIZE - 1) >> PAGE_SHIFT; > -- > 2.28.0 >
