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.
