Re: [PATCH v1 5/8] iomap: add iomap_writeback_dirty_folio()

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2025-06-11 at 05:34 +0100, Matthew Wilcox wrote:
> On Mon, Jun 09, 2025 at 08:59:53PM -0700, Christoph Hellwig wrote:
> > On Mon, Jun 09, 2025 at 10:14:44AM -0700, Darrick J. Wong wrote:
> > > > Where "folio laundering" means calling ->launder_folio, right?
> > > 
> > > What does fuse use folio laundering for, anyway?  It looks to me like
> > > the primary users are invalidate_inode_pages*.  Either the caller cares
> > > about flushing dirty data and has called filemap_write_and_wait_range;
> > > or it doesn't and wants to tear down the pagecache ahead of some other
> > > operation that's going to change the file contents and doesn't care.
> > > 
> > > I suppose it could be useful as a last-chance operation on a dirty folio
> > > that was dirtied after a filemap_write_and_wait_range but before
> > > invalidate_inode_pages*?  Though for xfs we just return EBUSY and let
> > > the caller try again (or not).  Is there a subtlety to fuse here that I
> > > don't know about?
> > 
> > My memory might be betraying me, but I think willy once launched an
> > attempt to see if we can kill launder_folio.  Adding him, and the
> > mm and nfs lists to check if I have a point :)
> 
> I ... got distracted with everything else.
> 
> Looking at the original addition of ->launder_page (e3db7691e9f3), I
> don't understand why we need it.  invalidate_inode_pages2() isn't
> supposed to invalidate dirty pages, so I don't understand why nfs
> found it necessary to do writeback from ->releasepage() instead
> of just returning false like iomap does.
> 
> There's now a new question of what the hell btrfs is up to with
> ->launder_folio, which they just added recently.

IIRC...

The problem was a race where a task could could dirty a page in a
mmap'ed file after it had been written back but before it was unmapped
from the pagecache.

Bear in mind that the NFS client may need write back and then
invalidate the pagecache for a file that is still in use if it
discovers that the inode's attributes have changed on the server.

Trond's solution was to write the page out while holding the page lock
in this situation. I think we'd all welcome a way to avoid this race
that didn't require launder_folio().
-- 
Jeff Layton <jlayton@kernel.org>