| Message ID | 20220128222605.66828-4-mike.kravetz@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add hugetlb MADV_DONTNEED support | expand |
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 <axelrasmussen@google.com> On Fri, Jan 28, 2022 at 2:26 PM Mike Kravetz <mike.kravetz@oracle.com> 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 <mike.kravetz@oracle.com> > --- > 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 <axelrasmussen@google.com> > > On Fri, Jan 28, 2022 at 2:26 PM Mike Kravetz <mike.kravetz@oracle.com> 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 <mike.kravetz@oracle.com> >> --- >> 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,
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);
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 <mike.kravetz@oracle.com> --- tools/testing/selftests/vm/userfaultfd.c | 67 ++++++++++++------------ 1 file changed, 34 insertions(+), 33 deletions(-)