Re: [PATCH v1 3/8] iomap: add buffered write support for IOMAP_IN_MEM iomaps

[Christoph Hellwig <hch@infradead.org>]

On Mon, Jun 09, 2025 at 03:45:05PM -0700, Joanne Koong wrote:
> > I just ran into this for another project and I hated my plumbing for
> > this.  I hate yours very slightly less but I still don't like it.
> >
> > This is really more of a VM level concept, so I  wonder if we should
> > instead:
> >
> >  - add a new read_folio_sync method to the address space operations that
> >    reads a folio without unlocking it.
> 
> imo I hate this more. AFAIU, basically fuse will be the only one
> actually needing/using this?

No, not at all.  We have a few projects that needs a submit_io hook
for iomap buffered reads - on is the btrfs conversion for their checksum
and raid, the other is my series to support T10 PI (and maybe checksums
in non-PI metadata as a follow on).  For ->read_folio and ->readahead
we just has patches from Goldwyn and me to pass a new ops vector for
that.  But I found it really ugly to do this for the write path, even
if it works (my current version).  Also this path is the only reason
why the srcmap in the iomap_iter exists - so that file systems that
do out of place writes can read partial folios from one place, but
return a different mapping to write it back to.  If we split it into
a separate aops we could kill all that, something I've been wanting
to do for a long time.

> Though it's a more intensive change, what about just expanding the
> existing address space operations ->read_folio() callback to take in
> an offset, length, and a boolean for whether the folio should be
> unlocked after the read?

The boolean for locking behavior is not a pattern liked much in
the kernel, for good reason - it makes verification really hard.
Now maybe always unlocking the folio in the caller might make sense,
but I'll need to consult at least willy on that before looking into
that.