userfaultfd move option

From: Andrea Arcangeli <[email protected]>

For now, folio_move_anon_rmap() was only used to move a folio to a different anon_vma after fork(), whereby the root anon_vma stayed unchanged. For that, it was sufficient to hold the folio lock when calling folio_move_anon_rmap().

However, we want to make use of folio_move_anon_rmap() to move folios between VMAs that have a different root anon_vma. As folio_referenced() performs an RMAP walk without holding the folio lock but only holding the anon_vma in read mode, holding the folio lock is insufficient.

When moving to an anon_vma with a different root anon_vma, we'll have to hold both, the folio lock and the anon_vma lock in write mode. Consequently, whenever we succeeded in folio_lock_anon_vma_read() to read-lock the anon_vma, we have to re-check if the mapping was changed in the meantime. If that was the case, we have to retry.

Note that folio_move_anon_rmap() must only be called if the anon page is exclusive to a process, and must not be called on KSM folios.

This is a preparation for UFFDIO_MOVE, which will hold the folio lock, the anon_vma lock in write mode, and the mmap_lock in read mode.

Signed-off-by Andrea Arcangeli <[email protected]>

Signed-off-by Suren Baghdasaryan <[email protected]>

uffd_test_ctx_clear() is being called from uffd_test_ctx_init() to unmap areas used in the previous test run. This approach is problematic because while unmapping areas uffd_test_ctx_clear() uses page_size and nr_pages which might differ from one test run to another.
Fix this by calling uffd_test_ctx_clear() after each test is done.

Signed-off-by Suren Baghdasaryan <[email protected]>

Quoted 7 lines · Suren Baghdasaryan

uffd_test_ctx_clear() is being called from uffd_test_ctx_init() to unmap areas used in the previous test run. This approach is problematic because while unmapping areas uffd_test_ctx_clear() uses page_size and nr_pages which might differ from one test run to another.
Fix this by calling uffd_test_ctx_clear() after each test is done.

Signed-off-by Suren Baghdasaryan <[email protected]>

Reviewed-by Peter Xu <[email protected]>

--
Peter Xu

Add tests for new UFFDIO_MOVE ioctl which uses uffd to move source into destination buffer while checking the contents of both after the move. After the operation the content of the destination buffer should match the original source buffer's content while the source buffer should be zeroed. Separate tests are designed for PMD aligned and unaligned cases because they utilize different code paths in the kernel.

Signed-off-by Suren Baghdasaryan <[email protected]>

---
 tools/testing/selftests/mm/uffd-common.c     |  24 ++++
 tools/testing/selftests/mm/uffd-common.h     |   1 +
 tools/testing/selftests/mm/uffd-unit-tests.c | 141 +++++++++++++++++++
 3 files changed, 166 insertions(+)

tools/testing/selftests/mm/uffd-common.c +24 -0

index 69e6653ad255..98957fd788d8 100644
@@ -643,6 +643,30 @@ int copy_page(int ufd, unsigned long offset, bool wp)
 	return __copy_page(ufd, offset, false, wp);
 }
 
+int move_page(int ufd, unsigned long offset)
+{
+	struct uffdio_move uffdio_move;
+
+	if (offset >= nr_pages * page_size)
+		err("unexpected offset %lu\n", offset);
+	uffdio_move.dst = (unsigned long) area_dst + offset;
+	uffdio_move.src = (unsigned long) area_src + offset;
+	uffdio_move.len = page_size;
+	uffdio_move.mode = UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES;
+	uffdio_move.move = 0;
+	if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) {
+		/* real retval in uffdio_move.move */
+		if (uffdio_move.move != -EEXIST)
+			err("UFFDIO_MOVE error: %"PRId64,
+			    (int64_t)uffdio_move.move);
+		wake_range(ufd, uffdio_move.dst, page_size);
+	} else if (uffdio_move.move != page_size) {
+		err("UFFDIO_MOVE error: %"PRId64, (int64_t)uffdio_move.move);
+	} else
+		return 1;
+	return 0;
+}
+
 int uffd_open_dev(unsigned int flags)
 {
 	int fd, uffd;

tools/testing/selftests/mm/uffd-common.h +1 -0

index 19930fd6682b..c9526b2cb6b3 100644
@@ -121,6 +121,7 @@ void wp_range(int ufd, __u64 start, __u64 len, bool wp);
 void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_args *args);
 int __copy_page(int ufd, unsigned long offset, bool retry, bool wp);
 int copy_page(int ufd, unsigned long offset, bool wp);
+int move_page(int ufd, unsigned long offset);
 void *uffd_poll_thread(void *arg);
 
 int uffd_open_dev(unsigned int flags);

tools/testing/selftests/mm/uffd-unit-tests.c +102 -0

index debc423bdbf4..89e9529ce941 100644
@@ -1064,6 +1064,133 @@ static void uffd_poison_test(uffd_test_args_t *targs)
 	uffd_test_pass();
 }
 
+static void uffd_move_handle_fault(
+	struct uffd_msg *msg, struct uffd_args *args)
+{
+	unsigned long offset;
+
+	if (msg->event != UFFD_EVENT_PAGEFAULT)
+		err("unexpected msg event %u", msg->event);
+
+	if (msg->arg.pagefault.flags &
+	    (UFFD_PAGEFAULT_FLAG_WP | UFFD_PAGEFAULT_FLAG_MINOR | UFFD_PAGEFAULT_FLAG_WRITE))
+		err("unexpected fault type %llu", msg->arg.pagefault.flags);
+
+	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
+	offset &= ~(page_size-1);
+
+	if (move_page(uffd, offset))
+		args->missing_faults++;
+}
+
+static void uffd_move_test(uffd_test_args_t *targs)
+{
+	unsigned long nr;
+	pthread_t uffd_mon;
+	char c;
+	unsigned long long count;
+	struct uffd_args args = { 0 };
+
+	/* Prevent source pages from being mapped more than once */
+	if (madvise(area_src, nr_pages * page_size, MADV_DONTFORK))
+		err("madvise(MADV_DONTFORK) failure");
+
+	if (uffd_register(uffd, area_dst, nr_pages * page_size,
+			  true, false, false))
+		err("register failure");
+
+	args.handle_fault = uffd_move_handle_fault;
+	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
+		err("uffd_poll_thread create");
+
+	/*
+	 * Read each of the pages back using the UFFD-registered mapping. We
+	 * expect that the first time we touch a page, it will result in a missing
+	 * fault. uffd_poll_thread will resolve the fault by moving source
+	 * page to destination.
+	 */
+	for (nr = 0; nr < nr_pages; nr++) {
+		/* Check area_src content */
+		count = *area_count(area_src, nr);
+		if (count != count_verify[nr])
+			err("nr %lu source memory invalid %llu %llu\n",
+			    nr, count, count_verify[nr]);
+
+		/* Faulting into area_dst should move the page */
+		count = *area_count(area_dst, nr);
+		if (count != count_verify[nr])
+			err("nr %lu memory corruption %llu %llu\n",
+			    nr, count, count_verify[nr]);
+
+		/* Re-check area_src content which should be empty */
+		count = *area_count(area_src, nr);
+		if (count != 0)
+			err("nr %lu move failed %llu %llu\n",
+			    nr, count, count_verify[nr]);
+	}
+
+	if (write(pipefd[1], &c, sizeof(c)) != sizeof(c))
+		err("pipe write");
+	if (pthread_join(uffd_mon, NULL))
+		err("join() failed");
+
+	if (args.missing_faults != nr_pages || args.minor_faults != 0)
+		uffd_test_fail("stats check error");
+	else
+		uffd_test_pass();
+}
+
+static int prevent_hugepages(void)
+{
+	/* This should be done before source area is populated */
+	if (madvise(area_src, nr_pages * page_size, MADV_NOHUGEPAGE)) {
+		/* Ignore if CONFIG_TRANSPARENT_HUGEPAGE=n */
+		if (errno != EINVAL)
+			return -errno;
+	}
+	return 0;
+}
+
+struct uffd_test_case_ops uffd_move_test_case_ops = {
+	.post_alloc = prevent_hugepages,
+};
+
+#define ALIGN_UP(x, align_to) \
+	(__typeof__(x))((((unsigned long)(x)) + ((align_to)-1)) & ~((align_to)-1))
+
+static char *orig_area_src, *orig_area_dst;
+static int pmd_align_areas(void)
+{
+	orig_area_src = area_src;
+	orig_area_dst = area_dst;
+	area_src = ALIGN_UP(area_src, page_size);
+	area_dst = ALIGN_UP(area_dst, page_size);
+	nr_pages--;

tools/testing/selftests/mm/uffd-unit-tests.c +25 -0

+	nr_pages = UFFD_TEST_MEM_SIZE / page_size;
+
+	return 0;
+}
+
+struct uffd_test_case_ops uffd_move_pmd_test_case_ops = {
+	.pre_alloc = adjust_page_size,
+	.post_alloc = pmd_align_areas,
+	.pre_release = pmd_restore_areas,
+};
+
 /*
  * Test the returned uffdio_register.ioctls with different register modes.
  * Note that _UFFDIO_ZEROPAGE is tested separately in the zeropage test.

@@ -1141,6 +1268,20 @@ uffd_test_case_t uffd_tests[] = {
 		.mem_targets = MEM_ALL,
 		.uffd_feature_required = 0,
 	},
+	{
+		.name = "move",
+		.uffd_fn = uffd_move_test,
+		.mem_targets = MEM_ANON,
+		.uffd_feature_required = UFFD_FEATURE_MOVE,
+		.test_case_ops = &uffd_move_test_case_ops,
+	},
+	{
+		.name = "move-pmd",
+		.uffd_fn = uffd_move_test,
+		.mem_targets = MEM_ANON,
+		.uffd_feature_required = UFFD_FEATURE_MOVE,
+		.test_case_ops = &uffd_move_pmd_test_case_ops,
+	},
 	{
 		.name = "wp-fork",
 		.uffd_fn = uffd_wp_fork_test,

Signature · 1 line

2.42.0.820.g83a721a137-goog

Quoted 171 lines · Suren Baghdasaryan

Add tests for new UFFDIO_MOVE ioctl which uses uffd to move source into destination buffer while checking the contents of both after the move. After the operation the content of the destination buffer should match the original source buffer's content while the source buffer should be zeroed. Separate tests are designed for PMD aligned and unaligned cases because they utilize different code paths in the kernel.

Signed-off-by Suren Baghdasaryan <[email protected]>

---
 tools/testing/selftests/mm/uffd-common.c     |  24 ++++
 tools/testing/selftests/mm/uffd-common.h     |   1 +
 tools/testing/selftests/mm/uffd-unit-tests.c | 141 +++++++++++++++++++
 3 files changed, 166 insertions(+)

tools/testing/selftests/mm/uffd-common.c +24 -0

index 69e6653ad255..98957fd788d8 100644
@@ -643,6 +643,30 @@ int copy_page(int ufd, unsigned long offset, bool wp)
      return __copy_page(ufd, offset, false, wp);
 }
 
+int move_page(int ufd, unsigned long offset)
+{
+     struct uffdio_move uffdio_move;
+
+     if (offset >= nr_pages * page_size)
+             err("unexpected offset %lu\n", offset);
+     uffdio_move.dst = (unsigned long) area_dst + offset;
+     uffdio_move.src = (unsigned long) area_src + offset;
+     uffdio_move.len = page_size;
+     uffdio_move.mode = UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES;
+     uffdio_move.move = 0;
+     if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) {
+             /* real retval in uffdio_move.move */
+             if (uffdio_move.move != -EEXIST)
+                     err("UFFDIO_MOVE error: %"PRId64,
+                         (int64_t)uffdio_move.move);
+             wake_range(ufd, uffdio_move.dst, page_size);
+     } else if (uffdio_move.move != page_size) {
+             err("UFFDIO_MOVE error: %"PRId64, (int64_t)uffdio_move.move);
+     } else
+             return 1;
+     return 0;
+}
+
 int uffd_open_dev(unsigned int flags)
 {
      int fd, uffd;

tools/testing/selftests/mm/uffd-common.h +1 -0

index 19930fd6682b..c9526b2cb6b3 100644
@@ -121,6 +121,7 @@ void wp_range(int ufd, __u64 start, __u64 len, bool wp);
 void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_args *args);
 int __copy_page(int ufd, unsigned long offset, bool retry, bool wp);
 int copy_page(int ufd, unsigned long offset, bool wp);
+int move_page(int ufd, unsigned long offset);
 void *uffd_poll_thread(void *arg);
 
 int uffd_open_dev(unsigned int flags);

tools/testing/selftests/mm/uffd-unit-tests.c +102 -0

index debc423bdbf4..89e9529ce941 100644
@@ -1064,6 +1064,133 @@ static void uffd_poison_test(uffd_test_args_t *targs)
      uffd_test_pass();
 }
 
+static void uffd_move_handle_fault(
+     struct uffd_msg *msg, struct uffd_args *args)
+{
+     unsigned long offset;
+
+     if (msg->event != UFFD_EVENT_PAGEFAULT)
+             err("unexpected msg event %u", msg->event);
+
+     if (msg->arg.pagefault.flags &
+         (UFFD_PAGEFAULT_FLAG_WP | UFFD_PAGEFAULT_FLAG_MINOR | UFFD_PAGEFAULT_FLAG_WRITE))
+             err("unexpected fault type %llu", msg->arg.pagefault.flags);
+
+     offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
+     offset &= ~(page_size-1);
+
+     if (move_page(uffd, offset))
+             args->missing_faults++;
+}
+
+static void uffd_move_test(uffd_test_args_t *targs)
+{
+     unsigned long nr;
+     pthread_t uffd_mon;
+     char c;
+     unsigned long long count;
+     struct uffd_args args = { 0 };
+
+     /* Prevent source pages from being mapped more than once */
+     if (madvise(area_src, nr_pages * page_size, MADV_DONTFORK))
+             err("madvise(MADV_DONTFORK) failure");
+
+     if (uffd_register(uffd, area_dst, nr_pages * page_size,
+                       true, false, false))
+             err("register failure");
+
+     args.handle_fault = uffd_move_handle_fault;
+     if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
+             err("uffd_poll_thread create");
+
+     /*
+      * Read each of the pages back using the UFFD-registered mapping. We
+      * expect that the first time we touch a page, it will result in a missing
+      * fault. uffd_poll_thread will resolve the fault by moving source
+      * page to destination.
+      */
+     for (nr = 0; nr < nr_pages; nr++) {
+             /* Check area_src content */
+             count = *area_count(area_src, nr);
+             if (count != count_verify[nr])
+                     err("nr %lu source memory invalid %llu %llu\n",
+                         nr, count, count_verify[nr]);
+
+             /* Faulting into area_dst should move the page */
+             count = *area_count(area_dst, nr);
+             if (count != count_verify[nr])
+                     err("nr %lu memory corruption %llu %llu\n",
+                         nr, count, count_verify[nr]);
+
+             /* Re-check area_src content which should be empty */
+             count = *area_count(area_src, nr);
+             if (count != 0)
+                     err("nr %lu move failed %llu %llu\n",
+                         nr, count, count_verify[nr]);
+     }
+
+     if (write(pipefd[1], &c, sizeof(c)) != sizeof(c))
+             err("pipe write");
+     if (pthread_join(uffd_mon, NULL))
+             err("join() failed");
+
+     if (args.missing_faults != nr_pages || args.minor_faults != 0)
+             uffd_test_fail("stats check error");
+     else
+             uffd_test_pass();
+}
+
+static int prevent_hugepages(void)
+{
+     /* This should be done before source area is populated */
+     if (madvise(area_src, nr_pages * page_size, MADV_NOHUGEPAGE)) {
+             /* Ignore if CONFIG_TRANSPARENT_HUGEPAGE=n */
+             if (errno != EINVAL)
+                     return -errno;
+     }
+     return 0;
+}
+
+struct uffd_test_case_ops uffd_move_test_case_ops = {
+     .post_alloc = prevent_hugepages,
+};
+
+#define ALIGN_UP(x, align_to) \
+     (__typeof__(x))((((unsigned long)(x)) + ((align_to)-1)) & ~((align_to)-1))
+
+static char *orig_area_src, *orig_area_dst;
+static int pmd_align_areas(void)
+{
+     orig_area_src = area_src;
+     orig_area_dst = area_dst;
+     area_src = ALIGN_UP(area_src, page_size);
+     area_dst = ALIGN_UP(area_dst, page_size);
+     nr_pages--;

tools/testing/selftests/mm/uffd-unit-tests.c static void uffd_poison_test(uffd_test_args_t *targs) · line 1178

Quoted 10 lines

tools/testing/selftests/mm/uffd-unit-tests.c +10 -0

+
+     return 0;
+}
+
+static void pmd_restore_areas(void)
+{
+     area_src = orig_area_src;
+     area_dst = orig_area_dst;
+     nr_pages++;
+}

Please stop using more global variables.. uffd tests are even less maintainable.

Maybe you can consider add a flag for uffd_test_ctx_init()? For allocating either small/thp/default?

tools/testing/selftests/mm/uffd-unit-tests.c static void uffd_poison_test(uffd_test_args_t *targs) · line 1182

Quoted 6 lines

tools/testing/selftests/mm/uffd-unit-tests.c +4 -0

+
+static int adjust_page_size(void)
+{
+     page_size = default_huge_page_size();

This is hacky too, currently page_size is the real page_size backing the memory.

To make thp test simple, maybe just add one more test to MOVE a large chunk to replace the thp test, which may contain a few thps? It also doesn't need to be fault based.

Quoted 37 lines

tools/testing/selftests/mm/uffd-unit-tests.c +25 -1

+     nr_pages = UFFD_TEST_MEM_SIZE / page_size;
+
+     return 0;
+}
+
+struct uffd_test_case_ops uffd_move_pmd_test_case_ops = {
+     .pre_alloc = adjust_page_size,
+     .post_alloc = pmd_align_areas,
+     .pre_release = pmd_restore_areas,
+};
+
 /*
  * Test the returned uffdio_register.ioctls with different register modes.
  * Note that _UFFDIO_ZEROPAGE is tested separately in the zeropage test.

@@ -1141,6 +1268,20 @@ uffd_test_case_t uffd_tests[] = {
              .mem_targets = MEM_ALL,
              .uffd_feature_required = 0,
      },
+     {
+             .name = "move",
+             .uffd_fn = uffd_move_test,
+             .mem_targets = MEM_ANON,
+             .uffd_feature_required = UFFD_FEATURE_MOVE,
+             .test_case_ops = &uffd_move_test_case_ops,
+     },
+     {
+             .name = "move-pmd",
+             .uffd_fn = uffd_move_test,
+             .mem_targets = MEM_ANON,
+             .uffd_feature_required = UFFD_FEATURE_MOVE,
+             .test_case_ops = &uffd_move_pmd_test_case_ops,
+     },
      {
              .name = "wp-fork",
              .uffd_fn = uffd_wp_fork_test,
--

Quoted 2 lines

2.42.0.820.g83a721a137-goog

--
Peter Xu

Quoted 7 lines

tools/testing/selftests/mm/uffd-unit-tests.c static void uffd_poison_test(uffd_test_args_t *targs) · line 1182

Quoted 3 lines

tools/testing/selftests/mm/uffd-unit-tests.c +3 -0

+static int adjust_page_size(void)
+{
+     page_size = default_huge_page_size();

This is hacky too, currently page_size is the real page_size backing the memory.

Quoted 3 lines

To make thp test simple, maybe just add one more test to MOVE a large chunk to replace the thp test, which may contain a few thps? It also doesn't need to be fault based.

Sorry, I didn't get your suggestion. Could you please clarify? Which thp test are you referring to?

Quoted 7 lines · Suren Baghdasaryan

Quoted 7 lines

tools/testing/selftests/mm/uffd-unit-tests.c static void uffd_poison_test(uffd_test_args_t *targs) · line 1182

Quoted 3 lines

tools/testing/selftests/mm/uffd-unit-tests.c +3 -0

+static int adjust_page_size(void)
+{
+     page_size = default_huge_page_size();

This is hacky too, currently page_size is the real page_size backing the memory.

Quoted 6 lines

Quoted 3 lines

To make thp test simple, maybe just add one more test to MOVE a large chunk to replace the thp test, which may contain a few thps? It also doesn't need to be fault based.

Sorry, I didn't get your suggestion. Could you please clarify? Which thp test are you referring to?

The new "move-pmd" test.

I meant maybe it makes sense to have one separate MOVE test for when one ioctl(MOVE) covers a large range which can cover some thps. Then that will trigger thp paths. Assuming the fault paths are already covered in the generic "move" test.

Thanks,

--
Peter Xu