Re: [PATCH] [RFC] iomap: zeroing needs to be pagecache aware

[Dave Chinner <david@fromorbit.com>]

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@redhat.com>
> > 
> > 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@fromorbit.com