Re: [PATCH v4] vfs: fix page locking deadlocks when deduping files

Matthew Wilcox
2019-08-16T16:57:14Z

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.