Message-ID: <7eb2bce9-d0b1-a0e3-8be3-f28d858a61a0@redhat.com>
Date:   Thu, 16 Feb 2023 11:47:23 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Content-Language: en-US
To:     Peter Xu <peterx@redhat.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
Cc:     Axel Rasmussen <axelrasmussen@google.com>,
        Mike Rapoport <rppt@linux.vnet.ibm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Nadav Amit <nadav.amit@gmail.com>,
        Muhammad Usama Anjum <usama.anjum@collabora.com>
References: <20230215210257.224243-1-peterx@redhat.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
Subject: Re: [PATCH] mm/uffd: UFFD_FEATURE_WP_ZEROPAGE
In-Reply-To: <20230215210257.224243-1-peterx@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 15.02.23 22:02, Peter Xu wrote:
> This is a new feature that controls how uffd-wp handles zero pages (aka,
> empty ptes), majorly for anonymous pages only.
> 
> Note, here we used "zeropage" as a replacement of "empty pte" just to avoid
> introducing the pte idea into uapi, since "zero page" is more well known to
> an user app developer.
> 
> File memories handles none ptes consistently by allowing wr-protecting of
> none ptes because of the unawareness of page cache being exist or not.  For
> anonymous it was not as persistent because we used to assume that we don't
> need protections on none ptes or known zero pages.
> 
> But it's actually not true.
> 
> One use case was VM live snapshot, where if without wr-protecting empty
> ptes the snapshot can contain random rubbish in the holes of the anonymous
> memory, which can cause misbehave of the guest when the guest assumes the
> pages should (and were) all zeros.
> 
> QEMU worked it around by pre-populate the section with reads to fill in
> zero page entries before starting the whole snapshot process [1].
> 
> Recently there's another need that raised on using userfaultfd wr-protect
> for detecting dirty pages (to replace soft-dirty) [2].  In that case if
> without being able to wr-protect zero pages by default, the dirty info can
> get lost as long as a zero page is written, even after the tracking was
> started.
> 
> In general, we want to be able to wr-protect empty ptes too even for
> anonymous.
> 
> This patch implements UFFD_FEATURE_WP_ZEROPAGE so that it'll make uffd-wp
> handling on zeropage being consistent no matter what the memory type is
> underneath.  It doesn't have any impact on file memories so far because we
> already have pte markers taking care of that.  So it only affects
> anonymous.
> 
> One way to implement this is to also install pte markers for anonymous
> memories.  However here we can actually do better (than i.e. shmem) because
> we know there's no page that is backing the pte, so the better solution is
> to directly install a zeropage read-only pte, so that if there'll be a
> upcoming read it'll not trigger a fault at all.  It will also reduce the
> changeset to implement this feature too.
> 

There are various reasons why I think a UFFD_FEATURE_WP_UNPOPULATED, 
using PTE markers, would be more benficial:

1) It would be applicable to anon hugetlb
2) It would be applicable even when the zeropage is disallowed
    (mm_forbids_zeropage())
3) It would be possible to optimize even without the huge zeropage, by
    using a PMD marker.
4) It would be possible to optimize even on the PUD level using a PMD
    marker.

Especially when uffd-wp'ing large ranges that are possibly all 
unpopulated (thinking about the existing VM background snapshot use case 
either with untouched memory or with things like free page reporting), 
we might neither be reading or writing that memory any time soon.

-- 
Thanks,

David / dhildenb