On Wed, Nov 30, 2022 at 06:08:04PM -0800, Darrick J. Wong wrote: > On Thu, Dec 01, 2022 at 11:52:14AM +1100, Dave Chinner wrote: > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > Unwritten extents can have page cache data over the range being > > zeroed so we can't just skip them entirely. Fix this by checking for > > an existing dirty folio over the unwritten range we are zeroing > > and only performing zeroing if the folio is already dirty. > > Hm, I'll look at this tomorrow morning when I'm less bleary. From a > cursory glance it looks ok though. > > > XXX: how do we detect a iomap containing a cow mapping over a hole > > in iomap_zero_iter()? The XFS code implies this case also needs to > > zero the page cache if there is data present, so trigger for page > > cache lookup only in iomap_zero_iter() needs to handle this case as > > well. > > I've been wondering for a while if we ought to rename iomap_iter.iomap > to write_iomap and iomap_iter.srcmap to read_iomap, and change all the > ->iomap_begin and ->iomap_end functions as needed. I think that would > make it more clear to iomap users which one they're supposed to use. > Right now we overload iomap_iter.iomap for reads and for writes if > srcmap is a hole (or SHARED isn't set on iomap) and it's getting > confusing to keep track of all that. *nod* We definitely need to clarify this - I find the overloading confusing at the best of times. No idea what the solution to this looks like, though... > I guess the hard part of all that is that writes to the pagecache don't > touch storage; and writeback doesn't care about the source mapping since > it's only using block granularity. Yup, that's why this code needs the IOMAP_F_STALE code to be in place before we can use the page cache lookups like this. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx