On Wed, Jun 24, 2020 at 08:14:31AM +1000, Dave Chinner wrote: > On Tue, Jun 23, 2020 at 02:19:10PM -0700, Darrick J. Wong wrote: > > On Tue, Jun 23, 2020 at 03:20:59PM +1000, Dave Chinner wrote: > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > The page faultround path ->map_pages is implemented in XFS via > > > > What does "faultround" mean? > > Typo - fault-around. > > i.e. when we take a read page fault, the do_read_fault() code first > opportunistically tries to map a range of pages surrounding > surrounding the faulted page into the PTEs, not just the faulted > page. It uses ->map_pages() to do the page cache lookups for > cached pages in the range of the fault around and then inserts them > into the PTES is if finds any. > > If the fault-around pass did not find the page fault page in cache > (i.e. vmf->page remains null) then it calls into do_fault(), which > ends up calling ->fault, which we then lock the MMAPLOCK and call > into filemap_fault() to populate the page cache and read the data > into it. > > So, essentially, fault-around is a mechanism to reduce page faults > in the situation where previous readahead has brought adjacent pages > into the page cache by optimistically mapping up to > fault_around_bytes into PTEs on any given read page fault. > > > I'm pretty convinced that this is merely another round of whackamole wrt > > taking the MMAPLOCK before relying on or doing anything to pages in the > > page cache, I just can't tell if 'faultround' is jargon or typo. > > Well, it's whack-a-mole in that this is the first time I've actually > looked at what map_pages does. I knew there was fault-around in the > page fault path, but I know that it had it's own method for > page cache lookups and pte mapping, nor that it had it's own > truncate race checks to ensure it didn't map pages invalidated by > truncate into the PTEs. <nod> Thanks for the explanation. /me wonders if someone could please check all the *_ops that point to generic helpers to see if we're missing obvious things like lock taking. Particularly someone who wants to learn about xfs' locking strategy; I promise it won't let out a ton of bees. Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > There's so much technical debt hidden in the kernel code base. The > fact we're still finding places that assume only truncate can > invalidate the page cache over a file range indicates just how deep > this debt runs... > > -Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
