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@xxxxxxxxxxxxx> > > 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?
