Re: [PATCH v1 4/8] iomap: add writepages support for IOMAP_IN_MEM iomaps

[Christoph Hellwig <hch@infradead.org>]

On Mon, Jun 09, 2025 at 04:15:27PM -0700, Joanne Koong wrote:
> ioends are used in fuse for cleaning up state. fuse implements a
> ->submit_ioend() callback in fuse_iomap_submit_ioend() (in patch 7/8).

But do you use struct iomap_ioend at all?  (Sorry, still don't have a
whole tree with the patches applied in front of me).

> > Given that the patch that moved things around already wrapped the
> > error propagation to the bio into a helpr, how does this differ
> > from the main path in the function now?
> >
> 
> If we don't add this special casing for IOMAP_IN_MEM here, then in
> this function it'll hit the "if (!wpc->ioend)" case right in the
> beginning and return error without calling the ->submit_ioend()

So this suggests you don't use struct iomap_ioend at all.  Given that
you add a private field to iomap_writepage_ctx I guess that is where
you chain the fuse requests?

Either way I think we should clean this up one way or another.  If the
non-block iomap writeback code doesn't use ioends we should probably move
the ioend chain into the private field, and hide everything using it (or
even the ioend name) in proper abstraction.  In this case this means
finding another way to check for a non-empty wpc.  One way would be to
check nr_folios as any non-empty wbc must have a number of folios
attached to it, the other would be to move the check into the
->submit_ioend method including the block fallback.  For a proper split
the method should probably be renamed, and we'd probably want to require
every use to provide the submit method, even if the trivial block
users set it to the default one provided.

> > > -             if (!count)
> > > +             /*
> > > +              * If wpc->ops->writeback_folio is set, then it is responsible
> > > +              * for ending the writeback itself.
> > > +              */
> > > +             if (!count && !wpc->ops->writeback_folio)
> > >                       folio_end_writeback(folio);
> >
> > This fails to explain why writeback_folio does the unlocking itself.
> 
> writeback_folio needs to do the unlocking itself because the writeback
> may be done asynchronously (as in the case for fuse). I'll add that as
> a comment in v2.

So how do you end up with a zero count here and still want
the fuse code to unlock?