On Wed, Jun 11, 2025 at 11:33:40AM -0700, Joanne Koong wrote: > 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/ <nod> If the mapping types don't change between read/write (which take i_rwsem in exclusive mode) and writeback (which doesn't take it at all) then I don't think there's a need to revalidate the mapping after grabbing a folio. I think the other ways to avoid those races are (a) avoid unaligned zeroing if you can guarantee that the folios are always fully uptodate; and (b) don't do things that change the out-of-place write status of pagecache (e.g. reflink). --D
