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

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

On Sun, Jun 08, 2025 at 09:51:54PM -0700, Christoph Hellwig wrote:
> On Fri, Jun 06, 2025 at 04:38:00PM -0700, Joanne Koong wrote:
> > Add iomap_writeback_dirty_folio() for writing back a dirty folio.
> > One use case of this is for folio laundering.
> 
> 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?

(Both of those questions are directed at hch or joanne or anyone else
who knows ;))

--D

> > @@ -1675,7 +1677,8 @@ static int iomap_writepage_map(struct iomap_writepage_ctx *wpc,
> >  	 * already at this point.  In that case we need to clear the writeback
> >  	 * bit ourselves right after unlocking the page.
> >  	 */
> > -	folio_unlock(folio);
> > +	if (unlock_folio)
> > +		folio_unlock(folio);
> >  	if (ifs) {
> >  		if (atomic_dec_and_test(&ifs->write_bytes_pending))
> >  			folio_end_writeback(folio);
> 
> When writing this code I was under the impression that
> folio_end_writeback needs to be called after unlocking the page.
> 
> If that is not actually the case we can just move the unlocking into the
> caller and make things a lot cleaner than the conditional locking
> argument.
> 
> > +int iomap_writeback_dirty_folio(struct folio *folio, struct writeback_control *wbc,
> > +				struct iomap_writepage_ctx *wpc,
> > +				const struct iomap_writeback_ops *ops)
> 
> Please stick to the usual iomap coding style:  80 character lines,
> two-tab indent for multiline function declarations.  (Also in a few
> other places).
> 
>