With MADV_DONTNEED support added to hugetlb mappings, mremap testing
can also be enabled for hugetlb.
Modify the tests to use madvise MADV_DONTNEED and MADV_REMOVE instead of
fallocate hole puch for releasing hugetlb pages.
Signed-off-by: Mike Kravetz <[email protected]>
---
tools/testing/selftests/vm/userfaultfd.c | 67 ++++++++++++------------
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index d3fd24f9fae8..f5578ef85560 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -88,7 +88,6 @@ static bool test_uffdio_minor = false;
static bool map_shared;
static int shm_fd;
static int huge_fd;
-static char *huge_fd_off0;
static unsigned long long *count_verify;
static int uffd = -1;
static int uffd_flags, finished, *pipefd;
@@ -124,9 +123,9 @@ const char *examples =
"./userfaultfd anon 100 99999\n\n"
"# Run share memory test on 1GiB region with 99 bounces:\n"
"./userfaultfd shmem 1000 99\n\n"
- "# Run hugetlb memory test on 256MiB region with 50 bounces (using /dev/hugepages/hugefile):\n"
+ "# Run hugetlb memory test on 256MiB region with 50 bounces:\n"
"./userfaultfd hugetlb 256 50 /dev/hugepages/hugefile\n\n"
- "# Run the same hugetlb test but using shmem:\n"
+ "# Run the same hugetlb test but using shared file:\n"
"./userfaultfd hugetlb_shared 256 50 /dev/hugepages/hugefile\n\n"
"# 10MiB-~6GiB 999 bounces anonymous test, "
"continue forever unless an error triggers\n"
@@ -223,10 +222,13 @@ static void noop_alias_mapping(__u64 *start, size_t len, unsigned long offset)
static void hugetlb_release_pages(char *rel_area)
{
- 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))
- err("fallocate() failed");
+ if (!map_shared) {
+ if (madvise(rel_area, nr_pages * page_size, MADV_DONTNEED))
+ err("madvise(MADV_DONTNEED) failed");
+ } else {
+ if (madvise(rel_area, nr_pages * page_size, MADV_REMOVE))
+ err("madvise(MADV_REMOVE) failed");
+ }
}
static void hugetlb_allocate_area(void **alloc_area)
@@ -234,26 +236,37 @@ static void hugetlb_allocate_area(void **alloc_area)
void *area_alias = NULL;
char **alloc_area_alias;
- *alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
- (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 (!map_shared)
+ *alloc_area = mmap(NULL,
+ nr_pages * page_size,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB |
+ (*alloc_area == area_src ? 0 : MAP_NORESERVE),
+ -1,
+ 0);
+ else
+ *alloc_area = mmap(NULL,
+ nr_pages * page_size,
+ PROT_READ | PROT_WRITE,
+ MAP_SHARED |
+ (*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,
- huge_fd, *alloc_area == area_src ? 0 :
- nr_pages * page_size);
+ area_alias = mmap(NULL,
+ nr_pages * page_size,
+ PROT_READ | PROT_WRITE,
+ MAP_SHARED,
+ huge_fd,
+ *alloc_area == area_src ? 0 : nr_pages * page_size);
if (area_alias == MAP_FAILED)
err("mmap of hugetlb file alias failed");
}
if (*alloc_area == area_src) {
- huge_fd_off0 = *alloc_area;
alloc_area_alias = &area_src_alias;
} else {
alloc_area_alias = &area_dst_alias;
@@ -266,12 +279,7 @@ static void hugetlb_alias_mapping(__u64 *start, size_t len, unsigned long offset
{
if (!map_shared)
return;
- /*
- * We can't zap just the pagetable with hugetlbfs because
- * MADV_DONTEED won't work. So exercise -EEXIST on a alias
- * mapping where the pagetables are not established initially,
- * this way we'll exercise the -EEXEC at the fs level.
- */
+
*start = (unsigned long) area_dst_alias + offset;
}
@@ -424,7 +432,6 @@ static void uffd_test_ctx_clear(void)
uffd = -1;
}
- huge_fd_off0 = NULL;
munmap_area((void **)&area_src);
munmap_area((void **)&area_src_alias);
munmap_area((void **)&area_dst);
@@ -922,10 +929,7 @@ static int faulting_process(int signal_test)
struct sigaction act;
unsigned long signalled = 0;
- if (test_type != TEST_HUGETLB)
- split_nr_pages = (nr_pages + 1) / 2;
- else
- split_nr_pages = nr_pages;
+ split_nr_pages = (nr_pages + 1) / 2;
if (signal_test) {
sigbuf = &jbuf;
@@ -982,9 +986,6 @@ static int faulting_process(int signal_test)
if (signal_test)
return signalled != split_nr_pages;
- if (test_type == TEST_HUGETLB)
- return 0;
-
area_dst = mremap(area_dst, nr_pages * page_size, nr_pages * page_size,
MREMAP_MAYMOVE | MREMAP_FIXED, area_src);
if (area_dst == MAP_FAILED)
@@ -1667,7 +1668,7 @@ int main(int argc, char **argv)
}
nr_pages = nr_pages_per_cpu * nr_cpus;
- if (test_type == TEST_HUGETLB) {
+ if (test_type == TEST_HUGETLB && map_shared) {
if (argc < 5)
usage();
huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
--
2.34.1
Besides the help text, looks correct to me. I applied the patches and
ran the userfaultfd selftests, and everything seems to work properly.
Reviewed-by: Axel Rasmussen <[email protected]>
On Fri, Jan 28, 2022 at 2:26 PM Mike Kravetz <[email protected]> wrote:
>
> With MADV_DONTNEED support added to hugetlb mappings, mremap testing
> can also be enabled for hugetlb.
>
> Modify the tests to use madvise MADV_DONTNEED and MADV_REMOVE instead of
> fallocate hole puch for releasing hugetlb pages.
>
> Signed-off-by: Mike Kravetz <[email protected]>
> ---
> tools/testing/selftests/vm/userfaultfd.c | 67 ++++++++++++------------
> 1 file changed, 34 insertions(+), 33 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index d3fd24f9fae8..f5578ef85560 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -88,7 +88,6 @@ static bool test_uffdio_minor = false;
> static bool map_shared;
> static int shm_fd;
> static int huge_fd;
> -static char *huge_fd_off0;
> static unsigned long long *count_verify;
> static int uffd = -1;
> static int uffd_flags, finished, *pipefd;
> @@ -124,9 +123,9 @@ const char *examples =
> "./userfaultfd anon 100 99999\n\n"
> "# Run share memory test on 1GiB region with 99 bounces:\n"
> "./userfaultfd shmem 1000 99\n\n"
> - "# Run hugetlb memory test on 256MiB region with 50 bounces (using /dev/hugepages/hugefile):\n"
> + "# Run hugetlb memory test on 256MiB region with 50 bounces:\n"
> "./userfaultfd hugetlb 256 50 /dev/hugepages/hugefile\n\n"
We should remove the path from the line above here as well, right?
Since for the hugetlb test type, we now just MAP_ANONYMOUS |
MAP_HUGETLB, we don't open a file descriptor.
> - "# Run the same hugetlb test but using shmem:\n"
> + "# Run the same hugetlb test but using shared file:\n"
> "./userfaultfd hugetlb_shared 256 50 /dev/hugepages/hugefile\n\n"
> "# 10MiB-~6GiB 999 bounces anonymous test, "
> "continue forever unless an error triggers\n"
> @@ -223,10 +222,13 @@ static void noop_alias_mapping(__u64 *start, size_t len, unsigned long offset)
>
> static void hugetlb_release_pages(char *rel_area)
> {
> - 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))
> - err("fallocate() failed");
> + if (!map_shared) {
> + if (madvise(rel_area, nr_pages * page_size, MADV_DONTNEED))
> + err("madvise(MADV_DONTNEED) failed");
> + } else {
> + if (madvise(rel_area, nr_pages * page_size, MADV_REMOVE))
> + err("madvise(MADV_REMOVE) failed");
> + }
> }
>
> static void hugetlb_allocate_area(void **alloc_area)
> @@ -234,26 +236,37 @@ static void hugetlb_allocate_area(void **alloc_area)
> void *area_alias = NULL;
> char **alloc_area_alias;
>
> - *alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
> - (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 (!map_shared)
> + *alloc_area = mmap(NULL,
> + nr_pages * page_size,
> + PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB |
> + (*alloc_area == area_src ? 0 : MAP_NORESERVE),
> + -1,
> + 0);
> + else
> + *alloc_area = mmap(NULL,
> + nr_pages * page_size,
> + PROT_READ | PROT_WRITE,
> + MAP_SHARED |
> + (*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,
> - huge_fd, *alloc_area == area_src ? 0 :
> - nr_pages * page_size);
> + area_alias = mmap(NULL,
> + nr_pages * page_size,
> + PROT_READ | PROT_WRITE,
> + MAP_SHARED,
> + huge_fd,
> + *alloc_area == area_src ? 0 : nr_pages * page_size);
> if (area_alias == MAP_FAILED)
> err("mmap of hugetlb file alias failed");
> }
>
> if (*alloc_area == area_src) {
> - huge_fd_off0 = *alloc_area;
> alloc_area_alias = &area_src_alias;
> } else {
> alloc_area_alias = &area_dst_alias;
> @@ -266,12 +279,7 @@ static void hugetlb_alias_mapping(__u64 *start, size_t len, unsigned long offset
> {
> if (!map_shared)
> return;
> - /*
> - * We can't zap just the pagetable with hugetlbfs because
> - * MADV_DONTEED won't work. So exercise -EEXIST on a alias
> - * mapping where the pagetables are not established initially,
> - * this way we'll exercise the -EEXEC at the fs level.
> - */
> +
> *start = (unsigned long) area_dst_alias + offset;
> }
>
> @@ -424,7 +432,6 @@ static void uffd_test_ctx_clear(void)
> uffd = -1;
> }
>
> - huge_fd_off0 = NULL;
> munmap_area((void **)&area_src);
> munmap_area((void **)&area_src_alias);
> munmap_area((void **)&area_dst);
> @@ -922,10 +929,7 @@ static int faulting_process(int signal_test)
> struct sigaction act;
> unsigned long signalled = 0;
>
> - if (test_type != TEST_HUGETLB)
> - split_nr_pages = (nr_pages + 1) / 2;
> - else
> - split_nr_pages = nr_pages;
> + split_nr_pages = (nr_pages + 1) / 2;
>
> if (signal_test) {
> sigbuf = &jbuf;
> @@ -982,9 +986,6 @@ static int faulting_process(int signal_test)
> if (signal_test)
> return signalled != split_nr_pages;
>
> - if (test_type == TEST_HUGETLB)
> - return 0;
> -
> area_dst = mremap(area_dst, nr_pages * page_size, nr_pages * page_size,
> MREMAP_MAYMOVE | MREMAP_FIXED, area_src);
> if (area_dst == MAP_FAILED)
> @@ -1667,7 +1668,7 @@ int main(int argc, char **argv)
> }
> nr_pages = nr_pages_per_cpu * nr_cpus;
>
> - if (test_type == TEST_HUGETLB) {
> + if (test_type == TEST_HUGETLB && map_shared) {
> if (argc < 5)
> usage();
> huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
> --
> 2.34.1
>
On 1/28/22 15:34, Axel Rasmussen wrote:
> Besides the help text, looks correct to me. I applied the patches and
> ran the userfaultfd selftests, and everything seems to work properly.
>
> Reviewed-by: Axel Rasmussen <[email protected]>
>
> On Fri, Jan 28, 2022 at 2:26 PM Mike Kravetz <[email protected]> wrote:
>>
>> With MADV_DONTNEED support added to hugetlb mappings, mremap testing
>> can also be enabled for hugetlb.
>>
>> Modify the tests to use madvise MADV_DONTNEED and MADV_REMOVE instead of
>> fallocate hole puch for releasing hugetlb pages.
>>
>> Signed-off-by: Mike Kravetz <[email protected]>
>> ---
>> tools/testing/selftests/vm/userfaultfd.c | 67 ++++++++++++------------
>> 1 file changed, 34 insertions(+), 33 deletions(-)
>>
>> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
>> index d3fd24f9fae8..f5578ef85560 100644
>> --- a/tools/testing/selftests/vm/userfaultfd.c
>> +++ b/tools/testing/selftests/vm/userfaultfd.c
>> @@ -88,7 +88,6 @@ static bool test_uffdio_minor = false;
>> static bool map_shared;
>> static int shm_fd;
>> static int huge_fd;
>> -static char *huge_fd_off0;
>> static unsigned long long *count_verify;
>> static int uffd = -1;
>> static int uffd_flags, finished, *pipefd;
>> @@ -124,9 +123,9 @@ const char *examples =
>> "./userfaultfd anon 100 99999\n\n"
>> "# Run share memory test on 1GiB region with 99 bounces:\n"
>> "./userfaultfd shmem 1000 99\n\n"
>> - "# Run hugetlb memory test on 256MiB region with 50 bounces (using /dev/hugepages/hugefile):\n"
>> + "# Run hugetlb memory test on 256MiB region with 50 bounces:\n"
>> "./userfaultfd hugetlb 256 50 /dev/hugepages/hugefile\n\n"
>
> We should remove the path from the line above here as well, right?
> Since for the hugetlb test type, we now just MAP_ANONYMOUS |
> MAP_HUGETLB, we don't open a file descriptor.
>
Yes, and I should also update run_vmtests.sh to not include file path.
The test just ignores the file path in this case.
Thanks,
--
Mike Kravetz