Re: [PATCH] mm, hugetlb: fix resv_huge_pages underflow on UFFDIO_COPY

From: Peter Xu <peterx@redhat.com>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Mina Almasry <almasrymina@google.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Linux-MM <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm, hugetlb: fix resv_huge_pages underflow on UFFDIO_COPY
Date: Fri, 14 May 2021 08:31:09 -0400	[thread overview]
Message-ID: <YJ5tjWKyVZk2mvxo@t490s> (raw)
In-Reply-To: <f9c85756-62e1-3d5c-9fbc-f38c6e8f07f3@oracle.com>

Hi, Mike,

On Thu, May 13, 2021 at 09:02:15PM -0700, Mike Kravetz wrote:

[...]

> I am also concerned with the semantics of this approach and what happens
> when a fault races with the userfaultfd copy.  Previously I asked Peter
> if we could/should use a page found in the cache for the copy.  His
> answer was as follows:
> 
>  AFAICT that's the expected behavior, and it need to be like that so as to avoid
>  silent data corruption (if the page cache existed, it means the page is not
>  "missing" at all, then it does not suite for a UFFDIO_COPY as it's only used
>  for uffd page missing case).

I didn't follow the rest discussion in depth yet... but just to mention that
the above answer was for the question whether we can "update the page in the
page cache", rather than "use a page found in the page cache".

I think reuse the page should be fine, however it'll definitely break existing
user interface (as it'll expect -EEXIST for now - we have kselftest covers
that), meanwhile I don't see why the -EEXIST bothers a lot: it still tells the
user that this page was filled in already.  Normally it was filled in by
another UFFDIO_COPY (as we could have multiple uffd service threads) along with
a valid pte, then this userspace thread can simply skip this message as it
means the event has been handled by some other servicing thread.

(This also reminded me that there won't be a chance of UFFDIO_COPY race on page
 no page fault at least, since no page fault will always go into the uffd
 missing handling rather than filling in the page cache for a VM_UFFD_MISSING
 vma; while mmap read lock should guarantee VM_UFFD_MISSING be persistent)

Thanks,

-- 
Peter Xu