On Fri, Jan 24, 2020 at 05:35:48PM -0800, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <[email protected]> > > Use the new readahead operation in erofs. Fix what I believe to be > a refcounting bug in the error case. > > Signed-off-by: Matthew Wilcox (Oracle) <[email protected]> > Cc: [email protected] > --- > fs/erofs/data.c | 34 ++++++++++++++-------------------- > fs/erofs/zdata.c | 2 +- > include/trace/events/erofs.h | 6 +++--- > 3 files changed, 18 insertions(+), 24 deletions(-) > > diff --git a/fs/erofs/data.c b/fs/erofs/data.c > index fc3a8d8064f8..335c1ab05312 100644 > --- a/fs/erofs/data.c > +++ b/fs/erofs/data.c > @@ -280,42 +280,36 @@ static int erofs_raw_access_readpage(struct file *file, > struct page *page) > return 0; > } > > -static int erofs_raw_access_readpages(struct file *filp, > +static unsigned erofs_raw_access_readahead(struct file *file, > struct address_space *mapping, > - struct list_head *pages, > + pgoff_t start, > unsigned int nr_pages) > { > erofs_off_t last_block; > struct bio *bio = NULL; > - gfp_t gfp = readahead_gfp_mask(mapping); > - struct page *page = list_last_entry(pages, struct page, lru); > > - trace_erofs_readpages(mapping->host, page, nr_pages, true); > + trace_erofs_readpages(mapping->host, start, nr_pages, true); > > for (; nr_pages; --nr_pages) { > - page = list_entry(pages->prev, struct page, lru); > + struct page *page = readahead_page(mapping, start++); > > prefetchw(&page->flags); > - list_del(&page->lru); > > - if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) { > - bio = erofs_read_raw_page(bio, mapping, page, > - &last_block, nr_pages, true); > + bio = erofs_read_raw_page(bio, mapping, page, &last_block, > + nr_pages, true); > > - /* all the page errors are ignored when readahead */ > - if (IS_ERR(bio)) { > - pr_err("%s, readahead error at page %lu of nid > %llu\n", > - __func__, page->index, > - EROFS_I(mapping->host)->nid); > + /* all the page errors are ignored when readahead */ > + if (IS_ERR(bio)) { > + pr_err("%s, readahead error at page %lu of nid %llu\n", > + __func__, page->index, > + EROFS_I(mapping->host)->nid); > > - bio = NULL; > - } > + bio = NULL; > + put_page(page); > } > > - /* pages could still be locked */ > put_page(page);
A double put_page() on error? Cheers, Dave. -- Dave Chinner [email protected]
