From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from verein.lst.de ([213.95.11.211]:54380 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726340AbeGKRiw (ORCPT ); Wed, 11 Jul 2018 13:38:52 -0400 Date: Wed, 11 Jul 2018 19:35:16 +0200 From: Christoph Hellwig Subject: Re: [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change Message-ID: <20180711173516.GA3961@lst.de> References: <20180710060528.4071-1-hch@lst.de> <20180710060528.4071-7-hch@lst.de> <20180711171551.GE32415@magnolia> <20180711172032.GA3738@lst.de> <20180711173152.GF32415@magnolia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180711173152.GF32415@magnolia> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: "Darrick J. Wong" Cc: Christoph Hellwig , linux-xfs@vger.kernel.org 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)