Re: [PATCH v1 2/5] userfaultfd: introduce access-likely mode for common operations

[Nadav Amit <namit@vmware.com>]

On Jun 23, 2022, at 4:24 PM, Peter Xu <peterx@redhat.com> wrote:

> ⚠ External Email
> 
> On Wed, Jun 22, 2022 at 11:50:35AM -0700, Nadav Amit wrote:
>> From: Nadav Amit <namit@vmware.com>
>> 
>> Using a PTE on x86 with cleared access-bit (aka young-bit)
>> takes ~600 cycles more than when the access bit is set. At the same
>> time, setting the access-bit for memory that is not used (e.g.,
>> prefetched) can introduce greater overheads, as the prefetched memory is
>> reclaimed later than it should be.
>> 
>> Userfaultfd currently does not set the access-bit (excluding the
>> huge-pages case). Arguably, it is best to let the user control whether
>> the access bit should be set or not. The expected use is to request
>> userfaultfd to set the access-bit when the copy/wp operation is done to
>> resolve a page-fault, and not to set the access-bit when the memory is
>> prefetched.
>> 
>> Introduce UFFDIO_[op]_ACCESS_LIKELY to enable userspace to request the
>> young bit to be set.
>> 
>> Cc: Mike Kravetz <mike.kravetz@oracle.com>
>> Cc: Hugh Dickins <hughd@google.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Axel Rasmussen <axelrasmussen@google.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>> Signed-off-by: Nadav Amit <namit@vmware.com>
> 
> Hmm.. is the hugetlb code overlooked (for both of the hints), or maybe I
> missed it? Do we need to cover them too? Thanks,

I do not know what the value of not setting the PTE’s access/dirty when it
comes to performance. The pages won’t be swapped out, just as you wrote in
your comment in hugetlb_mcopy_atomic_pte():

        /*
         * Always mark UFFDIO_COPY page dirty; note that this may not be
         * extremely important for hugetlbfs for now since swapping is not
         * supported, but we should still be clear in that this page cannot be
         * thrown away at will, even if write bit not set.
         */
        _dst_pte = huge_pte_mkdirty(_dst_pte);
        _dst_pte = pte_mkyoung(_dst_pte);

If you want for consistency/robustness not to set dirty on read-only
entries, that’s something that I can do.