[v2] mm/uffd: UFFD_FEATURE_WP_UNPOPULATED

  This is a new feature that controls how uffd-wp handles none ptes.  When
it's set, the kernel will handle anonymous memory the same way as file
memory, by allowing the user to wr-protect unpopulated ptes.

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.

One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
case if without being able to wr-protect none ptes by default, the dirty
info can get lost, since we cannot treat every none pte to be dirty (the
current design is identify a page dirty based on uffd-wp bit being cleared).

In general, we want to be able to wr-protect empty ptes too even for
anonymous.

This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
uffd-wp handling on none ptes 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.

The feature bit is by default off, so the old behavior will be maintained.
Sometimes it may be wanted because the wr-protect of none ptes will contain
overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
anonymous), but also on creating the pgtables to store the pte markers. So
there's potentially less chance of using thp on the first fault for a none
pmd or larger than a pmd.

The major implementation part is teaching the whole kernel to understand
pte markers even for anonymously mapped ranges, meanwhile allowing the
UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
new feature bit is set.

Note that even if the patch subject starts with mm/uffd, there're a few
small refactors to major mm path of handling anonymous page faults. But
they should be straightforward.

So far, add a very light smoke test within the userfaultfd kselftest
pagemap unit test to make sure anon pte markers work.

[1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
[1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/

Signed-off-by: Peter Xu <peterx@redhat.com>
---
v1->v2:
- Use pte markers rather than populate zero pages when protect [David]
- Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
---
 fs/userfaultfd.c                         | 14 ++++++
 include/linux/mm_inline.h                |  6 +++
 include/linux/userfaultfd_k.h            |  6 +++
 include/uapi/linux/userfaultfd.h         | 10 +++-
 mm/memory.c                              | 54 ++++++++++++++++------
 mm/mprotect.c                            | 59 ++++++++++++++++++++----
 tools/testing/selftests/mm/userfaultfd.c | 22 ++++++++-
 7 files changed, 146 insertions(+), 25 deletions(-)
  

On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
> This is a new feature that controls how uffd-wp handles none ptes.  When
> it's set, the kernel will handle anonymous memory the same way as file
> memory, by allowing the user to wr-protect unpopulated ptes.
> 
> 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.
> 
> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
> case if without being able to wr-protect none ptes by default, the dirty
> info can get lost, since we cannot treat every none pte to be dirty (the
> current design is identify a page dirty based on uffd-wp bit being cleared).
> 
> In general, we want to be able to wr-protect empty ptes too even for
> anonymous.
> 
> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
> uffd-wp handling on none ptes 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.
> 
> The feature bit is by default off, so the old behavior will be maintained.
> Sometimes it may be wanted because the wr-protect of none ptes will contain
> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
> anonymous), but also on creating the pgtables to store the pte markers. So
> there's potentially less chance of using thp on the first fault for a none
> pmd or larger than a pmd.
> 
> The major implementation part is teaching the whole kernel to understand
> pte markers even for anonymously mapped ranges, meanwhile allowing the
> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
> new feature bit is set.
> 
> Note that even if the patch subject starts with mm/uffd, there're a few
> small refactors to major mm path of handling anonymous page faults. But
> they should be straightforward.
> 
> So far, add a very light smoke test within the userfaultfd kselftest
> pagemap unit test to make sure anon pte markers work.
> 
> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> v1->v2:
> - Use pte markers rather than populate zero pages when protect [David]
> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]

Some very initial performance numbers (I only ran in a VM but it should be
similar, unit is "us") below as requested.  The measurement is about time
spent when wr-protecting 10G range of empty but mapped memory.  It's done
in a VM, assuming we'll get similar results on bare metal.

Four test cases:

        - default UFFDIO_WP
        - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
        - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
        - UFFDIO_WP with WP_UNPOPULATED

Results:

        Test DEFAULT: 2
        Test PRE-READ: 3277099 (pre-fault 3253826)
        Test MADVISE: 2250361 (pre-fault 2226310)
        Test WP-UNPOPULATE: 20850

I'll add these information into the commit message when there's a new
version.

[1] https://github.com/xzpeter/clibs/blob/master/uffd-test/uffd-wp-perf.c
  

Hi Peter,

Thank you so much for sending.

On 2/28/23 5:36 AM, Peter Xu wrote:
> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>> This is a new feature that controls how uffd-wp handles none ptes.  When
>> it's set, the kernel will handle anonymous memory the same way as file
>> memory, by allowing the user to wr-protect unpopulated ptes.
>>
>> 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.
>>
>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>> case if without being able to wr-protect none ptes by default, the dirty
>> info can get lost, since we cannot treat every none pte to be dirty (the
>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>
>> In general, we want to be able to wr-protect empty ptes too even for
>> anonymous.
>>
>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>> uffd-wp handling on none ptes 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.
>>
>> The feature bit is by default off, so the old behavior will be maintained.
>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>> anonymous), but also on creating the pgtables to store the pte markers. So
>> there's potentially less chance of using thp on the first fault for a none
>> pmd or larger than a pmd.
>>
>> The major implementation part is teaching the whole kernel to understand
>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>> new feature bit is set.
>>
>> Note that even if the patch subject starts with mm/uffd, there're a few
>> small refactors to major mm path of handling anonymous page faults. But
>> they should be straightforward.
>>
>> So far, add a very light smoke test within the userfaultfd kselftest
>> pagemap unit test to make sure anon pte markers work.
>>
>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> ---
>> v1->v2:
>> - Use pte markers rather than populate zero pages when protect [David]
>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
> 
> Some very initial performance numbers (I only ran in a VM but it should be
> similar, unit is "us") below as requested.  The measurement is about time
> spent when wr-protecting 10G range of empty but mapped memory.  It's done
> in a VM, assuming we'll get similar results on bare metal.
> 
> Four test cases:
> 
>         - default UFFDIO_WP
>         - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>         - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>         - UFFDIO_WP with WP_UNPOPULATED
> 
> Results:
> 
>         Test DEFAULT: 2
>         Test PRE-READ: 3277099 (pre-fault 3253826)
>         Test MADVISE: 2250361 (pre-fault 2226310)
>         Test WP-UNPOPULATE: 20850
> 
> I'll add these information into the commit message when there's a new
> version.
I'm hitting a bug where I'm unable to write to the memory after adding this
patch and wp the memory. I'm hitting this case in your test and my tests as
well. Please apply the following diff to your test to reproduce on your end:

--- uffd_wp_perf.c.orig 2023-02-28 12:09:38.971820791 +0500
+++ uffd_wp_perf.c      2023-02-28 12:13:11.077827160 +0500
@@ -114,6 +114,7 @@
         start1 = get_usec();
     }
     wp_range(uffd, buffer, SIZE, true);
+    buffer[0] = 'a';
     if (start1 == 0)
          printf("%"PRIu64"\n", get_usec() - start);
     else


> 
> [1] https://github.com/xzpeter/clibs/blob/master/uffd-test/uffd-wp-perf.c
>
  

On Tue, Feb 28, 2023 at 12:21:45PM +0500, Muhammad Usama Anjum wrote:
> Hi Peter,

Hi, Muhammad,

> 
> Thank you so much for sending.
> 
> On 2/28/23 5:36 AM, Peter Xu wrote:
> > On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
> >> This is a new feature that controls how uffd-wp handles none ptes.  When
> >> it's set, the kernel will handle anonymous memory the same way as file
> >> memory, by allowing the user to wr-protect unpopulated ptes.
> >>
> >> 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.
> >>
> >> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
> >> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
> >> case if without being able to wr-protect none ptes by default, the dirty
> >> info can get lost, since we cannot treat every none pte to be dirty (the
> >> current design is identify a page dirty based on uffd-wp bit being cleared).
> >>
> >> In general, we want to be able to wr-protect empty ptes too even for
> >> anonymous.
> >>
> >> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
> >> uffd-wp handling on none ptes 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.
> >>
> >> The feature bit is by default off, so the old behavior will be maintained.
> >> Sometimes it may be wanted because the wr-protect of none ptes will contain
> >> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
> >> anonymous), but also on creating the pgtables to store the pte markers. So
> >> there's potentially less chance of using thp on the first fault for a none
> >> pmd or larger than a pmd.
> >>
> >> The major implementation part is teaching the whole kernel to understand
> >> pte markers even for anonymously mapped ranges, meanwhile allowing the
> >> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
> >> new feature bit is set.
> >>
> >> Note that even if the patch subject starts with mm/uffd, there're a few
> >> small refactors to major mm path of handling anonymous page faults. But
> >> they should be straightforward.
> >>
> >> So far, add a very light smoke test within the userfaultfd kselftest
> >> pagemap unit test to make sure anon pte markers work.
> >>
> >> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
> >> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
> >>
> >> Signed-off-by: Peter Xu <peterx@redhat.com>
> >> ---
> >> v1->v2:
> >> - Use pte markers rather than populate zero pages when protect [David]
> >> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
> > 
> > Some very initial performance numbers (I only ran in a VM but it should be
> > similar, unit is "us") below as requested.  The measurement is about time
> > spent when wr-protecting 10G range of empty but mapped memory.  It's done
> > in a VM, assuming we'll get similar results on bare metal.
> > 
> > Four test cases:
> > 
> >         - default UFFDIO_WP
> >         - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
> >         - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
> >         - UFFDIO_WP with WP_UNPOPULATED
> > 
> > Results:
> > 
> >         Test DEFAULT: 2
> >         Test PRE-READ: 3277099 (pre-fault 3253826)
> >         Test MADVISE: 2250361 (pre-fault 2226310)
> >         Test WP-UNPOPULATE: 20850
> > 
> > I'll add these information into the commit message when there's a new
> > version.
> I'm hitting a bug where I'm unable to write to the memory after adding this
> patch and wp the memory. I'm hitting this case in your test and my tests as
> well. Please apply the following diff to your test to reproduce on your end:
> 
> --- uffd_wp_perf.c.orig 2023-02-28 12:09:38.971820791 +0500
> +++ uffd_wp_perf.c      2023-02-28 12:13:11.077827160 +0500
> @@ -114,6 +114,7 @@
>          start1 = get_usec();
>      }
>      wp_range(uffd, buffer, SIZE, true);
> +    buffer[0] = 'a';
>      if (start1 == 0)
>           printf("%"PRIu64"\n", get_usec() - start);
>      else

This is expected, because the test didn't start any fault resolving thread,
so the write will block until someone unprotects the page.

But it shouldn't happen to your use case if you applied both WP_UNPOPULATED
& WP_ASYNC.

Could you try "cat /proc/$PID/stack" to see where does your thread stuck
at?
  

On 2/28/23 8:58 PM, Peter Xu wrote:
> On Tue, Feb 28, 2023 at 12:21:45PM +0500, Muhammad Usama Anjum wrote:
>> Hi Peter,
> 
> Hi, Muhammad,
> 
>>
>> Thank you so much for sending.
>>
>> On 2/28/23 5:36 AM, Peter Xu wrote:
>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>>>> This is a new feature that controls how uffd-wp handles none ptes.  When
>>>> it's set, the kernel will handle anonymous memory the same way as file
>>>> memory, by allowing the user to wr-protect unpopulated ptes.
>>>>
>>>> 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.
>>>>
>>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>>>> case if without being able to wr-protect none ptes by default, the dirty
>>>> info can get lost, since we cannot treat every none pte to be dirty (the
>>>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>>>
>>>> In general, we want to be able to wr-protect empty ptes too even for
>>>> anonymous.
>>>>
>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>>>> uffd-wp handling on none ptes 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.
>>>>
>>>> The feature bit is by default off, so the old behavior will be maintained.
>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>>>> anonymous), but also on creating the pgtables to store the pte markers. So
>>>> there's potentially less chance of using thp on the first fault for a none
>>>> pmd or larger than a pmd.
>>>>
>>>> The major implementation part is teaching the whole kernel to understand
>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>>>> new feature bit is set.
>>>>
>>>> Note that even if the patch subject starts with mm/uffd, there're a few
>>>> small refactors to major mm path of handling anonymous page faults. But
>>>> they should be straightforward.
>>>>
>>>> So far, add a very light smoke test within the userfaultfd kselftest
>>>> pagemap unit test to make sure anon pte markers work.
>>>>
>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>>>
>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>> ---
>>>> v1->v2:
>>>> - Use pte markers rather than populate zero pages when protect [David]
>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
>>>
>>> Some very initial performance numbers (I only ran in a VM but it should be
>>> similar, unit is "us") below as requested.  The measurement is about time
>>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
>>> in a VM, assuming we'll get similar results on bare metal.
>>>
>>> Four test cases:
>>>
>>>         - default UFFDIO_WP
>>>         - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>>>         - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>>>         - UFFDIO_WP with WP_UNPOPULATED
>>>
>>> Results:
>>>
>>>         Test DEFAULT: 2
>>>         Test PRE-READ: 3277099 (pre-fault 3253826)
>>>         Test MADVISE: 2250361 (pre-fault 2226310)
>>>         Test WP-UNPOPULATE: 20850
>>>
>>> I'll add these information into the commit message when there's a new
>>> version.
>> I'm hitting a bug where I'm unable to write to the memory after adding this
>> patch and wp the memory. I'm hitting this case in your test and my tests as
>> well. Please apply the following diff to your test to reproduce on your end:
>>
>> --- uffd_wp_perf.c.orig 2023-02-28 12:09:38.971820791 +0500
>> +++ uffd_wp_perf.c      2023-02-28 12:13:11.077827160 +0500
>> @@ -114,6 +114,7 @@
>>          start1 = get_usec();
>>      }
>>      wp_range(uffd, buffer, SIZE, true);
>> +    buffer[0] = 'a';
>>      if (start1 == 0)
>>           printf("%"PRIu64"\n", get_usec() - start);
>>      else
> 
> This is expected, because the test didn't start any fault resolving thread,
> so the write will block until someone unprotects the page.
Ohh.. sorry. Wrong reproducer.

> 
> But it shouldn't happen to your use case if you applied both WP_UNPOPULATED
> & WP_ASYNC.
I'm using both WP_UNPOPULATED and ASYNC. The program gets stuck at right time:


1..57
ok 1 sanity_tests_sd wrong flag specified
ok 2 sanity_tests_sd wrong mask specified
ok 3 sanity_tests_sd wrong return mask specified
ok 4 sanity_tests_sd mixture of correct and wrong flag
ok 5 sanity_tests_sd PM_SCAN_OP_WP cannot be used without get
ok 6 sanity_tests_sd Clear area with larger vec size
^C
Program received signal SIGINT, Interrupt.
0x000000000040220c in sanity_tests_sd () at pagemap_ioctl.c:198
198                     mem[i]++;
(gdb) bt
#0  0x000000000040220c in sanity_tests_sd () at pagemap_ioctl.c:198
#1  0x0000000000404e14 in main () at pagemap_ioctl.c:846
()

/proc/$PID/stack is empty. Not sure why. I can see stack trace of other
applications, but not this one's.

Let me send better reproducer for you.

> 
> Could you try "cat /proc/$PID/stack" to see where does your thread stuck
> at?
>
  

Hi Peter,

Finally I've found the bug.

On 2/28/23 9:24 PM, Muhammad Usama Anjum wrote:
> On 2/28/23 8:58 PM, Peter Xu wrote:
>> On Tue, Feb 28, 2023 at 12:21:45PM +0500, Muhammad Usama Anjum wrote:
>>> Hi Peter,
>>
>> Hi, Muhammad,
>>
>>>
>>> Thank you so much for sending.
>>>
>>> On 2/28/23 5:36 AM, Peter Xu wrote:
>>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>>>>> This is a new feature that controls how uffd-wp handles none ptes.  When
>>>>> it's set, the kernel will handle anonymous memory the same way as file
>>>>> memory, by allowing the user to wr-protect unpopulated ptes.
>>>>>
>>>>> 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.
>>>>>
>>>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>>>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>>>>> case if without being able to wr-protect none ptes by default, the dirty
>>>>> info can get lost, since we cannot treat every none pte to be dirty (the
>>>>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>>>>
>>>>> In general, we want to be able to wr-protect empty ptes too even for
>>>>> anonymous.
>>>>>
>>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>>>>> uffd-wp handling on none ptes 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.
>>>>>
>>>>> The feature bit is by default off, so the old behavior will be maintained.
>>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>>>>> anonymous), but also on creating the pgtables to store the pte markers. So
>>>>> there's potentially less chance of using thp on the first fault for a none
>>>>> pmd or larger than a pmd.
>>>>>
>>>>> The major implementation part is teaching the whole kernel to understand
>>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>>>>> new feature bit is set.
>>>>>
>>>>> Note that even if the patch subject starts with mm/uffd, there're a few
>>>>> small refactors to major mm path of handling anonymous page faults. But
>>>>> they should be straightforward.
>>>>>
>>>>> So far, add a very light smoke test within the userfaultfd kselftest
>>>>> pagemap unit test to make sure anon pte markers work.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>>>>
>>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>>> ---
>>>>> v1->v2:
>>>>> - Use pte markers rather than populate zero pages when protect [David]
>>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
>>>>
>>>> Some very initial performance numbers (I only ran in a VM but it should be
>>>> similar, unit is "us") below as requested.  The measurement is about time
>>>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
>>>> in a VM, assuming we'll get similar results on bare metal.
>>>>
>>>> Four test cases:
>>>>
>>>>         - default UFFDIO_WP
>>>>         - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>>>>         - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>>>>         - UFFDIO_WP with WP_UNPOPULATED
>>>>
>>>> Results:
>>>>
>>>>         Test DEFAULT: 2
>>>>         Test PRE-READ: 3277099 (pre-fault 3253826)
>>>>         Test MADVISE: 2250361 (pre-fault 2226310)
>>>>         Test WP-UNPOPULATE: 20850
>>>>
>>>> I'll add these information into the commit message when there's a new
>>>> version.
>>> I'm hitting a bug where I'm unable to write to the memory after adding this
>>> patch and wp the memory. I'm hitting this case in your test and my tests as
>>> well. Please apply the following diff to your test to reproduce on your end:
>>>
>>> --- uffd_wp_perf.c.orig 2023-02-28 12:09:38.971820791 +0500
>>> +++ uffd_wp_perf.c      2023-02-28 12:13:11.077827160 +0500
>>> @@ -114,6 +114,7 @@
>>>          start1 = get_usec();
>>>      }
>>>      wp_range(uffd, buffer, SIZE, true);
>>> +    buffer[0] = 'a';
While using WP_UNPOPULATED, we get stuck if newly allocated memory is read
without initialization. This can be reproduced by either of the following
statements:
    printf("%c", buffer[0]);
    buffer[0]++;

This bug has start to appear on this patch. How are you handling reading
newly allocated memory when WP_UNPOPULATED is defined?



Running my pagemap_ioctl selftest as benchmark in a VM:
without zeropage / wp_unpopulated (decide from pte_none() if page is dirty
or not, buggy and wrong implementation, just for reference)
26.608 seconds
with zeropage
39.203 seconds
with wp_unpopulated
62.907 seconds

136% worse performance overall
60% worse performance of unpopulated than zeropage

>>>      if (start1 == 0)
>>>           printf("%"PRIu64"\n", get_usec() - start);
>>>      else
>>
>> This is expected, because the test didn't start any fault resolving thread,
>> so the write will block until someone unprotects the page.
> Ohh.. sorry. Wrong reproducer.
> 
>>
>> But it shouldn't happen to your use case if you applied both WP_UNPOPULATED
>> & WP_ASYNC.
> I'm using both WP_UNPOPULATED and ASYNC. The program gets stuck at right time:
> 
> 
> 1..57
> ok 1 sanity_tests_sd wrong flag specified
> ok 2 sanity_tests_sd wrong mask specified
> ok 3 sanity_tests_sd wrong return mask specified
> ok 4 sanity_tests_sd mixture of correct and wrong flag
> ok 5 sanity_tests_sd PM_SCAN_OP_WP cannot be used without get
> ok 6 sanity_tests_sd Clear area with larger vec size
> ^C
> Program received signal SIGINT, Interrupt.
> 0x000000000040220c in sanity_tests_sd () at pagemap_ioctl.c:198
> 198                     mem[i]++;
> (gdb) bt
> #0  0x000000000040220c in sanity_tests_sd () at pagemap_ioctl.c:198
> #1  0x0000000000404e14 in main () at pagemap_ioctl.c:846
> ()
> 
> /proc/$PID/stack is empty. Not sure why. I can see stack trace of other
> applications, but not this one's.
> 
> Let me send better reproducer for you.
> 
>>
>> Could you try "cat /proc/$PID/stack" to see where does your thread stuck
>> at?
>>
>
  

On Wed, Mar 01, 2023 at 12:55:51PM +0500, Muhammad Usama Anjum wrote:
> Hi Peter,

Hi, Muhammad,

> While using WP_UNPOPULATED, we get stuck if newly allocated memory is read
> without initialization. This can be reproduced by either of the following
> statements:
>     printf("%c", buffer[0]);
>     buffer[0]++;
> 
> This bug has start to appear on this patch. How are you handling reading
> newly allocated memory when WP_UNPOPULATED is defined?

Yes it's a bug, thanks for the reproducer. You're right I missed a trivial
but important detail.  Could you try apply below on top?

---8<---
diff --git a/mm/memory.c b/mm/memory.c
index 46934133bd0b..2f4b3892948b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4062,7 +4062,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
                                                vma->vm_page_prot));
                vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
                                vmf->address, &vmf->ptl);
-               if (!pte_none(*vmf->pte)) {
+               if (vmf_pte_changed(vmf)) {
                        update_mmu_tlb(vma, vmf->address, vmf->pte);
                        goto unlock;
                }
---8<---

I can send a new version after you confirmed it at least works on your
side. I'll also add some more test to cover that in the next version.

The current smoke test within this patch is really light; I somehow rely on
you on this patch on the testing side, and thanks for that.

> Running my pagemap_ioctl selftest as benchmark in a VM:
> without zeropage / wp_unpopulated (decide from pte_none() if page is dirty
> or not, buggy and wrong implementation, just for reference)
> 26.608 seconds
> with zeropage
> 39.203 seconds
> with wp_unpopulated
> 62.907 seconds
> 
> 136% worse performance overall
> 60% worse performance of unpopulated than zeropage

Yes this is unfortunate, because we're protecting more things than before
when with WP_ZEROPAGE / WP_UNPOPULATED but that's what it is for (when we
want to make sure that accuracy on the holes).

I didn't look closer to your whole test suite yet, but my pure test on
protection above should mean that it's still much better for such a use
case than either (1) pre-read or (2) MADV_POPULATE_READ.

Again, I hope the performance result is not a concern to you.  If it is,
please let us know.

Thanks,
  

On 3/1/23 8:19 PM, Peter Xu wrote:
> On Wed, Mar 01, 2023 at 12:55:51PM +0500, Muhammad Usama Anjum wrote:
>> Hi Peter,
> 
> Hi, Muhammad,
> 
>> While using WP_UNPOPULATED, we get stuck if newly allocated memory is read
>> without initialization. This can be reproduced by either of the following
>> statements:
>>     printf("%c", buffer[0]);
>>     buffer[0]++;
>>
>> This bug has start to appear on this patch. How are you handling reading
>> newly allocated memory when WP_UNPOPULATED is defined?
> 
> Yes it's a bug, thanks for the reproducer. You're right I missed a trivial
> but important detail.  Could you try apply below on top?
> 
> ---8<---
> diff --git a/mm/memory.c b/mm/memory.c
> index 46934133bd0b..2f4b3892948b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4062,7 +4062,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>                                                 vma->vm_page_prot));
>                 vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>                                 vmf->address, &vmf->ptl);
> -               if (!pte_none(*vmf->pte)) {
> +               if (vmf_pte_changed(vmf)) {
>                         update_mmu_tlb(vma, vmf->address, vmf->pte);
>                         goto unlock;
>                 }
> ---8<---
This patch works. Thank you so much!

> 
> I can send a new version after you confirmed it at least works on your
> side. I'll also add some more test to cover that in the next version.
> 
> The current smoke test within this patch is really light; I somehow rely on
> you on this patch on the testing side, and thanks for that.
> 
>> Running my pagemap_ioctl selftest as benchmark in a VM:
>> without zeropage / wp_unpopulated (decide from pte_none() if page is dirty
>> or not, buggy and wrong implementation, just for reference)
>> 26.608 seconds
>> with zeropage
>> 39.203 seconds
>> with wp_unpopulated
>> 62.907 seconds
>>
>> 136% worse performance overall
>> 60% worse performance of unpopulated than zeropage
> 
> Yes this is unfortunate, because we're protecting more things than before
> when with WP_ZEROPAGE / WP_UNPOPULATED but that's what it is for (when we
> want to make sure that accuracy on the holes).
> 
> I didn't look closer to your whole test suite yet, but my pure test on
> protection above should mean that it's still much better for such a use
> case than either (1) pre-read or (2) MADV_POPULATE_READ.
Ohh... I should stop comparing UNPOPULATE with buggy implementation and
compare with pre-read. I've compared apples with oranges.

I'll do better benchmark for the comparison sake. I'll let you know if the
performance is becoming an issue. Overall we need pagemap_ioctl + UFFD to
correctly emulate Windows syscall. Secondly we also need good performance
(more the better).

> 
> Again, I hope the performance result is not a concern to you.  If it is,
> please let us know.
> 
> Thanks,
>
  

On 01.03.23 18:13, Muhammad Usama Anjum wrote:
> On 3/1/23 8:19 PM, Peter Xu wrote:
>> On Wed, Mar 01, 2023 at 12:55:51PM +0500, Muhammad Usama Anjum wrote:
>>> Hi Peter,
>>
>> Hi, Muhammad,
>>
>>> While using WP_UNPOPULATED, we get stuck if newly allocated memory is read
>>> without initialization. This can be reproduced by either of the following
>>> statements:
>>>      printf("%c", buffer[0]);
>>>      buffer[0]++;
>>>
>>> This bug has start to appear on this patch. How are you handling reading
>>> newly allocated memory when WP_UNPOPULATED is defined?
>>
>> Yes it's a bug, thanks for the reproducer. You're right I missed a trivial
>> but important detail.  Could you try apply below on top?
>>
>> ---8<---
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 46934133bd0b..2f4b3892948b 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -4062,7 +4062,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>>                                                  vma->vm_page_prot));
>>                  vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>>                                  vmf->address, &vmf->ptl);
>> -               if (!pte_none(*vmf->pte)) {
>> +               if (vmf_pte_changed(vmf)) {
>>                          update_mmu_tlb(vma, vmf->address, vmf->pte);
>>                          goto unlock;
>>                  }
>> ---8<---
> This patch works. Thank you so much!
> 
>>
>> I can send a new version after you confirmed it at least works on your
>> side. I'll also add some more test to cover that in the next version.
>>
>> The current smoke test within this patch is really light; I somehow rely on
>> you on this patch on the testing side, and thanks for that.
>>
>>> Running my pagemap_ioctl selftest as benchmark in a VM:
>>> without zeropage / wp_unpopulated (decide from pte_none() if page is dirty
>>> or not, buggy and wrong implementation, just for reference)
>>> 26.608 seconds
>>> with zeropage
>>> 39.203 seconds
>>> with wp_unpopulated
>>> 62.907 seconds
>>>
>>> 136% worse performance overall
>>> 60% worse performance of unpopulated than zeropage
>>
>> Yes this is unfortunate, because we're protecting more things than before
>> when with WP_ZEROPAGE / WP_UNPOPULATED but that's what it is for (when we
>> want to make sure that accuracy on the holes).
>>
>> I didn't look closer to your whole test suite yet, but my pure test on
>> protection above should mean that it's still much better for such a use
>> case than either (1) pre-read or (2) MADV_POPULATE_READ.
> Ohh... I should stop comparing UNPOPULATE with buggy implementation and
> compare with pre-read. I've compared apples with oranges.

Note that I think there are ways to avoid that overhead (as raised in 
reply to Peter's reply), so IMHO it's still valuable to know which 
benefit we could have without allocating pagetables and placing the 
shared zeropages.

> 
> I'll do better benchmark for the comparison sake. I'll let you know if the
> performance is becoming an issue. Overall we need pagemap_ioctl + UFFD to
> correctly emulate Windows syscall. Secondly we also need good performance
> (more the better).

I'm curious, are you eventually applying UFFD to possibly large (sparse) 
VMAs, that are eventually even only PROT_READ or PROT_WRITE?

Especially for such large sparse VMAs, the current way of allocating 
pagetables to place markers/zeropages is far from optimal.
  

On Thu, Mar 02, 2023 at 10:37:44AM +0100, David Hildenbrand wrote:
> Especially for such large sparse VMAs, the current way of allocating
> pagetables to place markers/zeropages is far from optimal.

IMHO that's not a generic workload.  As mentioned in the reply there, I
would suggest we go with simple then we have space to optimize it in the
future if necessary, because the API will be the same.

If I want to monitor a super sparse VMA on dirty, one thing I can already
try is register with MISSING + WP (!WP_UNPOPULATED).  It may be a good
intermediate solution to avoid any marker ovearheads.
  

On 02.03.23 14:57, Peter Xu wrote:
> On Thu, Mar 02, 2023 at 10:37:44AM +0100, David Hildenbrand wrote:
>> Especially for such large sparse VMAs, the current way of allocating
>> pagetables to place markers/zeropages is far from optimal.
> 
> IMHO that's not a generic workload.  As mentioned in the reply there, I
> would suggest we go with simple then we have space to optimize it in the
> future if necessary, because the API will be the same.
> 

I disagree with "generic workload", we use sparse mmaps all over the 
place, and when blindly used by e.g., CRIU, we'll simply end up wasting 
memory and time.

But I already agreed that this optimization that is a separate thing to 
implement.
  

On 3/2/23 7:01 PM, David Hildenbrand wrote:
> On 02.03.23 14:57, Peter Xu wrote:
>> On Thu, Mar 02, 2023 at 10:37:44AM +0100, David Hildenbrand wrote:
>>> Especially for such large sparse VMAs, the current way of allocating
>>> pagetables to place markers/zeropages is far from optimal.
>>
>> IMHO that's not a generic workload.  As mentioned in the reply there, I
>> would suggest we go with simple then we have space to optimize it in the
>> future if necessary, because the API will be the same.
This is a good idea.

I'm trying to understand why aren't we going with most optimized
implementation. Why aren't we targeting it at this point in time?

>>
> 
> I disagree with "generic workload", we use sparse mmaps all over the place,
> and when blindly used by e.g., CRIU, we'll simply end up wasting memory and
> time.
I've heard about a use case where a file of size 10s of GBs can be mapped
to the memory and then accessed off and on. We need to handle this
correctly and efficiently.

> 
> But I already agreed that this optimization that is a separate thing to
> implement.
>
  

On 2/28/23 5:36 AM, Peter Xu wrote:
> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>> This is a new feature that controls how uffd-wp handles none ptes.  When
>> it's set, the kernel will handle anonymous memory the same way as file
>> memory, by allowing the user to wr-protect unpopulated ptes.
>>
>> 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.
>>
>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>> case if without being able to wr-protect none ptes by default, the dirty
>> info can get lost, since we cannot treat every none pte to be dirty (the
>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>
>> In general, we want to be able to wr-protect empty ptes too even for
>> anonymous.
>>
>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>> uffd-wp handling on none ptes 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.
>>
>> The feature bit is by default off, so the old behavior will be maintained.
>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>> anonymous), but also on creating the pgtables to store the pte markers. So
>> there's potentially less chance of using thp on the first fault for a none
>> pmd or larger than a pmd.
>>
>> The major implementation part is teaching the whole kernel to understand
>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>> new feature bit is set.
>>
>> Note that even if the patch subject starts with mm/uffd, there're a few
>> small refactors to major mm path of handling anonymous page faults. But
>> they should be straightforward.
>>
>> So far, add a very light smoke test within the userfaultfd kselftest
>> pagemap unit test to make sure anon pte markers work.
>>
>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> ---
>> v1->v2:
>> - Use pte markers rather than populate zero pages when protect [David]
>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
> 
> Some very initial performance numbers (I only ran in a VM but it should be
> similar, unit is "us") below as requested.  The measurement is about time
> spent when wr-protecting 10G range of empty but mapped memory.  It's done
> in a VM, assuming we'll get similar results on bare metal.
> 
> Four test cases:
> 
>         - default UFFDIO_WP
>         - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>         - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>         - UFFDIO_WP with WP_UNPOPULATED
> 
> Results:
> 
>         Test DEFAULT: 2
>         Test PRE-READ: 3277099 (pre-fault 3253826)
>         Test MADVISE: 2250361 (pre-fault 2226310)
>         Test WP-UNPOPULATE: 20850
In your case:
Default < WP-UNPOPULATE < MADVISE < PRE-READ


In my testing on next-20230228 with this patch and my uffd async patch:

Test DEFAULT: 6
Test PRE-READ: 37157 (pre-fault 37006)
Test MADVISE: 4884 (pre-fault 4465)
Test WP-UNPOPULATE: 17794

DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ

On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
I'm not sure why I'm getting this discrepancy here. I've liked your results
to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
can be done to get consistent benchmarks over your and my side?

> 
> I'll add these information into the commit message when there's a new
> version.
> 
> [1] https://github.com/xzpeter/clibs/blob/master/uffd-test/uffd-wp-perf.c
>
  

On 02.03.23 18:19, Muhammad Usama Anjum wrote:
> On 2/28/23 5:36 AM, Peter Xu wrote:
>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>>> This is a new feature that controls how uffd-wp handles none ptes.  When
>>> it's set, the kernel will handle anonymous memory the same way as file
>>> memory, by allowing the user to wr-protect unpopulated ptes.
>>>
>>> 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.
>>>
>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>>> case if without being able to wr-protect none ptes by default, the dirty
>>> info can get lost, since we cannot treat every none pte to be dirty (the
>>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>>
>>> In general, we want to be able to wr-protect empty ptes too even for
>>> anonymous.
>>>
>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>>> uffd-wp handling on none ptes 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.
>>>
>>> The feature bit is by default off, so the old behavior will be maintained.
>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>>> anonymous), but also on creating the pgtables to store the pte markers. So
>>> there's potentially less chance of using thp on the first fault for a none
>>> pmd or larger than a pmd.
>>>
>>> The major implementation part is teaching the whole kernel to understand
>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>>> new feature bit is set.
>>>
>>> Note that even if the patch subject starts with mm/uffd, there're a few
>>> small refactors to major mm path of handling anonymous page faults. But
>>> they should be straightforward.
>>>
>>> So far, add a very light smoke test within the userfaultfd kselftest
>>> pagemap unit test to make sure anon pte markers work.
>>>
>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>>
>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>> ---
>>> v1->v2:
>>> - Use pte markers rather than populate zero pages when protect [David]
>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
>>
>> Some very initial performance numbers (I only ran in a VM but it should be
>> similar, unit is "us") below as requested.  The measurement is about time
>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
>> in a VM, assuming we'll get similar results on bare metal.
>>
>> Four test cases:
>>
>>          - default UFFDIO_WP
>>          - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>>          - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>>          - UFFDIO_WP with WP_UNPOPULATED
>>
>> Results:
>>
>>          Test DEFAULT: 2
>>          Test PRE-READ: 3277099 (pre-fault 3253826)
>>          Test MADVISE: 2250361 (pre-fault 2226310)
>>          Test WP-UNPOPULATE: 20850
> In your case:
> Default < WP-UNPOPULATE < MADVISE < PRE-READ
> 
> 
> In my testing on next-20230228 with this patch and my uffd async patch:
> 
> Test DEFAULT: 6
> Test PRE-READ: 37157 (pre-fault 37006)
> Test MADVISE: 4884 (pre-fault 4465)
> Test WP-UNPOPULATE: 17794
> 
> DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ
> 
> On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
> I'm not sure why I'm getting this discrepancy here. I've liked your results
> to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
> can be done to get consistent benchmarks over your and my side?

Probably because the current approach from Peter uses uffd-wp markers, 
and these markers can currently only reside on the PTE level, not on the 
PMD level yet.

With MADVISE you get a huge zeropage and avoid dealing with PTEs.
  

On Thu, Mar 02, 2023 at 08:14:05PM +0500, Muhammad Usama Anjum wrote:
> I've heard about a use case where a file of size 10s of GBs can be mapped
> to the memory and then accessed off and on. We need to handle this
> correctly and efficiently.

Note again that besides any slowness it'll stop working for generic FSs.

One way is we can consider enabling uffd-wp async for all archs, but I need
to think more about it, and it will not be compatible with any other modes.
  

On Thu, Mar 02, 2023 at 06:38:20PM +0100, David Hildenbrand wrote:
> On 02.03.23 18:19, Muhammad Usama Anjum wrote:
> > On 2/28/23 5:36 AM, Peter Xu wrote:
> > > On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
> > > > This is a new feature that controls how uffd-wp handles none ptes.  When
> > > > it's set, the kernel will handle anonymous memory the same way as file
> > > > memory, by allowing the user to wr-protect unpopulated ptes.
> > > > 
> > > > 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.
> > > > 
> > > > One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
> > > > detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
> > > > case if without being able to wr-protect none ptes by default, the dirty
> > > > info can get lost, since we cannot treat every none pte to be dirty (the
> > > > current design is identify a page dirty based on uffd-wp bit being cleared).
> > > > 
> > > > In general, we want to be able to wr-protect empty ptes too even for
> > > > anonymous.
> > > > 
> > > > This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
> > > > uffd-wp handling on none ptes 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.
> > > > 
> > > > The feature bit is by default off, so the old behavior will be maintained.
> > > > Sometimes it may be wanted because the wr-protect of none ptes will contain
> > > > overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
> > > > anonymous), but also on creating the pgtables to store the pte markers. So
> > > > there's potentially less chance of using thp on the first fault for a none
> > > > pmd or larger than a pmd.
> > > > 
> > > > The major implementation part is teaching the whole kernel to understand
> > > > pte markers even for anonymously mapped ranges, meanwhile allowing the
> > > > UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
> > > > new feature bit is set.
> > > > 
> > > > Note that even if the patch subject starts with mm/uffd, there're a few
> > > > small refactors to major mm path of handling anonymous page faults. But
> > > > they should be straightforward.
> > > > 
> > > > So far, add a very light smoke test within the userfaultfd kselftest
> > > > pagemap unit test to make sure anon pte markers work.
> > > > 
> > > > [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
> > > > [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
> > > > 
> > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > ---
> > > > v1->v2:
> > > > - Use pte markers rather than populate zero pages when protect [David]
> > > > - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
> > > 
> > > Some very initial performance numbers (I only ran in a VM but it should be
> > > similar, unit is "us") below as requested.  The measurement is about time
> > > spent when wr-protecting 10G range of empty but mapped memory.  It's done
> > > in a VM, assuming we'll get similar results on bare metal.
> > > 
> > > Four test cases:
> > > 
> > >          - default UFFDIO_WP
> > >          - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
> > >          - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
> > >          - UFFDIO_WP with WP_UNPOPULATED
> > > 
> > > Results:
> > > 
> > >          Test DEFAULT: 2
> > >          Test PRE-READ: 3277099 (pre-fault 3253826)
> > >          Test MADVISE: 2250361 (pre-fault 2226310)
> > >          Test WP-UNPOPULATE: 20850
> > In your case:
> > Default < WP-UNPOPULATE < MADVISE < PRE-READ
> > 
> > 
> > In my testing on next-20230228 with this patch and my uffd async patch:
> > 
> > Test DEFAULT: 6
> > Test PRE-READ: 37157 (pre-fault 37006)
> > Test MADVISE: 4884 (pre-fault 4465)
> > Test WP-UNPOPULATE: 17794
> > 
> > DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ
> > 
> > On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
> > I'm not sure why I'm getting this discrepancy here. I've liked your results
> > to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
> > can be done to get consistent benchmarks over your and my side?
> 
> Probably because the current approach from Peter uses uffd-wp markers, and
> these markers can currently only reside on the PTE level, not on the PMD
> level yet.
> 
> With MADVISE you get a huge zeropage and avoid dealing with PTEs.

Yes, probably.  But then when write happens it'll be done there when split,
so the overhead was delayed.

Meanwhile I'll retest again (probably tomorrow..) with bare metals with THP
on/off to double check.

Muhammad, do you think the current performance will work for you?

Especially I want to double check with you again on whether
XFS/EXT4/... will be needed for the tracking purpose so you can reply here
together.  We shouldn't merge anything that doesn't have at least one
existing good use case, and we may need to rethink if it's not.

For performance, one approach is probably making uffd-wp async separate
from other features, where we can revert the meaning of uffd-wp bit to
mimic what soft-dirty does (I think this will look closer to what David
mentioned in the other thread), by defining uffd-wp=1 as "written" and
uffd-wp=0 as clean.

IIUC that'll make it one bit and work as fast as soft-dirty, meanwhile all
uffd-wp marker things can hopefully still be maintained.  However I really
don't like it to violate a lot of things, e.g., when UFFDIO_WRITEPROTECT
another round we'll need to DROP uffd-wp if async, but APPLY uffd-wp if
sync...  So in general it'll need more thoughts and slower to do.
  

On 3/3/23 3:21 AM, Peter Xu wrote:
> On Thu, Mar 02, 2023 at 06:38:20PM +0100, David Hildenbrand wrote:
>> On 02.03.23 18:19, Muhammad Usama Anjum wrote:
>>> On 2/28/23 5:36 AM, Peter Xu wrote:
>>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>>>>> This is a new feature that controls how uffd-wp handles none ptes.  When
>>>>> it's set, the kernel will handle anonymous memory the same way as file
>>>>> memory, by allowing the user to wr-protect unpopulated ptes.
>>>>>
>>>>> 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.
>>>>>
>>>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>>>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>>>>> case if without being able to wr-protect none ptes by default, the dirty
>>>>> info can get lost, since we cannot treat every none pte to be dirty (the
>>>>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>>>>
>>>>> In general, we want to be able to wr-protect empty ptes too even for
>>>>> anonymous.
>>>>>
>>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>>>>> uffd-wp handling on none ptes 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.
>>>>>
>>>>> The feature bit is by default off, so the old behavior will be maintained.
>>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>>>>> anonymous), but also on creating the pgtables to store the pte markers. So
>>>>> there's potentially less chance of using thp on the first fault for a none
>>>>> pmd or larger than a pmd.
>>>>>
>>>>> The major implementation part is teaching the whole kernel to understand
>>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>>>>> new feature bit is set.
>>>>>
>>>>> Note that even if the patch subject starts with mm/uffd, there're a few
>>>>> small refactors to major mm path of handling anonymous page faults. But
>>>>> they should be straightforward.
>>>>>
>>>>> So far, add a very light smoke test within the userfaultfd kselftest
>>>>> pagemap unit test to make sure anon pte markers work.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>>>>
>>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>>> ---
>>>>> v1->v2:
>>>>> - Use pte markers rather than populate zero pages when protect [David]
>>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
>>>>
>>>> Some very initial performance numbers (I only ran in a VM but it should be
>>>> similar, unit is "us") below as requested.  The measurement is about time
>>>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
>>>> in a VM, assuming we'll get similar results on bare metal.
>>>>
>>>> Four test cases:
>>>>
>>>>          - default UFFDIO_WP
>>>>          - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>>>>          - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>>>>          - UFFDIO_WP with WP_UNPOPULATED
>>>>
>>>> Results:
>>>>
>>>>          Test DEFAULT: 2
>>>>          Test PRE-READ: 3277099 (pre-fault 3253826)
>>>>          Test MADVISE: 2250361 (pre-fault 2226310)
>>>>          Test WP-UNPOPULATE: 20850
>>> In your case:
>>> Default < WP-UNPOPULATE < MADVISE < PRE-READ
>>>
>>>
>>> In my testing on next-20230228 with this patch and my uffd async patch:
>>>
>>> Test DEFAULT: 6
>>> Test PRE-READ: 37157 (pre-fault 37006)
>>> Test MADVISE: 4884 (pre-fault 4465)
>>> Test WP-UNPOPULATE: 17794
>>>
>>> DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ
>>>
>>> On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
>>> I'm not sure why I'm getting this discrepancy here. I've liked your results
>>> to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
>>> can be done to get consistent benchmarks over your and my side?
>>
>> Probably because the current approach from Peter uses uffd-wp markers, and
>> these markers can currently only reside on the PTE level, not on the PMD
>> level yet.
>>
>> With MADVISE you get a huge zeropage and avoid dealing with PTEs.
> 
> Yes, probably.  But then when write happens it'll be done there when split,
> so the overhead was delayed.
> 
> Meanwhile I'll retest again (probably tomorrow..) with bare metals with THP
> on/off to double check.
Turning on/off THP has effect.

(1) With huge page disabled
echo madvise > /sys/kernel/mm/transparent_hugepage/enabled
./uffd_wp_perf
Test DEFAULT: 4
Test PRE-READ: 1111453 (pre-fault 1101011)
Test MADVISE: 278276 (pre-fault 266378)
Test WP-UNPOPULATE: 11712

(2) With Huge page enabled
echo always > /sys/kernel/mm/transparent_hugepage/enabled
./uffd_wp_perf
Test DEFAULT: 4
Test PRE-READ: 22521 (pre-fault 22348)
Test MADVISE: 4909 (pre-fault 4743)
Test WP-UNPOPULATE: 14448

> 
> Muhammad, do you think the current performance will work for you?
Yes, the current performance is good enough for now. We'll be using normal
pages for memory on which PAGEMAP_IOCTL will be used for emulating Windows
syscall. When normal size pages are used, (1) show that WP-UNPOPULATE has
really good performance (278276 --> 11712). Lets proceed with it.

> 
> Especially I want to double check with you again on whether
> XFS/EXT4/... will be needed for the tracking purpose so you can reply here
> together.  We shouldn't merge anything that doesn't have at least one
> existing good use case, and we may need to rethink if it's not.
We have been trying to find soft dirty pages correctly on anon memory.
Right now with UN_POPULATED, ASYNC and PAGEMAP_IOCTL, we'll be able to
achieve this correctly.

We want to track the file mapped memory the same way for another Windows
api translation in which file memory will be tracked to find out soft-dirty
pages. I believe once it is supported in the mm, filesystem doesn't matter
as we'll be tracking memory mapped files.

So we have use cases for both soft-dirty tracking on anon and file backed
memories.

> 
> For performance, one approach is probably making uffd-wp async separate
> from other features, where we can revert the meaning of uffd-wp bit to
> mimic what soft-dirty does (I think this will look closer to what David
> mentioned in the other thread), by defining uffd-wp=1 as "written" and
> uffd-wp=0 as clean.
I'm happy with current state of things. WP Async is just a feature where
faults are resolved automatically. The page is WP or not perfectly
translates if page is written (dirty) or not with just not (!) operation.
If we go and change the meanings of uffd_wp=1 as written, it would cause
confusion and probably we'll need a lot more changes to add async feature
which is right now quite small changeset.

> 
> IIUC that'll make it one bit and work as fast as soft-dirty, meanwhile all
> uffd-wp marker things can hopefully still be maintained.  However I really
> don't like it to violate a lot of things, e.g., when UFFDIO_WRITEPROTECT
> another round we'll need to DROP uffd-wp if async, but APPLY uffd-wp if
> sync...  So in general it'll need more thoughts and slower to do.
Sorry, I'm unable to understand your example here. Can you please elaborate?

>
  

On Fri, Mar 03, 2023 at 11:42:46AM +0500, Muhammad Usama Anjum wrote:
> On 3/3/23 3:21 AM, Peter Xu wrote:
> > On Thu, Mar 02, 2023 at 06:38:20PM +0100, David Hildenbrand wrote:
> >> On 02.03.23 18:19, Muhammad Usama Anjum wrote:
> >>> On 2/28/23 5:36 AM, Peter Xu wrote:
> >>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
> >>>>> This is a new feature that controls how uffd-wp handles none ptes.  When
> >>>>> it's set, the kernel will handle anonymous memory the same way as file
> >>>>> memory, by allowing the user to wr-protect unpopulated ptes.
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
> >>>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
> >>>>> case if without being able to wr-protect none ptes by default, the dirty
> >>>>> info can get lost, since we cannot treat every none pte to be dirty (the
> >>>>> current design is identify a page dirty based on uffd-wp bit being cleared).
> >>>>>
> >>>>> In general, we want to be able to wr-protect empty ptes too even for
> >>>>> anonymous.
> >>>>>
> >>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
> >>>>> uffd-wp handling on none ptes 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.
> >>>>>
> >>>>> The feature bit is by default off, so the old behavior will be maintained.
> >>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
> >>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
> >>>>> anonymous), but also on creating the pgtables to store the pte markers. So
> >>>>> there's potentially less chance of using thp on the first fault for a none
> >>>>> pmd or larger than a pmd.
> >>>>>
> >>>>> The major implementation part is teaching the whole kernel to understand
> >>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
> >>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
> >>>>> new feature bit is set.
> >>>>>
> >>>>> Note that even if the patch subject starts with mm/uffd, there're a few
> >>>>> small refactors to major mm path of handling anonymous page faults. But
> >>>>> they should be straightforward.
> >>>>>
> >>>>> So far, add a very light smoke test within the userfaultfd kselftest
> >>>>> pagemap unit test to make sure anon pte markers work.
> >>>>>
> >>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
> >>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
> >>>>>
> >>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
> >>>>> ---
> >>>>> v1->v2:
> >>>>> - Use pte markers rather than populate zero pages when protect [David]
> >>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
> >>>>
> >>>> Some very initial performance numbers (I only ran in a VM but it should be
> >>>> similar, unit is "us") below as requested.  The measurement is about time
> >>>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
> >>>> in a VM, assuming we'll get similar results on bare metal.
> >>>>
> >>>> Four test cases:
> >>>>
> >>>>          - default UFFDIO_WP
> >>>>          - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
> >>>>          - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
> >>>>          - UFFDIO_WP with WP_UNPOPULATED
> >>>>
> >>>> Results:
> >>>>
> >>>>          Test DEFAULT: 2
> >>>>          Test PRE-READ: 3277099 (pre-fault 3253826)
> >>>>          Test MADVISE: 2250361 (pre-fault 2226310)
> >>>>          Test WP-UNPOPULATE: 20850
> >>> In your case:
> >>> Default < WP-UNPOPULATE < MADVISE < PRE-READ
> >>>
> >>>
> >>> In my testing on next-20230228 with this patch and my uffd async patch:
> >>>
> >>> Test DEFAULT: 6
> >>> Test PRE-READ: 37157 (pre-fault 37006)
> >>> Test MADVISE: 4884 (pre-fault 4465)
> >>> Test WP-UNPOPULATE: 17794
> >>>
> >>> DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ
> >>>
> >>> On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
> >>> I'm not sure why I'm getting this discrepancy here. I've liked your results
> >>> to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
> >>> can be done to get consistent benchmarks over your and my side?
> >>
> >> Probably because the current approach from Peter uses uffd-wp markers, and
> >> these markers can currently only reside on the PTE level, not on the PMD
> >> level yet.
> >>
> >> With MADVISE you get a huge zeropage and avoid dealing with PTEs.
> > 
> > Yes, probably.  But then when write happens it'll be done there when split,
> > so the overhead was delayed.
> > 
> > Meanwhile I'll retest again (probably tomorrow..) with bare metals with THP
> > on/off to double check.
> Turning on/off THP has effect.
> 
> (1) With huge page disabled
> echo madvise > /sys/kernel/mm/transparent_hugepage/enabled
> ./uffd_wp_perf
> Test DEFAULT: 4
> Test PRE-READ: 1111453 (pre-fault 1101011)
> Test MADVISE: 278276 (pre-fault 266378)
> Test WP-UNPOPULATE: 11712
> 
> (2) With Huge page enabled
> echo always > /sys/kernel/mm/transparent_hugepage/enabled
> ./uffd_wp_perf
> Test DEFAULT: 4
> Test PRE-READ: 22521 (pre-fault 22348)
> Test MADVISE: 4909 (pre-fault 4743)
> Test WP-UNPOPULATE: 14448

Thanks. Yes then this is explaine-able to me.  As mentioned previously, thp
will drastically speedup everything during prefault+ioctl(UFFDIO_WP),
though that overhead will be applied when the 1st write triggers later,
afaik.

> 
> > 
> > Muhammad, do you think the current performance will work for you?
> Yes, the current performance is good enough for now. We'll be using normal
> pages for memory on which PAGEMAP_IOCTL will be used for emulating Windows
> syscall. When normal size pages are used, (1) show that WP-UNPOPULATE has
> really good performance (278276 --> 11712). Lets proceed with it.

OK.

> 
> > 
> > Especially I want to double check with you again on whether
> > XFS/EXT4/... will be needed for the tracking purpose so you can reply here
> > together.  We shouldn't merge anything that doesn't have at least one
> > existing good use case, and we may need to rethink if it's not.
> We have been trying to find soft dirty pages correctly on anon memory.
> Right now with UN_POPULATED, ASYNC and PAGEMAP_IOCTL, we'll be able to
> achieve this correctly.
> 
> We want to track the file mapped memory the same way for another Windows
> api translation in which file memory will be tracked to find out soft-dirty
> pages. I believe once it is supported in the mm, filesystem doesn't matter
> as we'll be tracking memory mapped files.

Some more elaboration would be great here.

For example, what is the other windows API?  What will you do with the
dirty information?  What is the impact of supporting only shmem/hugetlbfs
for "!anon"?

> 
> So we have use cases for both soft-dirty tracking on anon and file backed
> memories.
> 
> > 
> > For performance, one approach is probably making uffd-wp async separate
> > from other features, where we can revert the meaning of uffd-wp bit to
> > mimic what soft-dirty does (I think this will look closer to what David
> > mentioned in the other thread), by defining uffd-wp=1 as "written" and
> > uffd-wp=0 as clean.
> I'm happy with current state of things. WP Async is just a feature where
> faults are resolved automatically. The page is WP or not perfectly
> translates if page is written (dirty) or not with just not (!) operation.
> If we go and change the meanings of uffd_wp=1 as written, it would cause
> confusion and probably we'll need a lot more changes to add async feature
> which is right now quite small changeset.

Yes, that's also why I wanted to keep it like this unless necessary.  It
makes all uffd features still more aligned with everything so more maintainable.

> 
> > 
> > IIUC that'll make it one bit and work as fast as soft-dirty, meanwhile all
> > uffd-wp marker things can hopefully still be maintained.  However I really
> > don't like it to violate a lot of things, e.g., when UFFDIO_WRITEPROTECT
> > another round we'll need to DROP uffd-wp if async, but APPLY uffd-wp if
> > sync...  So in general it'll need more thoughts and slower to do.
> Sorry, I'm unable to understand your example here. Can you please elaborate?

I think it's mostly what you stated above, the changeset will just get more
involved because of either the redefinition of uffd-wp depending on feature
enabled, or using yet another bit (e.g., if you still remember the very
original reply I was planning to reuse soft-dirty bit but used together
with VM_UFFD_WP; I think that resolves the degraded perf on unpopulated
memory too because it'll work more like soft-dirty).

This is some detail which, IMHO, we can leave that for later.  To me I'm
more curious on knowing more details of your use case, and how uffd could
suite that the best way it could, assuming that (or part of it like the
unpopulate work here) can also be leveraged by others.  Your answers to
above questions may help.

Thanks,
  

On 3/3/23 9:47 PM, Peter Xu wrote:
> On Fri, Mar 03, 2023 at 11:42:46AM +0500, Muhammad Usama Anjum wrote:
>> On 3/3/23 3:21 AM, Peter Xu wrote:
>>> On Thu, Mar 02, 2023 at 06:38:20PM +0100, David Hildenbrand wrote:
>>>> On 02.03.23 18:19, Muhammad Usama Anjum wrote:
>>>>> On 2/28/23 5:36 AM, Peter Xu wrote:
>>>>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>>>>>>> This is a new feature that controls how uffd-wp handles none ptes.  When
>>>>>>> it's set, the kernel will handle anonymous memory the same way as file
>>>>>>> memory, by allowing the user to wr-protect unpopulated ptes.
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>>>>>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>>>>>>> case if without being able to wr-protect none ptes by default, the dirty
>>>>>>> info can get lost, since we cannot treat every none pte to be dirty (the
>>>>>>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>>>>>>
>>>>>>> In general, we want to be able to wr-protect empty ptes too even for
>>>>>>> anonymous.
>>>>>>>
>>>>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>>>>>>> uffd-wp handling on none ptes 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.
>>>>>>>
>>>>>>> The feature bit is by default off, so the old behavior will be maintained.
>>>>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>>>>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>>>>>>> anonymous), but also on creating the pgtables to store the pte markers. So
>>>>>>> there's potentially less chance of using thp on the first fault for a none
>>>>>>> pmd or larger than a pmd.
>>>>>>>
>>>>>>> The major implementation part is teaching the whole kernel to understand
>>>>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>>>>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>>>>>>> new feature bit is set.
>>>>>>>
>>>>>>> Note that even if the patch subject starts with mm/uffd, there're a few
>>>>>>> small refactors to major mm path of handling anonymous page faults. But
>>>>>>> they should be straightforward.
>>>>>>>
>>>>>>> So far, add a very light smoke test within the userfaultfd kselftest
>>>>>>> pagemap unit test to make sure anon pte markers work.
>>>>>>>
>>>>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>>>>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>>>>>>
>>>>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>>>>> ---
>>>>>>> v1->v2:
>>>>>>> - Use pte markers rather than populate zero pages when protect [David]
>>>>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
>>>>>>
>>>>>> Some very initial performance numbers (I only ran in a VM but it should be
>>>>>> similar, unit is "us") below as requested.  The measurement is about time
>>>>>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
>>>>>> in a VM, assuming we'll get similar results on bare metal.
>>>>>>
>>>>>> Four test cases:
>>>>>>
>>>>>>          - default UFFDIO_WP
>>>>>>          - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>>>>>>          - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>>>>>>          - UFFDIO_WP with WP_UNPOPULATED
>>>>>>
>>>>>> Results:
>>>>>>
>>>>>>          Test DEFAULT: 2
>>>>>>          Test PRE-READ: 3277099 (pre-fault 3253826)
>>>>>>          Test MADVISE: 2250361 (pre-fault 2226310)
>>>>>>          Test WP-UNPOPULATE: 20850
>>>>> In your case:
>>>>> Default < WP-UNPOPULATE < MADVISE < PRE-READ
>>>>>
>>>>>
>>>>> In my testing on next-20230228 with this patch and my uffd async patch:
>>>>>
>>>>> Test DEFAULT: 6
>>>>> Test PRE-READ: 37157 (pre-fault 37006)
>>>>> Test MADVISE: 4884 (pre-fault 4465)
>>>>> Test WP-UNPOPULATE: 17794
>>>>>
>>>>> DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ
>>>>>
>>>>> On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
>>>>> I'm not sure why I'm getting this discrepancy here. I've liked your results
>>>>> to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
>>>>> can be done to get consistent benchmarks over your and my side?
>>>>
>>>> Probably because the current approach from Peter uses uffd-wp markers, and
>>>> these markers can currently only reside on the PTE level, not on the PMD
>>>> level yet.
>>>>
>>>> With MADVISE you get a huge zeropage and avoid dealing with PTEs.
>>>
>>> Yes, probably.  But then when write happens it'll be done there when split,
>>> so the overhead was delayed.
>>>
>>> Meanwhile I'll retest again (probably tomorrow..) with bare metals with THP
>>> on/off to double check.
>> Turning on/off THP has effect.
>>
>> (1) With huge page disabled
>> echo madvise > /sys/kernel/mm/transparent_hugepage/enabled
>> ./uffd_wp_perf
>> Test DEFAULT: 4
>> Test PRE-READ: 1111453 (pre-fault 1101011)
>> Test MADVISE: 278276 (pre-fault 266378)
>> Test WP-UNPOPULATE: 11712
Hi Paul,

We were reading, touching or faulting memory before engaging wp. This
reading is called pre-read in this benchmark. We can fault the memory
through madvise as well, it is called madvise here. WP-Unpopulated is being
added in this patch which will remove the need to pre-fault the memory and
it is way faster as shown by the numbers.

>>
>> (2) With Huge page enabled
>> echo always > /sys/kernel/mm/transparent_hugepage/enabled
>> ./uffd_wp_perf
>> Test DEFAULT: 4
>> Test PRE-READ: 22521 (pre-fault 22348)
>> Test MADVISE: 4909 (pre-fault 4743)
>> Test WP-UNPOPULATE: 14448
> 
> Thanks. Yes then this is explaine-able to me.  As mentioned previously, thp
> will drastically speedup everything during prefault+ioctl(UFFDIO_WP),
> though that overhead will be applied when the 1st write triggers later,
> afaik.
> 
>>
>>>
>>> Muhammad, do you think the current performance will work for you?
>> Yes, the current performance is good enough for now. We'll be using normal
>> pages for memory on which PAGEMAP_IOCTL will be used for emulating Windows
>> syscall. When normal size pages are used, (1) show that WP-UNPOPULATE has
>> really good performance (278276 --> 11712). Lets proceed with it.
> 
> OK.
> 
>>
>>>
>>> Especially I want to double check with you again on whether
>>> XFS/EXT4/... will be needed for the tracking purpose so you can reply here
>>> together.  We shouldn't merge anything that doesn't have at least one
>>> existing good use case, and we may need to rethink if it's not.
>> We have been trying to find soft dirty pages correctly on anon memory.
>> Right now with UN_POPULATED, ASYNC and PAGEMAP_IOCTL, we'll be able to
>> achieve this correctly.
>>
>> We want to track the file mapped memory the same way for another Windows
>> api translation in which file memory will be tracked to find out soft-dirty
>> pages. I believe once it is supported in the mm, filesystem doesn't matter
>> as we'll be tracking memory mapped files.
> 
> Some more elaboration would be great here.
There is something called "writecopy" in Windows. Paul has given me some
details.

@Paul can you share more details that:
1) What is writecopy?
2) If there is no documentation or documented behavior, Can you please
share the current reference implementation in Wine even though it is buggy
at the time? (I can implement correct implementation with this patch and my
wip v11 pagemap_ioctl patches for testing purpose.)

> 
> For example, what is the other windows API?  What will you do with the
> dirty information?  What is the impact of supporting only shmem/hugetlbfs
> for "!anon"?
> 
>>
>> So we have use cases for both soft-dirty tracking on anon and file backed
>> memories.
>>
>>>
>>> For performance, one approach is probably making uffd-wp async separate
>>> from other features, where we can revert the meaning of uffd-wp bit to
>>> mimic what soft-dirty does (I think this will look closer to what David
>>> mentioned in the other thread), by defining uffd-wp=1 as "written" and
>>> uffd-wp=0 as clean.
>> I'm happy with current state of things. WP Async is just a feature where
>> faults are resolved automatically. The page is WP or not perfectly
>> translates if page is written (dirty) or not with just not (!) operation.
>> If we go and change the meanings of uffd_wp=1 as written, it would cause
>> confusion and probably we'll need a lot more changes to add async feature
>> which is right now quite small changeset.
> 
> Yes, that's also why I wanted to keep it like this unless necessary.  It
> makes all uffd features still more aligned with everything so more maintainable.
> 
>>
>>>
>>> IIUC that'll make it one bit and work as fast as soft-dirty, meanwhile all
>>> uffd-wp marker things can hopefully still be maintained.  However I really
>>> don't like it to violate a lot of things, e.g., when UFFDIO_WRITEPROTECT
>>> another round we'll need to DROP uffd-wp if async, but APPLY uffd-wp if
>>> sync...  So in general it'll need more thoughts and slower to do.
>> Sorry, I'm unable to understand your example here. Can you please elaborate?
> 
> I think it's mostly what you stated above, the changeset will just get more
> involved because of either the redefinition of uffd-wp depending on feature
> enabled, or using yet another bit (e.g., if you still remember the very
> original reply I was planning to reuse soft-dirty bit but used together
> with VM_UFFD_WP; I think that resolves the degraded perf on unpopulated
> memory too because it'll work more like soft-dirty).
> 
> This is some detail which, IMHO, we can leave that for later.  To me I'm
> more curious on knowing more details of your use case, and how uffd could
> suite that the best way it could, assuming that (or part of it like the
> unpopulate work here) can also be leveraged by others.  Your answers to
> above questions may help.
> 
> Thanks,
>
  

On 3/6/23 2:03 PM, Muhammad Usama Anjum wrote:
> On 3/3/23 9:47 PM, Peter Xu wrote:
>> On Fri, Mar 03, 2023 at 11:42:46AM +0500, Muhammad Usama Anjum wrote:
>>> On 3/3/23 3:21 AM, Peter Xu wrote:
>>>> On Thu, Mar 02, 2023 at 06:38:20PM +0100, David Hildenbrand wrote:
>>>>> On 02.03.23 18:19, Muhammad Usama Anjum wrote:
>>>>>> On 2/28/23 5:36 AM, Peter Xu wrote:
>>>>>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote:
>>>>>>>> This is a new feature that controls how uffd-wp handles none ptes.  When
>>>>>>>> it's set, the kernel will handle anonymous memory the same way as file
>>>>>>>> memory, by allowing the user to wr-protect unpopulated ptes.
>>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> One use case of such a feature bit 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 OS assumes the pages should be 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 raised on using userfaultfd wr-protect for
>>>>>>>> detecting dirty pages (to replace soft-dirty in some cases) [2].  In that
>>>>>>>> case if without being able to wr-protect none ptes by default, the dirty
>>>>>>>> info can get lost, since we cannot treat every none pte to be dirty (the
>>>>>>>> current design is identify a page dirty based on uffd-wp bit being cleared).
>>>>>>>>
>>>>>>>> In general, we want to be able to wr-protect empty ptes too even for
>>>>>>>> anonymous.
>>>>>>>>
>>>>>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make
>>>>>>>> uffd-wp handling on none ptes 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.
>>>>>>>>
>>>>>>>> The feature bit is by default off, so the old behavior will be maintained.
>>>>>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain
>>>>>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to
>>>>>>>> anonymous), but also on creating the pgtables to store the pte markers. So
>>>>>>>> there's potentially less chance of using thp on the first fault for a none
>>>>>>>> pmd or larger than a pmd.
>>>>>>>>
>>>>>>>> The major implementation part is teaching the whole kernel to understand
>>>>>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the
>>>>>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the
>>>>>>>> new feature bit is set.
>>>>>>>>
>>>>>>>> Note that even if the patch subject starts with mm/uffd, there're a few
>>>>>>>> small refactors to major mm path of handling anonymous page faults. But
>>>>>>>> they should be straightforward.
>>>>>>>>
>>>>>>>> So far, add a very light smoke test within the userfaultfd kselftest
>>>>>>>> pagemap unit test to make sure anon pte markers work.
>>>>>>>>
>>>>>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/
>>>>>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/
>>>>>>>>
>>>>>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>>>>>> ---
>>>>>>>> v1->v2:
>>>>>>>> - Use pte markers rather than populate zero pages when protect [David]
>>>>>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David]
>>>>>>>
>>>>>>> Some very initial performance numbers (I only ran in a VM but it should be
>>>>>>> similar, unit is "us") below as requested.  The measurement is about time
>>>>>>> spent when wr-protecting 10G range of empty but mapped memory.  It's done
>>>>>>> in a VM, assuming we'll get similar results on bare metal.
>>>>>>>
>>>>>>> Four test cases:
>>>>>>>
>>>>>>>          - default UFFDIO_WP
>>>>>>>          - pre-read the memory, then UFFDIO_WP (what QEMU does right now)
>>>>>>>          - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP
>>>>>>>          - UFFDIO_WP with WP_UNPOPULATED
>>>>>>>
>>>>>>> Results:
>>>>>>>
>>>>>>>          Test DEFAULT: 2
>>>>>>>          Test PRE-READ: 3277099 (pre-fault 3253826)
>>>>>>>          Test MADVISE: 2250361 (pre-fault 2226310)
>>>>>>>          Test WP-UNPOPULATE: 20850
>>>>>> In your case:
>>>>>> Default < WP-UNPOPULATE < MADVISE < PRE-READ
>>>>>>
>>>>>>
>>>>>> In my testing on next-20230228 with this patch and my uffd async patch:
>>>>>>
>>>>>> Test DEFAULT: 6
>>>>>> Test PRE-READ: 37157 (pre-fault 37006)
>>>>>> Test MADVISE: 4884 (pre-fault 4465)
>>>>>> Test WP-UNPOPULATE: 17794
>>>>>>
>>>>>> DEFAULT < MADVISE < WP-UNPOPULATE < PRE-READ
>>>>>>
>>>>>> On my setup, MADVISE is performing better than WP-UNPOPULATE consistently.
>>>>>> I'm not sure why I'm getting this discrepancy here. I've liked your results
>>>>>> to be honest where we perform better with WP-UNPOPULATE than MADVISE. What
>>>>>> can be done to get consistent benchmarks over your and my side?
>>>>>
>>>>> Probably because the current approach from Peter uses uffd-wp markers, and
>>>>> these markers can currently only reside on the PTE level, not on the PMD
>>>>> level yet.
>>>>>
>>>>> With MADVISE you get a huge zeropage and avoid dealing with PTEs.
>>>>
>>>> Yes, probably.  But then when write happens it'll be done there when split,
>>>> so the overhead was delayed.
>>>>
>>>> Meanwhile I'll retest again (probably tomorrow..) with bare metals with THP
>>>> on/off to double check.
>>> Turning on/off THP has effect.
>>>
>>> (1) With huge page disabled
>>> echo madvise > /sys/kernel/mm/transparent_hugepage/enabled
>>> ./uffd_wp_perf
>>> Test DEFAULT: 4
>>> Test PRE-READ: 1111453 (pre-fault 1101011)
>>> Test MADVISE: 278276 (pre-fault 266378)
>>> Test WP-UNPOPULATE: 11712
> Hi Paul,
> 
> We were reading, touching or faulting memory before engaging wp. This
> reading is called pre-read in this benchmark. We can fault the memory
> through madvise as well, it is called madvise here. WP-Unpopulated is being
> added in this patch which will remove the need to pre-fault the memory and
> it is way faster as shown by the numbers.
> 
>>>
>>> (2) With Huge page enabled
>>> echo always > /sys/kernel/mm/transparent_hugepage/enabled
>>> ./uffd_wp_perf
>>> Test DEFAULT: 4
>>> Test PRE-READ: 22521 (pre-fault 22348)
>>> Test MADVISE: 4909 (pre-fault 4743)
>>> Test WP-UNPOPULATE: 14448
>>
>> Thanks. Yes then this is explaine-able to me.  As mentioned previously, thp
>> will drastically speedup everything during prefault+ioctl(UFFDIO_WP),
>> though that overhead will be applied when the 1st write triggers later,
>> afaik.
>>
>>>
>>>>
>>>> Muhammad, do you think the current performance will work for you?
>>> Yes, the current performance is good enough for now. We'll be using normal
>>> pages for memory on which PAGEMAP_IOCTL will be used for emulating Windows
>>> syscall. When normal size pages are used, (1) show that WP-UNPOPULATE has
>>> really good performance (278276 --> 11712). Lets proceed with it.
>>
>> OK.
>>
>>>
>>>>
>>>> Especially I want to double check with you again on whether
>>>> XFS/EXT4/... will be needed for the tracking purpose so you can reply here
>>>> together.  We shouldn't merge anything that doesn't have at least one
>>>> existing good use case, and we may need to rethink if it's not.
>>> We have been trying to find soft dirty pages correctly on anon memory.
>>> Right now with UN_POPULATED, ASYNC and PAGEMAP_IOCTL, we'll be able to
>>> achieve this correctly.
>>>
>>> We want to track the file mapped memory the same way for another Windows
>>> api translation in which file memory will be tracked to find out soft-dirty
>>> pages. I believe once it is supported in the mm, filesystem doesn't matter
>>> as we'll be tracking memory mapped files.
>>
>> Some more elaboration would be great here.
> There is something called "writecopy" in Windows. Paul has given me some
> details.
> 
> @Paul can you share more details that:
> 1) What is writecopy?
> 2) If there is no documentation or documented behavior, Can you please
> share the current reference implementation in Wine even though it is buggy
> at the time? (I can implement correct implementation with this patch and my
> wip v11 pagemap_ioctl patches for testing purpose.)
Hi Peter,

Just had some offline discussion with Paul, lets leave this second use case
for future. We don't want to make discussions more entangled. There isn't
much clarity on it right now. Lets only complete the first use case.

The current patch seems good. Please can you post the next version by
including the missing part. Then I'll also be able to send the v11 of
pagemap_ioctl by basing on it.

> 
>>
>> For example, what is the other windows API?  What will you do with the
>> dirty information?  What is the impact of supporting only shmem/hugetlbfs
>> for "!anon"?
>>
>>>
>>> So we have use cases for both soft-dirty tracking on anon and file backed
>>> memories.
>>>
>>>>
>>>> For performance, one approach is probably making uffd-wp async separate
>>>> from other features, where we can revert the meaning of uffd-wp bit to
>>>> mimic what soft-dirty does (I think this will look closer to what David
>>>> mentioned in the other thread), by defining uffd-wp=1 as "written" and
>>>> uffd-wp=0 as clean.
>>> I'm happy with current state of things. WP Async is just a feature where
>>> faults are resolved automatically. The page is WP or not perfectly
>>> translates if page is written (dirty) or not with just not (!) operation.
>>> If we go and change the meanings of uffd_wp=1 as written, it would cause
>>> confusion and probably we'll need a lot more changes to add async feature
>>> which is right now quite small changeset.
>>
>> Yes, that's also why I wanted to keep it like this unless necessary.  It
>> makes all uffd features still more aligned with everything so more maintainable.
>>
>>>
>>>>
>>>> IIUC that'll make it one bit and work as fast as soft-dirty, meanwhile all
>>>> uffd-wp marker things can hopefully still be maintained.  However I really
>>>> don't like it to violate a lot of things, e.g., when UFFDIO_WRITEPROTECT
>>>> another round we'll need to DROP uffd-wp if async, but APPLY uffd-wp if
>>>> sync...  So in general it'll need more thoughts and slower to do.
>>> Sorry, I'm unable to understand your example here. Can you please elaborate?
>>
>> I think it's mostly what you stated above, the changeset will just get more
>> involved because of either the redefinition of uffd-wp depending on feature
>> enabled, or using yet another bit (e.g., if you still remember the very
>> original reply I was planning to reuse soft-dirty bit but used together
>> with VM_UFFD_WP; I think that resolves the degraded perf on unpopulated
>> memory too because it'll work more like soft-dirty).
>>
>> This is some detail which, IMHO, we can leave that for later.  To me I'm
>> more curious on knowing more details of your use case, and how uffd could
>> suite that the best way it could, assuming that (or part of it like the
>> unpopulate work here) can also be leveraged by others.  Your answers to
>> above questions may help.
>>
>> Thanks,
>>
>