Re: [RFC PATCH 01/14] userfaultfd: set dirty and young on writeprotect

From: David Hildenbrand <david@redhat.com>
To: Nadav Amit <nadav.amit@gmail.com>
Cc: Peter Xu <peterx@redhat.com>, Linux MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will@kernel.org>, Yu Zhao <yuzhao@google.com>,
	Nick Piggin <npiggin@gmail.com>
Subject: Re: [RFC PATCH 01/14] userfaultfd: set dirty and young on writeprotect
Date: Thu, 21 Jul 2022 16:10:50 +0200	[thread overview]
Message-ID: <7a6b346a-9225-c6f4-aca4-93c588b6917e@redhat.com> (raw)
In-Reply-To: <8e2dcae3-5f50-cd59-7588-b8f566549ad4@redhat.com>

On 21.07.22 09:52, David Hildenbrand wrote:
>>> Yes. Especially for any MAP_PRIVATE mappings.
>>>
>>> If you want to write to something that's not mapped writable in a
>>> MAP_PRIVATE mapping it
>>> a) Has to be an exclusive anonymous page
>>> b) The pte has to be dirty
>>
>> Do you need both conditions to be true? I thought (a) is sufficient (if
>> the soft-dirty and related checks succeed).
> 
> If we force-write to a page, we need it to be dirty to tell reclaim code
> that the content stale. We can either mark the pte dirty manually, or
> just let the write fault handler deal with it to simplify GUP code. This
> needs some more thought, but that's my understanding.

Extending on my previous answer after staring at the code

a) I have to dig if the FOLL_FORCE special-retry-handling is required
for MAP_SHARED at all.

check_vma_flags() allows FOLL_FORCE only on MAP_PRIVATE VMAs that lack
VM_WRITE.

Consequently, I would have assumed that the first write fault should be
sufficient on a MAP_SHARED VMA to actually map the PTE writable and not
require any of that special retry magic.

b) I wonder if we have to take care of uffd-wp and softdirty (just like
in mprotect code here) as well in case we stumble over an exclusive
anonymous page. Yes, the VMA is currently not writable, but I'd have
expected at least softdirty tracking to apply.

... I'll dig into the details.

-- 
Thanks,

David / dhildenb