[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a90ee936-67a9-340d-bf2c-2f331617b0da@redhat.com>
Date: Mon, 14 Nov 2022 16:46:18 +0100
From: David Hildenbrand <david@...hat.com>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>,
Michał Mirosław <emmir@...gle.com>,
Andrei Vagin <avagin@...il.com>,
Danylo Mocherniuk <mdanylo@...gle.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>,
Paul Gofman <pgofman@...eweavers.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>,
Greg KH <gregkh@...uxfoundation.org>,
Christian Brauner <brauner@...nel.org>,
Peter Xu <peterx@...hat.com>, Yang Shi <shy828301@...il.com>,
Vlastimil Babka <vbabka@...e.cz>,
Zach O'Keefe <zokeefe@...gle.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>,
Dan Williams <dan.j.williams@...el.com>, kernel@...labora.com,
Gabriel Krisman Bertazi <krisman@...labora.com>,
Peter Enderborg <peter.enderborg@...y.com>,
"open list : KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>, Shuah Khan <shuah@...nel.org>,
open list <linux-kernel@...r.kernel.org>,
"open list : PROC FILESYSTEM" <linux-fsdevel@...r.kernel.org>,
"open list : MEMORY MANAGEMENT" <linux-mm@...ck.org>
Subject: Re: [PATCH v6 0/3] Implement IOCTL to get and/or the clear info about
PTEs
> 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.
These are, AFAIU, the current semantics, and I am not sure if we want
user space to explicitly work around that.
>>
>>
>> I'd suggest moving forward without this controversial
>> PAGEMAP_NO_REUSED_REGIONS functionality for now, and preparing it as a
>> clear add-on we can discuss separately.Like I've described above, I've only added this flag to not get the
> non-dirty pages as dirty. Can there be some alternative to adding this
> flag? Please suggest.
Please split it out into a separate patch for now. We can discuss
further what the semantics are and if there are better alternatives for
that. In the meantime, you could move forward without
PAGEMAP_NO_REUSED_REGIONS while we are discussing them further.
--
Thanks,
David / dhildenb
Powered by blists - more mailing lists