diff mbox series

[1/2] userfaultfd/selftests: fix hugetlb area allocations

Message ID 20211231041231.zUK9DUgLl%akpm@linux-foundation.org (mailing list archive)
State New
Headers show
Series [1/2] userfaultfd/selftests: fix hugetlb area allocations | expand

Commit Message

Andrew Morton Dec. 31, 2021, 4:12 a.m. UTC
From: Mike Kravetz <mike.kravetz@oracle.com>
Subject: userfaultfd/selftests: fix hugetlb area allocations

Currently, userfaultfd selftest for hugetlb as run from run_vmtests.sh or
any environment where there are 'just enough' hugetlb pages will always
fail with:

testing events (fork, remap, remove):
		ERROR: UFFDIO_COPY error: -12 (errno=12, line=616)

The ENOMEM error code implies there are not enough hugetlb pages. 
However, there are free hugetlb pages but they are all reserved.  There is
a basic problem with the way the test allocates hugetlb pages which has
existed since the test was originally written.  Due to the way 'cleanup'
was done between different phases of the test, this issue was masked until
recently.  The issue was uncovered by commit 8ba6e8640844
("userfaultfd/selftests: reinitialize test context in each test").

For the hugetlb test, src and dst areas are allocated as PRIVATE mappings
of a hugetlb file.  This means that at mmap time, pages are reserved for
the src and dst areas.  At the start of event testing (and other tests)
the src area is populated which results in allocation of huge pages to
fill the area and consumption of reserves associated with the area.  Then,
a child is forked to fault in the dst area.  Note that the dst area was
allocated in the parent and hence the parent owns the reserves associated
with the mapping.  The child has normal access to the dst area, but can
not use the reserves created/owned by the parent.  Thus, if there are no
other huge pages available allocation of a page for the dst by the child
will fail.

Fix by not creating reserves for the dst area.  In this way the child can
use free (non-reserved) pages.

Also, MAP_PRIVATE of a file only makes sense if you are interested in the
contents of the file before making a COW copy.  The test does not do this.
So, just use MAP_ANONYMOUS | MAP_HUGETLB to create an anonymous hugetlb
mapping.  There is no need to create a hugetlb file in the non-shared
case.

Link: https://lkml.kernel.org/r/20211217172919.7861-1-mike.kravetz@oracle.com
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Axel Rasmussen <axelrasmussen@google.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Mina Almasry <almasrymina@google.com>
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 tools/testing/selftests/vm/userfaultfd.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
diff mbox series

Patch

--- a/tools/testing/selftests/vm/userfaultfd.c~userfaultfd-selftests-fix-hugetlb-area-allocations
+++ a/tools/testing/selftests/vm/userfaultfd.c
@@ -87,7 +87,7 @@  static bool test_uffdio_minor = false;
 
 static bool map_shared;
 static int shm_fd;
-static int huge_fd;
+static int huge_fd = -1;	/* only used for hugetlb_shared test */
 static char *huge_fd_off0;
 static unsigned long long *count_verify;
 static int uffd = -1;
@@ -223,6 +223,9 @@  static void noop_alias_mapping(__u64 *st
 
 static void hugetlb_release_pages(char *rel_area)
 {
+	if (huge_fd == -1)
+		return;
+
 	if (fallocate(huge_fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 		      rel_area == huge_fd_off0 ? 0 : nr_pages * page_size,
 		      nr_pages * page_size))
@@ -235,16 +238,17 @@  static void hugetlb_allocate_area(void *
 	char **alloc_area_alias;
 
 	*alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
-			   (map_shared ? MAP_SHARED : MAP_PRIVATE) |
-			   MAP_HUGETLB,
-			   huge_fd, *alloc_area == area_src ? 0 :
-			   nr_pages * page_size);
+			   map_shared ? MAP_SHARED :
+			   MAP_PRIVATE | MAP_HUGETLB |
+			   (*alloc_area == area_src ? 0 : MAP_NORESERVE),
+			   huge_fd,
+			   *alloc_area == area_src ? 0 : nr_pages * page_size);
 	if (*alloc_area == MAP_FAILED)
 		err("mmap of hugetlbfs file failed");
 
 	if (map_shared) {
 		area_alias = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
-				  MAP_SHARED | MAP_HUGETLB,
+				  MAP_SHARED,
 				  huge_fd, *alloc_area == area_src ? 0 :
 				  nr_pages * page_size);
 		if (area_alias == MAP_FAILED)