Re: [PATCH] dax: dax_layout_busy_page() should not unmap cow pages

From: Dan Williams <dan.j.williams@intel.com>
To: Boaz Harrosh <boaz@plexistor.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	virtio-fs@redhat.com, linux-nvdimm <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH] dax: dax_layout_busy_page() should not unmap cow pages
Date: Mon, 5 Aug 2019 13:11:48 -0700	[thread overview]
Message-ID: <CAPcyv4jgxTNbGBy9cQjS_mEzeuaXa6PQSQ+C5xBgy0mpxUmNzg@mail.gmail.com> (raw)
In-Reply-To: <9c0ec951-01e7-7ae0-2d69-1b26f3450d65@plexistor.com>

On Mon, Aug 5, 2019 at 12:17 PM Boaz Harrosh <boaz@plexistor.com> wrote:
>
> On 05/08/2019 21:49, Vivek Goyal wrote:
> > On Mon, Aug 05, 2019 at 02:53:06PM +0300, Boaz Harrosh wrote:
> <>
> >> So as I understand the man page:
> >> fallocate(FL_PUNCH_HOLE); means user is asking to get rid also of COW pages.
> >> On the other way fallocate(FL_ZERO_RANGE) only the pmem portion is zeroed and COW (private pages) stays
> >
> > I tested fallocate(FL_PUNCH_HOLE) on xfs (non-dax) and it does not seem to
> > get rid of COW pages and my test case still can read the data it wrote
> > in private pages.
> >
>
> It seems you are right and I am wrong. This is what the Kernel code has to say about it:
>
>         /*
>          * Unlike in truncate_pagecache, unmap_mapping_range is called only
>          * once (before truncating pagecache), and without "even_cows" flag:
>          * hole-punching should not remove private COWed pages from the hole.
>          */
>
> For me this is confusing but that is what it is. So remove private COWed pages
> is only done when we do an setattr(ATTR_SIZE).
>
> >>
> >> Just saying I have not followed the above code path
> >> (We should have an xfstest for this?)
> >
> > I don't know either. It indeed is interesting to figure out what's the
> > expected behavior with fallocate() and truncate() for COW pages and cover
> > that using xfstest (if not already done).
> >
>
> I could not find any test for the COW positive FL_PUNCH_HOLE (I have that bug)
> could be nice to make one, and let FSs like mine fail.
> Any way very nice catch.
>

Yes, and this bug is worse because it affects COW pages that are not
the direct target of the truncate / hole punch. This unmap in
dax_layout_busy_page() is only there to allow the fs to synchronize
against get_user_pages_fast() which might otherwise race to grab a
page reference and prevent the fs from making forward progress. The
unmap_mapping_range() that addresses COW pages in the truncated range
occurs later after the filesystem has regained control of the extent
layout (i.e. break layouts has succeeded).
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm