On Thu, Aug 15, 2019 at 11:47:53PM -0700, Christoph Hellwig wrote:
> On Thu, Aug 15, 2019 at 11:18:04AM -0700, Matthew Wilcox wrote:
> > But I don't think read_mapping_page() can return a page which doesn't have
> > PageUptodate set. Follow the path down through read_cache_page() into
> > do_read_cache_page().
>
> read_mapping_page() can't, but I think we need the check after the
> lock_page still.
The current code checks before locking the page:
if (!PageUptodate(page)) {
put_page(page);
return ERR_PTR(-EIO);
}
lock_page(page);
so the race with clearing Uptodate already existed.
XFS can ClearPageUptodate if it gets an error while doing writeback of
a dirty page. But we shouldn't be able to get a dirty page. Before we
get to this point, we call filemap_write_and_wait_range() while holding
the inode's modification lock.
So I think we can't see a !Uptodate page, but of course I'd be happy to
see a sequence of events that breaks this reasoning.
