userfaultfd: support access/write hints

mm/userfaultfd.c +4 -2

+
 	dst_pte = pte_offset_map_lock(dst_mm, dst_pmd, dst_addr, &ptl);
 
 	if (vma_is_shmem(dst_vma)) {

@@ -202,7 +208,8 @@ static int mcopy_atomic_pte(struct mm_struct *dst_mm,
 static int mfill_zeropage_pte(struct mm_struct *dst_mm,
 			      pmd_t *dst_pmd,
 			      struct vm_area_struct *dst_vma,
-			      unsigned long dst_addr)
+			      unsigned long dst_addr,
+			      uffd_flags_t uffd_flags)
 {
 	pte_t _dst_pte, *dst_pte;
 	spinlock_t *ptl;

@@ -495,7 +502,7 @@ static __always_inline ssize_t mfill_atomic_pte(struct mm_struct *dst_mm,
 					       uffd_flags);
 		else
 			err = mfill_zeropage_pte(dst_mm, dst_pmd,
-						 dst_vma, dst_addr);
+						 dst_vma, dst_addr, uffd_flags);
 	} else {
 		err = shmem_mfill_atomic_pte(dst_mm, dst_pmd, dst_vma,
 					     dst_addr, src_addr,

Signature · 1 line

2.25.1

⚠ External Email

include/uapi/linux/userfaultfd.h struct uffdio_copy { · line 275

Quoted 5 lines · Nadav Amit

include/uapi/linux/userfaultfd.h +1 -0

@@ -261,6 +272,7 @@ struct uffdio_copy {
 struct uffdio_zeropage {
     struct uffdio_range range;
 #define UFFDIO_ZEROPAGE_MODE_DONTWAKE                ((__u64)1<<0)
+#define UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY   ((__u64)1<<1)

Would access hint help zeropage use case? I remembered you used to comment around and said it won't help since we won't reclaim zero page anyway.

It won't help either even if this flag is only used for the follow up WRITE_HINT (since then there'll be a CoW) because when WRITE_HINT attached it doesn't make sense to not have ACCESS_HINT, then it seems the WRITE_HINT itself would be enough for ZEROPAGE to me.

[...]

mm/userfaultfd.c int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, · line 68

Quoted 9 lines

mm/userfaultfd.c +1 -0

index 421784d26651..c15679f3eb6a 100644
@@ -65,6 +65,7 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
     bool writable = dst_vma->vm_flags & VM_WRITE;
     bool vm_shared = dst_vma->vm_flags & VM_SHARED;
     bool page_in_cache = page->mapping;
+     bool prefault = !(uffd_flags & UFFD_FLAGS_ACCESS_LIKELY);

I think it's okay to name it "prefault" as a temp var, but ideally IMHO we shouldn't assume what the user app is doing - it is only installing some uffd pgtables with !ACCESS_LIKELY and it does not necessarily need to be a prefault process..

Quoted 7 lines

mm/userfaultfd.c +0 -0

     spinlock_t *ptl;
     struct inode *inode;
     pgoff_t offset, max_off;

@@ -92,6 +93,11 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
              */
             _dst_pte = pte_wrprotect(_dst_pte);
 

mm/userfaultfd.c int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, · line 99

Quoted 4 lines

mm/userfaultfd.c +4 -0

+     if (prefault && arch_wants_old_prefaulted_pte())
+             _dst_pte = pte_mkold(_dst_pte);
+     else
+             _dst_pte = pte_sw_mkyoung(_dst_pte);

Could you explain why we couldn't unconditionally mkold here even for x86?

It'll be a pity if this feature bit will only be useful on arm64 but not covering x86 (which is so far still the majority I think).

IMHO it's slightly different here comparing to kernel prefaults - the uesr app may not be aware of kernel prefaults, but here !ACCESS_HINT it's user-aware, and it's what user app explicitly provided. IMO it's a stronger proof of a cold page already.

The other thing I got confused here is arch_wants_old_prefaulted_pte() returns true if arm64 supports hardware AF. However for all the rest archs (including x86_64 which, afaict, support AF too in most models) it'll constantly return false. Do you know what's the rational behind?

500 cycles; this actually affected UnixBench when people tried to change

Quoted 11 lines · Peter Xu

⚠ External Email

include/uapi/linux/userfaultfd.h struct uffdio_copy { · line 275

Quoted 5 lines · Nadav Amit

include/uapi/linux/userfaultfd.h +1 -0

@@ -261,6 +272,7 @@ struct uffdio_copy {
 struct uffdio_zeropage {
     struct uffdio_range range;
 #define UFFDIO_ZEROPAGE_MODE_DONTWAKE                ((__u64)1<<0)
+#define UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY   ((__u64)1<<1)

Would access hint help zeropage use case? I remembered you used to comment around and said it won't help since we won't reclaim zero page anyway.

I agree that there is no meaning for access bit on zero page. I just think that it is best to have the flags for consistency. If you ask me, I would prefer to have all the flags in a fixed place (highest bits?). Anyhow, if we expose the hints as a feature, I do not think we would later want to say “here is another feature that enables another hint that we thought is not needed before”. Userfaultfd’s feature bits are already nuts, IMHO.

Quoted 4 lines

It won't help either even if this flag is only used for the follow up WRITE_HINT (since then there'll be a CoW) because when WRITE_HINT attached it doesn't make sense to not have ACCESS_HINT, then it seems the WRITE_HINT itself would be enough for ZEROPAGE to me.

Agreed. Again, I think it is worthy for consistency.

Quoted 25 lines

[...]

mm/userfaultfd.c int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, · line 68

Quoted 9 lines

mm/userfaultfd.c +1 -0

index 421784d26651..c15679f3eb6a 100644
@@ -65,6 +65,7 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
     bool writable = dst_vma->vm_flags & VM_WRITE;
     bool vm_shared = dst_vma->vm_flags & VM_SHARED;
     bool page_in_cache = page->mapping;
+     bool prefault = !(uffd_flags & UFFD_FLAGS_ACCESS_LIKELY);

I think it's okay to name it "prefault" as a temp var, but ideally IMHO we shouldn't assume what the user app is doing - it is only installing some uffd pgtables with !ACCESS_LIKELY and it does not necessarily need to be a prefault process..

Quoted 7 lines

mm/userfaultfd.c +0 -0

     spinlock_t *ptl;
     struct inode *inode;
     pgoff_t offset, max_off;

@@ -92,6 +93,11 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
              */
             _dst_pte = pte_wrprotect(_dst_pte);
 

Quoted 6 lines

mm/userfaultfd.c int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, · line 99

Quoted 4 lines

mm/userfaultfd.c +4 -0

+     if (prefault && arch_wants_old_prefaulted_pte())
+             _dst_pte = pte_mkold(_dst_pte);
+     else
+             _dst_pte = pte_sw_mkyoung(_dst_pte);

Could you explain why we couldn't unconditionally mkold here even for x86?

To answer this question and the previous one, please note that the logic is “borrowed” from do_set_pte(). If you want me to refactor and extract a function, please let me know.

Here is the deal: for x86, we don’t do pte_mkold() because setting the access bit is expensive (>500 cycles). For arm64 that have access-bit we don’t since (according to arm64 code or commit log), the cost of setting the access bit on arm is low.

Quoted 3 lines

It'll be a pity if this feature bit will only be useful on arm64 but not covering x86 (which is so far still the majority I think).

Quoted 4 lines

IMHO it's slightly different here comparing to kernel prefaults - the uesr app may not be aware of kernel prefaults, but here !ACCESS_HINT it's user-aware, and it's what user app explicitly provided. IMO it's a stronger proof of a cold page already.

I’m ok with that if that is your choice. I actually prefer to give userspace more control, but I tried to be consistent with other parts of the kernel.

Having said that, it’s really hard for me to see why young bit would be clear, but dirty bit would be set...

Quoted 5 lines

The other thing I got confused here is arch_wants_old_prefaulted_pte() returns true if arm64 supports hardware AF. However for all the rest archs (including x86_64 which, afaict, support AF too in most models) it'll constantly return false. Do you know what's the rational behind?

All x86 (32/64) since 386 support access-bit in the page-tables (IIRC, 286 had access bit in the segments).

I thought we discussed it before: if you access an old PTE on x86, you pay

Quoted 1 line

500 cycles; this actually affected UnixBench when people tried to change

this behavior [1]. In contrast, on arm64, which I have never profiled, you probably saw the comment saying: "Experimentally, it's cheap to set the access flag in hardware and we benefit from prefaulting mappings as 'old’ to start with.”.

I do not know what happens on other architectures.

( sorry if I have some repetitions in this email )

[1] https://marc.info/?l=linux-kernel&m=146582237922378&w=2

From: Nadav Amit <[email protected]>

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 <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 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(-)

fs/userfaultfd.c +2 -1

index 2ae24327beec..8d8792b27c53 100644
@@ -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;

fs/userfaultfd.c +9 -1

 	if (mmget_not_zero(ctx->mm)) {
 		ret = mcopy_atomic(ctx->mm, uffdio_copy.dst, uffdio_copy.src,
 				   uffdio_copy.len, &ctx->mmap_changing,

@@ -1765,9 +1773,17 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
 	if (ret)
 		goto out;
 	ret = -EINVAL;
-	if (uffdio_zeropage.mode & ~UFFDIO_ZEROPAGE_MODE_DONTWAKE)
+	if (uffdio_zeropage.mode & ~(UFFDIO_ZEROPAGE_MODE_DONTWAKE|
+				     UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY))
 		goto out;
 
+	if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
+		if (uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY)
+			uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+	} else {
+		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+	}
+
 	if (mmget_not_zero(ctx->mm)) {
 		ret = mfill_zeropage(ctx->mm, uffdio_zeropage.range.start,
 				     uffdio_zeropage.range.len,

Quoted 49 lines · Nadav Amit

From: Nadav Amit <[email protected]>

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 <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---

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(-)

fs/userfaultfd.c +2 -1

index 2ae24327beec..8d8792b27c53 100644
@@ -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;
 

fs/userfaultfd.c static int userfaultfd_copy(struct userfaultfd_ctx *ctx, · line 1725

Quoted 7 lines

fs/userfaultfd.c +7 -0

+	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?

Quoted 53 lines · Mike Rapoport

Quoted 49 lines · Nadav Amit

From: Nadav Amit <[email protected]>

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 <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---

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(-)

fs/userfaultfd.c +2 -1

index 2ae24327beec..8d8792b27c53 100644
@@ -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;
 

Quoted 10 lines

fs/userfaultfd.c static int userfaultfd_copy(struct userfaultfd_ctx *ctx, · line 1725

Quoted 7 lines

fs/userfaultfd.c +7 -0

+	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?

From: Nadav Amit <[email protected]>

Introduce write-likely hints for uffd. These hints would be used in a future patch to decide whether to attempt to map pages in the page-table or whether to only mark them logically as writable. This allows userspace to determine whether a page would be accessed faster or whether removal of the page would be possible, potentially, without writeback and TLB flush.

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 fs/userfaultfd.c                 | 32 ++++++++++++++++++++++++--------
 include/linux/userfaultfd_k.h    |  1 +
 include/uapi/linux/userfaultfd.h | 13 ++++++++++++-
 3 files changed, 37 insertions(+), 9 deletions(-)

fs/userfaultfd.c +24 -8

index 8d8792b27c53..3027d228550a 100644
@@ -1709,7 +1709,8 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 	if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src)
 		goto out;
 	if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP|
-				 UFFDIO_COPY_MODE_ACCESS_LIKELY))
+				 UFFDIO_COPY_MODE_ACCESS_LIKELY|
+				 UFFDIO_COPY_MODE_WRITE_LIKELY))
 		goto out;
 
 	mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP;

@@ -1719,8 +1720,11 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 	if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
 		if (uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY)
 			uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		if (uffdio_copy.mode & UFFDIO_COPY_MODE_WRITE_LIKELY)
+			uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
 	} else {
-		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+			      UFFD_FLAGS_WRITE_LIKELY;
 	}
 
 	if (mmget_not_zero(ctx->mm)) {

@@ -1774,14 +1778,18 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
 		goto out;
 	ret = -EINVAL;
 	if (uffdio_zeropage.mode & ~(UFFDIO_ZEROPAGE_MODE_DONTWAKE|
-				     UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY))
+				     UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY|
+				     UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY))
 		goto out;
 
 	if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
 		if (uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY)
 			uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		if (uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY)
+			uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
 	} else {
-		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+			      UFFD_FLAGS_WRITE_LIKELY;
 	}
 
 	if (mmget_not_zero(ctx->mm)) {

@@ -1834,7 +1842,8 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
 
 	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
 			       UFFDIO_WRITEPROTECT_MODE_WP |
-			       UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY))
+			       UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY |
+			       UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY))
 		return -EINVAL;
 
 	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;

@@ -1847,8 +1856,11 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
 	if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
 		if (uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY)
 			uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		if (uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY)
+			uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
 	} else {
-		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+			      UFFD_FLAGS_WRITE_LIKELY;
 	}
 
 	if (mmget_not_zero(ctx->mm)) {

@@ -1903,14 +1915,18 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
 		goto out;
 	}
 	if (uffdio_continue.mode & ~(UFFDIO_CONTINUE_MODE_DONTWAKE|
-				     UFFDIO_CONTINUE_MODE_ACCESS_LIKELY))
+				     UFFDIO_CONTINUE_MODE_ACCESS_LIKELY|
+				     UFFDIO_CONTINUE_MODE_WRITE_LIKELY))
 		goto out;
 
 	if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
 		if (uffdio_continue.mode & UFFDIO_CONTINUE_MODE_ACCESS_LIKELY)
 			uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		if (uffdio_continue.mode & UFFDIO_CONTINUE_MODE_WRITE_LIKELY)
+			uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
 	} else {
-		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+		uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+			      UFFD_FLAGS_WRITE_LIKELY;
 	}
 
 	if (mmget_not_zero(ctx->mm)) {

include/linux/userfaultfd_k.h +1 -0

index b326798b5677..4968c86938b2 100644
@@ -60,6 +60,7 @@ typedef unsigned int __bitwise uffd_flags_t;
 #define UFFD_FLAGS_NONE			((__force uffd_flags_t)0)
 #define UFFD_FLAGS_WP			((__force uffd_flags_t)BIT(0))
 #define UFFD_FLAGS_ACCESS_LIKELY	((__force uffd_flags_t)BIT(1))
+#define UFFD_FLAGS_WRITE_LIKELY		((__force uffd_flags_t)BIT(2))
 
 extern int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
 				    struct vm_area_struct *dst_vma,

include/uapi/linux/userfaultfd.h +11 -1

index 02e0c1f56939..f52cbe4c9c44 100644
@@ -202,7 +202,7 @@ struct uffdio_api {
 	 * write-protection mode is supported on both shmem and hugetlbfs.
 	 *
 	 * UFFD_FEATURE_ACCESS_HINTS indicates that the ioctl operations
-	 * support the UFFDIO_*_MODE_ACCESS_LIKELY hints.
+	 * support the UFFDIO_*_MODE_[ACCESS|WRITE]_LIKELY hints.
 	 */
 #define UFFD_FEATURE_PAGEFAULT_FLAG_WP		(1<<0)
 #define UFFD_FEATURE_EVENT_FORK			(1<<1)

@@ -257,9 +257,13 @@ struct uffdio_copy {
 	 * page is likely to be access in the near future. Providing the hint
 	 * properly can improve performance.
 	 *
+	 * UFFDIO_COPY_MODE_WRITE_LIKELY provides a hint to the kernel that the
+	 * page is likely to be written in the near future. Providing the hint
+	 * properly can improve performance.
 	 */
 #define UFFDIO_COPY_MODE_WP			((__u64)1<<1)
 #define UFFDIO_COPY_MODE_ACCESS_LIKELY		((__u64)1<<2)
+#define UFFDIO_COPY_MODE_WRITE_LIKELY		((__u64)1<<3)
 	__u64 mode;
 
 	/*

@@ -273,6 +277,7 @@ struct uffdio_zeropage {
 	struct uffdio_range range;
 #define UFFDIO_ZEROPAGE_MODE_DONTWAKE		((__u64)1<<0)
 #define UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY	((__u64)1<<1)
+#define UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY	((__u64)1<<2)
 	__u64 mode;
 
 	/*

@@ -296,6 +301,10 @@ struct uffdio_writeprotect {
  * that the page is likely to be access in the near future. Providing
  * the hint properly can improve performance.
  *
+ * UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY: provides a hint to the kernel
+ * that the page is likely to be written in the near future. Providing
+ * the hint properly can improve performance.
+ *
  * NOTE: Write protecting a region (WP=1) is unrelated to page faults,
  * therefore DONTWAKE flag is meaningless with WP=1.  Removing write
  * protection (WP=0) in response to a page fault wakes the faulting

@@ -304,6 +313,7 @@ struct uffdio_writeprotect {
 #define UFFDIO_WRITEPROTECT_MODE_WP		((__u64)1<<0)
 #define UFFDIO_WRITEPROTECT_MODE_DONTWAKE	((__u64)1<<1)
 #define UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY	((__u64)1<<2)
+#define UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY	((__u64)1<<3)
 	__u64 mode;
 };
 

⚠ External Email

Quoted 182 lines · Nadav Amit

From: Nadav Amit <[email protected]>

Introduce write-likely hints for uffd. These hints would be used in a future patch to decide whether to attempt to map pages in the page-table or whether to only mark them logically as writable. This allows userspace to determine whether a page would be accessed faster or whether removal of the page would be possible, potentially, without writeback and TLB flush.

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
fs/userfaultfd.c | 32 ++++++++++++++++++++++++-------- include/linux/userfaultfd_k.h | 1 + include/uapi/linux/userfaultfd.h | 13 ++++++++++++-

3 files changed, 37 insertions(+), 9 deletions(-)

fs/userfaultfd.c +0 -0

index 8d8792b27c53..3027d228550a 100644
@@ -1709,7 +1709,8 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src)
 goto out;

if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP|
- UFFDIO_COPY_MODE_ACCESS_LIKELY))
+ UFFDIO_COPY_MODE_ACCESS_LIKELY|
+ UFFDIO_COPY_MODE_WRITE_LIKELY))
goto out;

mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP;

fs/userfaultfd.c +4 -1

@@ -1719,8 +1720,11 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
 if (uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY)
 uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ if (uffdio_copy.mode & UFFDIO_COPY_MODE_WRITE_LIKELY)
+ uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
 } else {
- uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+ UFFD_FLAGS_WRITE_LIKELY;
 }
 
 if (mmget_not_zero(ctx->mm)) {

@@ -1774,14 +1778,18 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
 goto out;
 ret = -EINVAL;

if (uffdio_zeropage.mode & ~(UFFDIO_ZEROPAGE_MODE_DONTWAKE|
- UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY))
+ UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY|
+ UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY))
goto out;

if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
if (uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY) uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ if (uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY)
+ uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
} else {
- uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+ UFFD_FLAGS_WRITE_LIKELY;
}

if (mmget_not_zero(ctx->mm)) {

fs/userfaultfd.c +0 -0

@@ -1834,7 +1842,8 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
 

if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE | UFFDIO_WRITEPROTECT_MODE_WP |
- UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY))
+ UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY |
+ UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY))
return -EINVAL;

mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;

fs/userfaultfd.c +4 -1

@@ -1847,8 +1856,11 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
 if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
 if (uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY)
 uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ if (uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY)
+ uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
 } else {
- uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+ UFFD_FLAGS_WRITE_LIKELY;
 }
 
 if (mmget_not_zero(ctx->mm)) {

@@ -1903,14 +1915,18 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
 goto out;
 }

if (uffdio_continue.mode & ~(UFFDIO_CONTINUE_MODE_DONTWAKE|
- UFFDIO_CONTINUE_MODE_ACCESS_LIKELY))
+ UFFDIO_CONTINUE_MODE_ACCESS_LIKELY|
+ UFFDIO_CONTINUE_MODE_WRITE_LIKELY))
goto out;

if (ctx->features & UFFD_FEATURE_ACCESS_HINTS) {
if (uffdio_continue.mode & UFFDIO_CONTINUE_MODE_ACCESS_LIKELY) uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ if (uffdio_continue.mode & UFFDIO_CONTINUE_MODE_WRITE_LIKELY)
+ uffd_flags |= UFFD_FLAGS_WRITE_LIKELY;
} else {
- uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY;
+ uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY |
+ UFFD_FLAGS_WRITE_LIKELY;
}

if (mmget_not_zero(ctx->mm)) {

include/linux/userfaultfd_k.h +1 -0

index b326798b5677..4968c86938b2 100644
@@ -60,6 +60,7 @@ typedef unsigned int __bitwise uffd_flags_t;
 #define UFFD_FLAGS_NONE ((__force uffd_flags_t)0)
 #define UFFD_FLAGS_WP ((__force uffd_flags_t)BIT(0))
 #define UFFD_FLAGS_ACCESS_LIKELY ((__force uffd_flags_t)BIT(1))
+#define UFFD_FLAGS_WRITE_LIKELY ((__force uffd_flags_t)BIT(2))
 
 extern int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
 struct vm_area_struct *dst_vma,

include/uapi/linux/userfaultfd.h +11 -1

index 02e0c1f56939..f52cbe4c9c44 100644
@@ -202,7 +202,7 @@ struct uffdio_api {
 * write-protection mode is supported on both shmem and hugetlbfs.
 *
 * UFFD_FEATURE_ACCESS_HINTS indicates that the ioctl operations
- * support the UFFDIO_*_MODE_ACCESS_LIKELY hints.
+ * support the UFFDIO_*_MODE_[ACCESS|WRITE]_LIKELY hints.
 */
 #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0)
 #define UFFD_FEATURE_EVENT_FORK (1<<1)

@@ -257,9 +257,13 @@ struct uffdio_copy {
 * page is likely to be access in the near future. Providing the hint
 * properly can improve performance.
 *
+ * UFFDIO_COPY_MODE_WRITE_LIKELY provides a hint to the kernel that the
+ * page is likely to be written in the near future. Providing the hint
+ * properly can improve performance.
 */
 #define UFFDIO_COPY_MODE_WP ((__u64)1<<1)
 #define UFFDIO_COPY_MODE_ACCESS_LIKELY ((__u64)1<<2)
+#define UFFDIO_COPY_MODE_WRITE_LIKELY ((__u64)1<<3)
 __u64 mode;
 
 /*

@@ -273,6 +277,7 @@ struct uffdio_zeropage {
 struct uffdio_range range;
 #define UFFDIO_ZEROPAGE_MODE_DONTWAKE ((__u64)1<<0)
 #define UFFDIO_ZEROPAGE_MODE_ACCESS_LIKELY ((__u64)1<<1)
+#define UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY ((__u64)1<<2)
 __u64 mode;
 
 /*

@@ -296,6 +301,10 @@ struct uffdio_writeprotect {
 * that the page is likely to be access in the near future. Providing
 * the hint properly can improve performance.
 *
+ * UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY: provides a hint to the kernel
+ * that the page is likely to be written in the near future. Providing
+ * the hint properly can improve performance.
+ *
 * NOTE: Write protecting a region (WP=1) is unrelated to page faults,
 * therefore DONTWAKE flag is meaningless with WP=1. Removing write
 * protection (WP=0) in response to a page fault wakes the faulting

@@ -304,6 +313,7 @@ struct uffdio_writeprotect {
 #define UFFDIO_WRITEPROTECT_MODE_WP ((__u64)1<<0)
 #define UFFDIO_WRITEPROTECT_MODE_DONTWAKE ((__u64)1<<1)
 #define UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY ((__u64)1<<2)
+#define UFFDIO_WRITEPROTECT_MODE_WRITE_LIKELY ((__u64)1<<3)
 __u64 mode;
 };
 

include/uapi/linux/userfaultfd.h struct uffdio_continue { · line 325

Quoted 5 lines

include/uapi/linux/userfaultfd.h +1 -0

@@ -311,6 +321,7 @@ struct uffdio_continue {
 struct uffdio_range range;
 #define UFFDIO_CONTINUE_MODE_DONTWAKE ((__u64)1<<0)
 #define UFFDIO_CONTINUE_MODE_ACCESS_LIKELY ((__u64)1<<1)
+#define UFFDIO_CONTINUE_MODE_WRITE_LIKELY ((__u64)1<<2)
 __u64 mode;

I thought you would have some reasoning on having the flag for unprotect (since our last discussion you mentioned it) but it seems not there..

Then, could we only keep the zeropage write hint but drop the rest? They're never used in this whole series besides the zeropage one, meanwhile I think we're still not reaching consensus on whether they'll be helpful?

Quoted 10 lines

include/uapi/linux/userfaultfd.h struct uffdio_continue { · line 325

Quoted 6 lines

include/uapi/linux/userfaultfd.h +1 -0

@@ -311,6 +321,7 @@ struct uffdio_continue {
 struct uffdio_range range;
 #define UFFDIO_CONTINUE_MODE_DONTWAKE ((__u64)1<<0)
 #define UFFDIO_CONTINUE_MODE_ACCESS_LIKELY ((__u64)1<<1)
+#define UFFDIO_CONTINUE_MODE_WRITE_LIKELY ((__u64)1<<2)
 __u64 mode;

I thought you would have some reasoning on having the flag for unprotect (since our last discussion you mentioned it) but it seems not there..

Quoted 3 lines

Then, could we only keep the zeropage write hint but drop the rest? They're never used in this whole series besides the zeropage one, meanwhile I think we're still not reaching consensus on whether they'll be helpful?

I think that I didn’t communicate clearly enough two things. First, the access flags are used here.

Now, you are correct that although the unprotect flag is defined here, it is not used in this patch-set. There is a reason for that.

It turns out that using David’s work to map a writable page can cause undesired behaviors - the clean PTE, which we discussed, and additional TLB shootdowns. Since it required a lot of changes to get rid of these additional shootdowns, I put the unprotect changes in a different patch-set.

https://lore.kernel.org/all/[email protected]/

Let me know if that answers your question.

From: Nadav Amit <[email protected]>

When userfaultfd provides a zeropage in response to ioctl, it provides a readonly alias to the zero page. If the page is later written (which is the likely scenario), page-fault occurs and the page-fault allocator allocates a page and rewires the page-tables.

This is an expensive flow for cases in which a page is likely be written to. Users can use the copy ioctl to initialize zero page (by copying zeros), but this is also wasteful.

Allow userfaultfd users to efficiently map initialized zero-pages that are writable. IF UFFDIO_ZEROPAGE_MODE_WRITE_LIKELY is provided would map a clear page instead of an alias to the zero page.

Suggested-by David Hildenbrand <[email protected]>

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Mike Rapoport <[email protected]>

Acked-by Peter Xu <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 mm/userfaultfd.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

mm/userfaultfd.c +31 -0

index c15679f3eb6a..954c6980b29f 100644
@@ -241,6 +241,37 @@ static int mfill_zeropage_pte(struct mm_struct *dst_mm,
 	return ret;
 }
 
+static int mfill_clearpage_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
+			       struct vm_area_struct *dst_vma,
+			       unsigned long dst_addr,
+			       uffd_flags_t uffd_flags)
+{
+	struct page *page;
+	int ret;
+
+	ret = -ENOMEM;
+	page = alloc_zeroed_user_highpage_movable(dst_vma, dst_addr);
+	if (!page)
+		goto out;
+
+	/* The PTE is not marked as dirty unconditionally */
+	SetPageDirty(page);
+	__SetPageUptodate(page);
+
+	if (mem_cgroup_charge(page_folio(page), dst_vma->vm_mm, GFP_KERNEL))
+		goto out_release;
+
+	ret = mfill_atomic_install_pte(dst_mm, dst_pmd, dst_vma, dst_addr,
+				       page, true, uffd_flags);
+	if (ret)
+		goto out_release;
+out:
+	return ret;
+out_release:
+	put_page(page);
+	goto out;
+}
+
 /* Handles UFFDIO_CONTINUE for all shmem VMAs (shared or private). */
 static int mcontinue_atomic_pte(struct mm_struct *dst_mm,
 				pmd_t *dst_pmd,

From: Nadav Amit <[email protected]>

As the next patches are going to introduce more information that needs to be propagated regarding handled user requests, introduce uffd_flags that would be used to propagate this information.

Remove the unused UFFD_FLAGS_SET to avoid confusion in the constant names.

Introducing uffd flags also allows to avoid mm/userfaultfd from being using uapi (e.g., UFFDIO_COPY_MODE_WP).

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc Mike Rapoport <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

From: Nadav Amit <[email protected]>

As the next patches are going to introduce more information that needs to be propagated regarding handled user requests, introduce uffd_flags that would be used to propagate this information.

Remove the unused UFFD_FLAGS_SET to avoid confusion in the constant names.

Introducing uffd flags also allows to avoid mm/userfaultfd from being using uapi (e.g., UFFDIO_COPY_MODE_WP).

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc Mike Rapoport <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

⚠ External Email

Quoted 20 lines · Nadav Amit

From: Nadav Amit <[email protected]>

As the next patches are going to introduce more information that needs to be propagated regarding handled user requests, introduce uffd_flags that would be used to propagate this information.

Remove the unused UFFD_FLAGS_SET to avoid confusion in the constant names.

Introducing uffd flags also allows to avoid mm/userfaultfd from being using uapi (e.g., UFFDIO_COPY_MODE_WP).

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc Mike Rapoport <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

[...]

Quoted 7 lines

int mwriteprotect_range(struct mm_struct *dst_mm, unsigned long start,

(quoted diff) +2 -2

-                     unsigned long len, bool enable_wp,
-                     atomic_t *mmap_changing)
+                     unsigned long len,
+                     atomic_t *mmap_changing, uffd_flags_t uffd_flags)

{
+ bool enable_wp = uffd_flags & UFFD_FLAGS_WP;

Could be that this will trigger a sparse warnings, but I haven't fully understood yet when/how sparse will start to complain. If so, this would have to be

bool enable_wp = !!(uffd_flags & UFFD_FLAGS_WP);

I stumbled into something like that in
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F202202252038.ij1YGn0d-lkp%40intel.com%2FT%2F&amp;data=05%7C01%7Cnamit%40vmware.com%7Cc237032d11f04972fdb708da6bb77ada%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637940733049609220%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=qW0xe6sS7PjP3papl890GoPTbZ97iE%2Ffztt1rA9t6%2F0%3D&amp;reserved=0

From: Nadav Amit <[email protected]>

As the next patches are going to introduce more information that needs to be propagated regarding handled user requests, introduce uffd_flags that would be used to propagate this information.

Remove the unused UFFD_FLAGS_SET to avoid confusion in the constant names.

Introducing uffd flags also allows to avoid mm/userfaultfd from being using uapi (e.g., UFFDIO_COPY_MODE_WP).

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc Mike Rapoport <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 fs/userfaultfd.c              | 22 +++++++++++---
 include/linux/hugetlb.h       |  4 +--
 include/linux/shmem_fs.h      |  8 +++--
 include/linux/userfaultfd_k.h | 24 +++++++++------
 mm/hugetlb.c                  |  3 +-
 mm/shmem.c                    |  6 ++--
 mm/userfaultfd.c              | 57 ++++++++++++++++++-----------------
 7 files changed, 73 insertions(+), 51 deletions(-)

fs/userfaultfd.c +2 -0

index e943370107d0..2ae24327beec 100644
@@ -1682,6 +1682,8 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 	struct uffdio_copy uffdio_copy;
 	struct uffdio_copy __user *user_uffdio_copy;
 	struct userfaultfd_wake_range range;
+	bool mode_wp;
+	uffd_flags_t uffd_flags;
 
 	user_uffdio_copy = (struct uffdio_copy __user *) arg;
 

fs/userfaultfd.c +4 -1

+
+	uffd_flags = mode_wp ? UFFD_FLAGS_WP : UFFD_FLAGS_NONE;
+
 	if (mmget_not_zero(ctx->mm)) {
 		ret = mcopy_atomic(ctx->mm, uffdio_copy.dst, uffdio_copy.src,
 				   uffdio_copy.len, &ctx->mmap_changing,
-				   uffdio_copy.mode);
+				   uffd_flags);
 		mmput(ctx->mm);
 	} else {
 		return -ESRCH;

Quoted 44 lines · Nadav Amit

From: Nadav Amit <[email protected]>

As the next patches are going to introduce more information that needs to be propagated regarding handled user requests, introduce uffd_flags that would be used to propagate this information.

Remove the unused UFFD_FLAGS_SET to avoid confusion in the constant names.

Introducing uffd flags also allows to avoid mm/userfaultfd from being using uapi (e.g., UFFDIO_COPY_MODE_WP).

Cc Mike Kravetz <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Axel Rasmussen <[email protected]>

Cc Peter Xu <[email protected]>

Cc Mike Rapoport <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---

fs/userfaultfd.c              | 22 +++++++++++---
include/linux/hugetlb.h       |  4 +--
include/linux/shmem_fs.h      |  8 +++--
include/linux/userfaultfd_k.h | 24 +++++++++------
mm/hugetlb.c                  |  3 +-
mm/shmem.c                    |  6 ++--
mm/userfaultfd.c              | 57 ++++++++++++++++++-----------------
7 files changed, 73 insertions(+), 51 deletions(-)

fs/userfaultfd.c +2 -0

index e943370107d0..2ae24327beec 100644
@@ -1682,6 +1682,8 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 	struct uffdio_copy uffdio_copy;
 	struct uffdio_copy __user *user_uffdio_copy;
 	struct userfaultfd_wake_range range;
+	bool mode_wp;
+	uffd_flags_t uffd_flags;
 
 	user_uffdio_copy = (struct uffdio_copy __user *) arg;
 

fs/userfaultfd.c static int userfaultfd_copy(struct userfaultfd_ctx *ctx, · line 1714

Quoted 5 lines

fs/userfaultfd.c +2 -0

@@ -1708,10 +1710,15 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 		goto out;
 	if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP))
 		goto out;
+
+	mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP;

This seems to be the only place where mode_wp is used in this function. I'd just drop it, and set uffd_flags directly from uffdio_copy.mode. E.g. something like

	uffd_flags_t uffd_flags = UFFD_FLAGS_NONE;

	...

	if (uffdio_copy.mode & UFFDIO_COPY_MODE_WP)
		uffd_flags = UFFD_FLAGS_WP;