Messages in this thread | | | Date | Wed, 30 Nov 2022 13:10:53 +0100 | Subject | Re: [PATCH v6 0/3] Implement IOCTL to get and/or the clear info about PTEs | From | David Hildenbrand <> |
| |
On 30.11.22 12:42, Muhammad Usama Anjum wrote: > On 11/21/22 8:55 PM, David Hildenbrand wrote: >> 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 >> > Thank you so much for sharing the issue and reproducer. > > After remapping the second part of the memory and m-protecting + > m-unprotecting the whole memory, the PTE of the first half of the memory > doesn't get marked as soft dirty even after writing multiple times to it. > Even if soft-dirtiness is cleared on the whole process, the PTE of the > first half memory doesn't get dirty. This seems like more of a bug in > mprotect. The mprotect should not mess up with the soft-dirty flag in the PTEs. > > I'm debugging this. I hope to find the issue soon. Soft-dirty tracking in > PTEs should be working correctly irrespective of the VM_SOFTDIRTY is set or > not on the VMA.
No, it's not a bug and these are not the VM_SOFTDIRTY semantics -- just because you think they should be like this. As people explained, VM_SOFTDIRTY implies *until now* that any PTE is consideres softdirty. And there are other scenarios that can similarly trigger something like that, besides mprotect().
Sorry if I sound annoyed, but please
1) factor out that from your patch set for now 2) find a way to handle this cleanly, for example, not merging VMAs that differ in VM_SOFTDIRTY
-- Thanks,
David / dhildenb
| |