Re: [PATCH v6 0/3] Implement IOCTL to get and/or the clear info about PTEs

From: David Hildenbrand
Date: Mon Nov 21 2022 - 10:57:11 EST

On 21.11.22 16:00, Muhammad Usama Anjum wrote:
Hello,

Thank you for replying.

On 11/14/22 8:46 PM, David Hildenbrand wrote:
The soft-dirtiness is stored in the PTE. VMA is marked dirty to store the
dirtiness for reused regions. Clearing the soft-dirty status of whole
process is straight forward. When we want to clear/monitor the
soft-dirtiness of a part of the virtual memory, there is a lot of internal
noise. We don't want the non-dirty pages to become dirty because of how the
soft-dirty feature has been working. Soft-dirty feature wasn't being used
the way we want to use now. While monitoring a part of memory, it is not
acceptable to get non-dirty pages as dirty. Non-dirty pages become dirty
when the two VMAs are merged without considering if they both are dirty or
not (34228d473efe). To monitor changes over the memory, sometimes VMAs are
split to clear the soft-dirty bit in the VMA flags. But sometimes kernel
decide to merge them backup. It is so waste of resources.

Maybe you'd want a per-process option to not merge if the VM_SOFTDIRTY
property differs. But that might be just one alternative for handling this
case.


To keep things consistent, the default behavior of the IOCTL is to output
even the extra non-dirty pages as dirty from the kernel noise. A optional
PAGEMAP_NO_REUSED_REGIONS flag is added for those use cases which aren't
tolerant of extra non-dirty pages. This flag can be considered as something
which is by-passing the already present buggy implementation in the kernel.
It is not buggy per say as the issue can be solved if we don't allow the
two VMA which have different soft-dirty bits to get merged. But we are
allowing that so that the total number of VMAs doesn't increase. This was
acceptable at the time, but now with the use case of monitoring a part of
memory for soft-dirty doesn't want this merging. So either we need to
revert 34228d473efe and PAGEMAP_NO_REUSED_REGIONS flag will not be needed
or we should allow PAGEMAP_NO_REUSED_REGIONS or similar mechanism to ignore
the extra dirty pages which aren't dirty in reality.

When PAGEMAP_NO_REUSED_REGIONS flag is used, only the PTEs are checked to
find if the pages are dirty. So re-used regions cannot be detected. This
has the only side-effect of not checking the VMAs. So this is limitation of
using this flag which should be acceptable in the current state of code.
This limitation is okay for the users as they can clear the soft-dirty bit
of the VMA before starting to monitor a range of memory for soft-dirtiness.


Please separate that part out from the other changes; I am still not
convinced that we want this and what the semantical implications are.

Let's take a look at an example: can_change_pte_writable()

     /* Do we need write faults for softdirty tracking? */
     if (vma_soft_dirty_enabled(vma) && !pte_soft_dirty(pte))
         return false;

We care about PTE softdirty tracking, if it is enabled for the VMA.
Tracking is enabled if: vma_soft_dirty_enabled()

     /*
      * Soft-dirty is kind of special: its tracking is enabled when
      * the vma flags not set.
      */
     return !(vma->vm_flags & VM_SOFTDIRTY);

Consequently, if VM_SOFTDIRTY is set, we are not considering the soft_dirty
PTE bits accordingly.
Sorry, I'm unable to completely grasp the meaning of the example. We have
followed clear_refs_write() to write the soft-dirty bit clearing code in
the current patch. Dirtiness of the VMA and the PTE may be set
independently. Newer allocated memory has dirty bit set in the VMA. When
something is written the memory, the soft dirty bit is set in the PTEs as
well regardless if the soft dirty bit is set in the VMA or not.


Let me try to find a simple explanation:

After clearing a SOFTDIRTY PTE flag inside an area with VM_SOFTDIRTY set,
there are ways that PTE could get written to and it could become dirty,
without the PTE becoming softdirty.

Essentially, inside a VMA with VM_SOFTDIRTY set, the PTE softdirty values
might be stale: there might be entries that are softdirty even though the
PTE is *not* marked softdirty.
Can someone please share the example to reproduce this? In all of my
testing, even if I ignore VM_SOFTDIRTY and only base my decision of
soft-dirtiness on individual pages, it always passes.

Quick reproducer (the first and easiest one that triggered :) )
attached.

With no kernel changes, it works as expected.

# ./softdirty_mprotect


With the following kernel change to simulate what you propose it fails:

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index d22687d2e81e..f2c682bf7f64 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1457,8 +1457,8 @@ static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm,
flags |= PM_FILE;
if (page && !migration && page_mapcount(page) == 1)
flags |= PM_MMAP_EXCLUSIVE;
- if (vma->vm_flags & VM_SOFTDIRTY)
- flags |= PM_SOFT_DIRTY;
+ //if (vma->vm_flags & VM_SOFTDIRTY)
+ // flags |= PM_SOFT_DIRTY;
return make_pme(frame, flags);
}


# ./softdirty_mprotect
Page #1 should be softdirty

--
Thanks,

David / dhildenb
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdint.h>
#include <stdbool.h>
#include <sys/mman.h>

static size_t pagesize;
static int pagemap_fd;

static void clear_softdirty(void)
{
int fd = open("/proc/self/clear_refs", O_WRONLY);
const char *ctrl = "4";
int ret;

if (fd < 0) {
fprintf(stderr, "open() failed\n");
exit(1);
}
ret = write(fd, ctrl, strlen(ctrl));
close(fd);
if (ret != strlen(ctrl)) {
fprintf(stderr, "write() failed\n");
exit(1);
}
}

static uint64_t pagemap_get_entry(int fd, char *start)
{
const unsigned long pfn = (unsigned long)start / pagesize;
uint64_t entry;
int ret;

ret = pread(fd, &entry, sizeof(entry), pfn * sizeof(entry));
if (ret != sizeof(entry)) {
fprintf(stderr, "pread() failed\n");
exit(1);
}

return entry;
}

static bool pagemap_is_softdirty(int fd, char *start)
{
uint64_t entry = pagemap_get_entry(fd, start);

return entry & 0x0080000000000000ull;
}

void main(void)
{
char *mem, *mem2;

pagesize = getpagesize();
pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
if (pagemap_fd < 0) {
fprintf(stderr, "open() failed\n");
exit(1);
}

/* Map 2 pages. */
mem = mmap(0, 2 * pagesize, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANON, -1, 0);
if (mem == MAP_FAILED) {
fprintf(stderr, "mmap() failed\n");
exit(1);
}

/* Populate both pages. */
memset(mem, 1, 2 * pagesize);

if (!pagemap_is_softdirty(pagemap_fd, mem))
fprintf(stderr, "Page #1 should be softdirty\n");
if (!pagemap_is_softdirty(pagemap_fd, mem + pagesize))
fprintf(stderr, "Page #2 should be softdirty\n");

/*
* Start softdirty tracking. Clear VM_SOFTDIRTY and clear the softdirty
* PTE bit.
*/
clear_softdirty();

if (pagemap_is_softdirty(pagemap_fd, mem))
fprintf(stderr, "Page #1 should not be softdirty\n");
if (pagemap_is_softdirty(pagemap_fd, mem + pagesize))
fprintf(stderr, "Page #2 should not be softdirty\n");

/*
* Remap the second page. The VMA gets VM_SOFTDIRTY set. Both VMAs
* get merged such that the resulting VMA has VM_SOFTDIRTY set.
*/
mem2 = mmap(mem + pagesize, pagesize, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANON|MAP_FIXED, -1, 0);
if (mem2 == MAP_FAILED) {
fprintf(stderr, "mmap() failed\n");
exit(1);
}

/* Protect + unprotect. */
mprotect(mem, 2 * pagesize, PROT_READ);
mprotect(mem, 2 * pagesize, PROT_READ|PROT_WRITE);

/* Modify both pages. */
memset(mem, 2, 2 * pagesize);

if (!pagemap_is_softdirty(pagemap_fd, mem))
fprintf(stderr, "Page #1 should be softdirty\n");
if (!pagemap_is_softdirty(pagemap_fd, mem + pagesize))
fprintf(stderr, "Page #2 should be softdirty\n");
}