On Tue, Jun 10, 2025 at 11:00 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote: > > On Tue, Jun 10, 2025 at 9:04 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > > > On Tue, Jun 10, 2025 at 01:13:09PM -0700, Joanne Koong wrote: > > > > > 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. > > > Okay thanks, I think I'll need to add this in for fuse then. I'll look > at this some more I read some of the thread in [1] and I don't think fuse needs this after all. The iomap mapping won't be changing state and concurrent writes are already protected by the file lock (if we don't use the plain ->read_folio variant). [1] https://lore.kernel.org/linux-xfs/20220817093627.GZ3600936@xxxxxxxxxxxxxxxxxxx/
