Re: [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change

[Christoph Hellwig <hch@lst.de>]

On Wed, Jul 11, 2018 at 10:31:52AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 11, 2018 at 07:20:32PM +0200, Christoph Hellwig wrote:
> > On Wed, Jul 11, 2018 at 10:15:51AM -0700, Darrick J. Wong wrote:
> > > Hmm, this troubles me slightly -- a short time ago you removed the "trim
> > > this data fork mapping to the first overlap with a cow fork mapping"
> > > (i.e. xfs_reflink_trim_irec_to_next_cow) behavior on the grounds that
> > > _writepage_map calls _map_blocks for every block in the page and so it
> > > was unnecessary.  But this seems to put that back.  Why is that?
> > 
> > Because back then map_blocks did a lookup in the COW fork everytime
> > (unless we are already on a COW mapping), and this patch wants to
> > only look it up when the COW fork actually changed, so the trim is
> > required now.
> 
> Ah, ok.   Mind if I change the comment to:
> 
> /*
>  * Truncate to the next COW extent if there is one.  This is the only
>  * opportunity to do this because we can skip COW fork lookups for the
>  * subsequent blocks in the mapping; however, the requirement to treat
>  * the COW range separately remains.
>  */
> 
> I also wonder why we don't need to do the same for holes, but I think
> the answer is that any dirty page with a cow fork mapping must also have
> a data fork mapping (even if it's merely a delalloc reservation) and so
> a hole will never overlap with a cow fork mapping.

I'll update the comment.  I'll need to resend against the 4.19-merge
tree anyway, and I also found a little buglet in this patch (it updates
wpc->cow_seq for all allocations and not just COW ones, with that fixed
we should do even less lookups)