On Tue, Jun 10, 2025 at 01:13:09PM -0700, Joanne Koong wrote: > > synchronous ones. And if the file system fragmented the folio so badly > > that we'll now need to do more than two reads we're still at least > > pipelining it, although that should basically never happen with modern > > file systems. > > If the filesystem wants granular folio reads, it can also just do that > itself by calling an iomap helper (eg what iomap_adjust_read_range() > is doing right now) in its ->read_folio() implementation, correct? Well, nothing tells ->read_folio how much to read. But having a new 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 :) > For fuse at least, we definitely want granular reads, since reads may > be extremely expensive (eg it may be a network fetch) and there's > non-trivial mempcy overhead incurred with fuse needing to memcpy read > buffer data from userspace back to the kernel. Ok, with that the plain ->read_folio variant is not going to fly. > > + folio_lock(folio); > > + if (unlikely(folio->mapping != inode->i_mapping)) > > + return 1; > > + if (unlikely(!iomap_validate(iter))) > > + return 1; > > Does this now basically mean that every caller that uses iomap for > writes will have to implement ->iomap_valid and up the sequence > counter anytime there's a write or truncate, in case the folio changes > during the lock drop? Or were we already supposed to be doing this? Not any more than before. It's is still option, but you still very much want it to protect against races updating the mapping.