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

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jun 25, 2025 at 09:44:31AM -0700, Joanne Koong wrote:
> On Tue, Jun 24, 2025 at 11:26 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Tue, Jun 24, 2025 at 10:26:01PM -0700, Joanne Koong wrote:
> > > > The question is whether this is acceptable for all the filesystem
> > > > which implement ->launder_folio today.  Because we could just move the
> > > > folio_test_dirty() to after the folio_lock() and remove all the testing
> > > > of folio dirtiness from individual filesystems.
> > >
> > > Or could the filesystems that implement ->launder_folio (from what I
> > > see, there's only 4: fuse, nfs, btrfs, and orangefs) just move that
> > > logic into their .release_folio implementation? I don't see why not.
> > > In folio_unmap_invalidate(), we call:
> >
> > Without even looking into the details from the iomap POV that basically
> > doesn't matter.  You'd still need the write back a single locked folio
> > interface, which adds API surface, and because it only writes a single
> > folio at a time is rather inefficient.  Not a deal breaker because
> > the current version look ok, but it would still be preferable to not
> > have an extra magic interface for it.
> >
> 
> Yes but as I understand it, the focus right now is on getting rid of
> ->launder_folio as an API. The iomap pov imo is a separate issue with
> determining whether fuse in particular needs to write back the dirty
> page before releasing or should just fail.

This might not help for Joanne's case, but so far the lack of a
launder_folio in my fuse+iomap prototype hasn't hindered it at all.
From what I can tell it's ok to bounce EBUSY back to dio callers...

> btrfs uses ->launder_folio() to free some previously allocated
> reservation (added in commit 872617a "btrfs: implement launder_folio
> for clearing dirty page reserve") so at the very least, that logic
> would need to be moved to .release_folio() (if that suffices? Adding
> the btrfs group to cc). It's still vague to me whether
> fuse/nfs/orangefs need to write back the dirty page, but it seems fine

...but only because a retry will initiate another writeback so
eventually we can make some forward progress.  But it helps a lot that
fuse+iomap is handing the entire IO stack over to iomap.

> to me not to - as I understand it, the worst that can happen (and
> please correct me if I'm wrong here, Matthew) from just failing it
> with -EBUSY is that the folio lingers longer in the page cache until
> it eventually gets written back and cleared out, and that only happens
> if the file is mapped and written to in that window between
> filemap_write_and_wait_range() and unmap_mapping_folio(). afaics, if
> fuse/nfs/orangefs do need to write back the dirty folio instead of
> failing w/ -EBUSY, they could just do that logic in .release_folio.

What do you do in ->release_folio if writeback fails?  Redirty it and
return false?

--D