> On Jul 23, 2022, at 2:16 AM, Mike Rapoport <rppt@xxxxxxxxxxxxx> wrote:
>
> On Mon, Jul 18, 2022 at 04:47:45AM -0700, Nadav Amit wrote:
>> From: Nadav Amit <namit@xxxxxxxxxx>
>>
>> Introduce access-hints in userfaultfd. The expectation is that userspace
>> would set access-hints when a page-fault occurred on a page and would
>> not provide the access-hint on prefaulted memory. The exact behavior of
>> the kernel in regard to the hints would not be part of userfaultfd api.
>>
>> At this time the use of the access-hint is only in setting access-bit
>> similarly to the way it is done in do_set_pte(). In x86, currently PTEs
>> are always marked as young, including prefetched ones. But on arm64,
>> PTEs would be marked as old (when access bit is supported).
>>
>> If access hints are not enabled, the kernel would behave as if the
>> access-hint was provided for backward compatibility.
>>
>> Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
>> Cc: Hugh Dickins <hughd@xxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: Axel Rasmussen <axelrasmussen@xxxxxxxxxx>
>> Cc: Peter Xu <peterx@xxxxxxxxxx>
>> Cc: David Hildenbrand <david@xxxxxxxxxx>
>> Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
>> Signed-off-by: Nadav Amit <namit@xxxxxxxxxx>
>> ---
>> fs/userfaultfd.c | 39 ++++++++++++++++++++++++++++----
>> include/linux/userfaultfd_k.h | 1 +
>> include/uapi/linux/userfaultfd.h | 20 +++++++++++++++-
>> mm/internal.h | 13 +++++++++++
>> mm/memory.c | 12 ----------
>> mm/userfaultfd.c | 11 +++++++--
>> 6 files changed, 77 insertions(+), 19 deletions(-)
>>
>> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
>> index 2ae24327beec..8d8792b27c53 100644
>> --- a/fs/userfaultfd.c
>> +++ b/fs/userfaultfd.c
>> @@ -1708,13 +1708,21 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
>> ret = -EINVAL;
>> if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src)
>> goto out;
>> - if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP))
>> + if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP|
>> + UFFDIO_COPY_MODE_ACCESS_LIKELY))
>> goto out;
>>
>> mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP;
>>
>> uffd_flags = mode_wp ? UFFD_FLAGS_WP : UFFD_FLAGS_NONE;
>>
>> + if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
>> + if (uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY)
>> + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
>> + } else {
>> + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
>> + }
>> +
>
> This is quite a construct and it gets more complex in the following
> patches. How about making it to a static inline function?
Possible. There is another option though. I think it would have been
much cleaner if some flags were in common offsets in the different
“mode” fields. It might be too late for some fields (WP), but I can
put these the ACCESS/WRITE fields in the the high bits in fixed
place for all modes, which would allow to at least reuse the logic.
Is that ok?
