On Tue, Jun 10, 2025 at 11:00:51PM -0700, Joanne Koong wrote: > Not a great idea, but theoretically we could stash that info (offset > and len) in the folio->private iomap_folio_state struct. I don't think > that runs into synchronization issues since it would be set and > cleared while the file lock is held for that read. Yeah, I thought about it, but there's no simple hole, so we'd bloat a long living structure for short-term argument passing. So I've been thinking of maybe sticking to something similar to your version, but with a few tweaks. Let me cook up a patch for review. > But regardless I think we still need a new variant of read_folio > because if a non block-io iomap wants to use iomap_read_folio() / > iomap_readahead() for the granular uptodate parsing logic that's in > there, it'll need to provide a method for reading a partial folio. I > initially wasn't planning to have fuse use iomap_read_folio() / > iomap_readahead() but I realized there's some cases where fuse will > find it useful, so i'm planning to add that in. Heh. How much of the iomap code can you reuse vs just using another variant that shareds the uptodate/dirty bitmaps? > > variant of read_folio that allows partial reads might still be nicer > > than a iomap_folio_op. Let me draft that and see if willy or other mm > > folks choke on it :) > > writeback_folio() is also a VM level concept so under that same logic, > should writeback_folio() also be an address space operation? Not really. Yes, writing back a part of a folio is high level in concept, but you really need quite a lot of iomap speific in practive. > > A more general question i've been trying to figure out is if the > vision is that iomap is going to be the defacto generic library that > all/most filesystems will be using in the future? That's always been my plan. But it's not been overly successful so far. > If so then it makes > sense to me to add this to the address space operations but if not > then I don't think I see the hate for having the folio callbacks be > embedded in iomap_folio_op. I'd really avoid sticking something that is just a callback into the address_space operations. We've done that for write_begin/write_end and it has turned out to be a huge mess. Something in the aops must be callable standalone and not depend on a very specific caller context.