Re: [PATCH 1/3] xfs: Remove xfs_filemap_map_pages() wrapper

[Matthew Wilcox <willy@infradead.org>]

On Thu, Feb 09, 2023 at 08:53:11AM +1100, Dave Chinner wrote:
> > If XFS really needs it,
> > it can trylock the semaphore and return 0 if it fails, falling back to
> > the ->fault path.  But I don't think XFS actually needs it.
> >
> > The ->map_pages path trylocks the folio, checks the folio->mapping,
> > checks uptodate, then checks beyond EOF (not relevant to hole punch).
> > Then it takes the page table lock and puts the page(s) into the page
> > tables, unlocks the folio and moves on to the next folio.
> > 
> > The hole-punch path, like the truncate path, takes the folio lock,
> > unmaps the folio (which will take the page table lock) and removes
> > it from the page cache.
> > 
> > So what's the race?
> 
> Hole punch is a multi-folio operation, so while we are operating on
> invalidating one folio, another folio in the range we've already
> invalidated could be instantiated and mapped, leaving mapped
> up-to-date pages over a range we *require* the page cache to empty.

Nope.  ->map_pages is defined to _not_ instantiate new pages.
If there are uptodate pages in the page cache, they can be mapped, but
missing pages will be skipped, and left to ->fault to bring in.