Re: [PATCH 10/21] xfs: remove xfs_map_cow

[Christoph Hellwig <hch@lst.de>]

On Mon, Jun 04, 2018 at 08:25:33AM -0400, Brian Foster wrote:
> > +		if (offset > i_size_read(inode)) {
> > +			whichfork = XFS_IO_HOLE;
> 
> Not sure what the point of this assignment is if we're going to return,
> but that's not a valid fork either way. Did you mean to assign
> wpc->io_type?

Yes, I did.  And this bug actually caused my unexplainable test
failures with 1k file systems..

> > +		/*
> > +		 * If we already have a valid COW mapping keep using it.
> > +		 */
> > +		if (wpc->io_type == XFS_IO_COW &&
> > +		    xfs_imap_valid(inode, &wpc->imap, offset)) {
> > +			wpc->imap_valid = true;
> > +			new_type = XFS_IO_COW;
> >  		}
> 
> So this lifts the cow I/O type check from xfs_map_cow(). That path would
> set imap_valid and new_type and return, which essentially looks like a
> "blocks found" case where we go ahead and add the buffer to the ioend.
> Now we set the same values, but we have to call xfs_map_blocks() to make
> sure we cover the cow blocks overlap case that xfs_map_cow() used to
> handle.
> 
> Shouldn't passing this check effectively jump straight to to the buffer
> add? Otherwise it looks like we unconditionally attempt to allocate
> blocks in the cow fork.
> 
> Alternatively...

It's all a bit of a mess.  I've now bitten the bullet and added a
modification counter to the ifork and use that instead of our
previous hacks.  Still work in progress, though.