Messages in this thread | | | Date | Fri, 31 Jan 2020 01:44:18 -0800 | From | Matthew Wilcox <> | Subject | Re: [PATCH 12/12] iomap: Convert from readpages to readahead |
| |
On Wed, Jan 29, 2020 at 12:38:39PM +1100, Dave Chinner wrote: > On Fri, Jan 24, 2020 at 05:35:53PM -0800, Matthew Wilcox wrote: > > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > Use the new readahead operation in XFS and iomap. > > + if (ctx.cur_page && ctx.cur_page_in_bio) > > put_page(ctx.cur_page); > > - } > > > > - /* > > - * Check that we didn't lose a page due to the arcance calling > > - * conventions.. > > - */ > > - WARN_ON_ONCE(!ret && !list_empty(ctx.pages)); > > - return ret; > > + return length / PAGE_SIZE; > > Took me quite some time to get my head around whether this was > correct or not.
Yes. Unfortunately, this is the most complex of the conversions ;-(
> I'm still not certain in the cases where block size != page size and > we've got an extent boundary in the middle of the page and had a > read error on the second extent in the page. In this case, > ctx.cur_page_in_bio is true so we drop the readahead reference to > the page. Also, length is not a multiple of page size, and so the > nr_pages value returned includes the partial page that we have IO > underway on. > > That, I think, leads to both a double unlock and a double put_page() > of the partial page in question.
But C division rounds down. So we neither unlock, nor put_page() the page which was in the bio ... do we?
| |