| Message ID | 20221004214125.120993-2-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/vm: Drop hugetlb mntpoint in run_vmtests.sh | expand |
On Tue, Oct 4, 2022 at 2:41 PM Peter Xu <peterx@redhat.com> wrote: > > We already used memfd for shmem test, move it forward with hugetlb too so > that we don't need user to specify the hugetlb file path explicitly when > running hugetlb shared tests. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > tools/testing/selftests/vm/userfaultfd.c | 60 ++++++++---------------- > 1 file changed, 20 insertions(+), 40 deletions(-) > > diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c > index 74babdbc02e5..c0c6853cdce5 100644 > --- a/tools/testing/selftests/vm/userfaultfd.c > +++ b/tools/testing/selftests/vm/userfaultfd.c > @@ -93,10 +93,8 @@ static volatile bool test_uffdio_zeropage_eexist = true; > static bool test_uffdio_wp = true; > /* Whether to test uffd minor faults */ > static bool test_uffdio_minor = false; > - > static bool map_shared; > -static int shm_fd; > -static int huge_fd; > +static int mem_fd; > static unsigned long long *count_verify; > static int uffd = -1; > static int uffd_flags, finished, *pipefd; > @@ -260,35 +258,21 @@ static void hugetlb_release_pages(char *rel_area) > > static void hugetlb_allocate_area(void **alloc_area, bool is_src) > { > + off_t size = nr_pages * page_size; > + off_t offset = is_src ? 0 : size; > void *area_alias = NULL; > char **alloc_area_alias; > > - if (!map_shared) > - *alloc_area = mmap(NULL, > - nr_pages * page_size, > - PROT_READ | PROT_WRITE, > - MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | > - (is_src ? 0 : MAP_NORESERVE), > - -1, > - 0); > - else > - *alloc_area = mmap(NULL, > - nr_pages * page_size, > - PROT_READ | PROT_WRITE, > - MAP_SHARED | > - (is_src ? 0 : MAP_NORESERVE), > - huge_fd, > - is_src ? 0 : nr_pages * page_size); > + *alloc_area = mmap(NULL, size, PROT_READ | PROT_WRITE, > + (map_shared ? MAP_SHARED : MAP_PRIVATE) | > + (is_src ? 0 : MAP_NORESERVE), > + mem_fd, offset); > 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, > - huge_fd, > - is_src ? 0 : nr_pages * page_size); > + area_alias = mmap(NULL, size, PROT_READ | PROT_WRITE, > + MAP_SHARED, mem_fd, offset); > if (area_alias == MAP_FAILED) > err("mmap of hugetlb file alias failed"); > } > @@ -334,14 +318,14 @@ static void shmem_allocate_area(void **alloc_area, bool is_src) > } > > *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, > - shm_fd, offset); > + mem_fd, offset); > if (*alloc_area == MAP_FAILED) > err("mmap of memfd failed"); > if (test_collapse && *alloc_area != p) > err("mmap of memfd failed at %p", p); > > area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, > - shm_fd, offset); > + mem_fd, offset); > if (area_alias == MAP_FAILED) > err("mmap of memfd alias failed"); > if (test_collapse && area_alias != p_alias) > @@ -1821,21 +1805,17 @@ int main(int argc, char **argv) > } > nr_pages = nr_pages_per_cpu * nr_cpus; > > - if (test_type == TEST_HUGETLB && map_shared) { > - if (argc < 5) > - usage(); > - huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755); > - if (huge_fd < 0) > - err("Open of %s failed", argv[4]); > - if (ftruncate(huge_fd, 0)) > - err("ftruncate %s to size 0 failed", argv[4]); We should also update the examples / help text near the top of the file, since we're changing what arguments this accepts. It might also be better to squash the changes to test arguments in run_vmtests.sh into each patch, so if we're bisecting we'll have a matching / working test + run_vmtests.sh combo at each commit. > - } else if (test_type == TEST_SHMEM) { > - shm_fd = memfd_create(argv[0], 0); > - if (shm_fd < 0) > + if (test_type == TEST_SHMEM || test_type == TEST_HUGETLB) { > + unsigned int memfd_flags = 0; > + > + if (test_type == TEST_HUGETLB) > + memfd_flags = MFD_HUGETLB; > + mem_fd = memfd_create(argv[0], memfd_flags); > + if (mem_fd < 0) > err("memfd_create"); > - if (ftruncate(shm_fd, nr_pages * page_size * 2)) > + if (ftruncate(mem_fd, nr_pages * page_size * 2)) > err("ftruncate"); > - if (fallocate(shm_fd, > + if (fallocate(mem_fd, > FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, > nr_pages * page_size * 2)) > err("fallocate"); > -- > 2.37.3 > This is a nice simplification! Thanks for doing it. Besides the comments above, the rest of the code here looks correct to me. Feel free to take: Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
On Tue, Oct 04, 2022 at 03:04:23PM -0700, Axel Rasmussen wrote: > We should also update the examples / help text near the top of the > file, since we're changing what arguments this accepts. Definitely, I missed that. > > It might also be better to squash the changes to test arguments in > run_vmtests.sh into each patch, so if we're bisecting we'll have a > matching / working test + run_vmtests.sh combo at each commit. I can do that, but may not be required - note that I did the first three patches just to make sure they're still compatible with the old cmdlines, by ignoring the extra params. [...] > This is a nice simplification! Thanks for doing it. Besides the > comments above, the rest of the code here looks correct to me. Feel > free to take: > > Reviewed-by: Axel Rasmussen <axelrasmussen@google.com> Thanks!
On Wed, Oct 5, 2022 at 7:44 AM Peter Xu <peterx@redhat.com> wrote: > > On Tue, Oct 04, 2022 at 03:04:23PM -0700, Axel Rasmussen wrote: > > We should also update the examples / help text near the top of the > > file, since we're changing what arguments this accepts. > > Definitely, I missed that. > > > > > It might also be better to squash the changes to test arguments in > > run_vmtests.sh into each patch, so if we're bisecting we'll have a > > matching / working test + run_vmtests.sh combo at each commit. > > I can do that, but may not be required - note that I did the first three > patches just to make sure they're still compatible with the old cmdlines, > by ignoring the extra params. Ah! In that case, forget it. :) > > [...] > > > This is a nice simplification! Thanks for doing it. Besides the > > comments above, the rest of the code here looks correct to me. Feel > > free to take: > > > > Reviewed-by: Axel Rasmussen <axelrasmussen@google.com> > > Thanks! > > -- > Peter Xu >
diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c index 74babdbc02e5..c0c6853cdce5 100644 --- a/tools/testing/selftests/vm/userfaultfd.c +++ b/tools/testing/selftests/vm/userfaultfd.c @@ -93,10 +93,8 @@ static volatile bool test_uffdio_zeropage_eexist = true; static bool test_uffdio_wp = true; /* Whether to test uffd minor faults */ static bool test_uffdio_minor = false; - static bool map_shared; -static int shm_fd; -static int huge_fd; +static int mem_fd; static unsigned long long *count_verify; static int uffd = -1; static int uffd_flags, finished, *pipefd; @@ -260,35 +258,21 @@ static void hugetlb_release_pages(char *rel_area) static void hugetlb_allocate_area(void **alloc_area, bool is_src) { + off_t size = nr_pages * page_size; + off_t offset = is_src ? 0 : size; void *area_alias = NULL; char **alloc_area_alias; - if (!map_shared) - *alloc_area = mmap(NULL, - nr_pages * page_size, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | - (is_src ? 0 : MAP_NORESERVE), - -1, - 0); - else - *alloc_area = mmap(NULL, - nr_pages * page_size, - PROT_READ | PROT_WRITE, - MAP_SHARED | - (is_src ? 0 : MAP_NORESERVE), - huge_fd, - is_src ? 0 : nr_pages * page_size); + *alloc_area = mmap(NULL, size, PROT_READ | PROT_WRITE, + (map_shared ? MAP_SHARED : MAP_PRIVATE) | + (is_src ? 0 : MAP_NORESERVE), + mem_fd, offset); 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, - huge_fd, - is_src ? 0 : nr_pages * page_size); + area_alias = mmap(NULL, size, PROT_READ | PROT_WRITE, + MAP_SHARED, mem_fd, offset); if (area_alias == MAP_FAILED) err("mmap of hugetlb file alias failed"); } @@ -334,14 +318,14 @@ static void shmem_allocate_area(void **alloc_area, bool is_src) } *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, - shm_fd, offset); + mem_fd, offset); if (*alloc_area == MAP_FAILED) err("mmap of memfd failed"); if (test_collapse && *alloc_area != p) err("mmap of memfd failed at %p", p); area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, - shm_fd, offset); + mem_fd, offset); if (area_alias == MAP_FAILED) err("mmap of memfd alias failed"); if (test_collapse && area_alias != p_alias) @@ -1821,21 +1805,17 @@ int main(int argc, char **argv) } nr_pages = nr_pages_per_cpu * nr_cpus; - if (test_type == TEST_HUGETLB && map_shared) { - if (argc < 5) - usage(); - huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755); - if (huge_fd < 0) - err("Open of %s failed", argv[4]); - if (ftruncate(huge_fd, 0)) - err("ftruncate %s to size 0 failed", argv[4]); - } else if (test_type == TEST_SHMEM) { - shm_fd = memfd_create(argv[0], 0); - if (shm_fd < 0) + if (test_type == TEST_SHMEM || test_type == TEST_HUGETLB) { + unsigned int memfd_flags = 0; + + if (test_type == TEST_HUGETLB) + memfd_flags = MFD_HUGETLB; + mem_fd = memfd_create(argv[0], memfd_flags); + if (mem_fd < 0) err("memfd_create"); - if (ftruncate(shm_fd, nr_pages * page_size * 2)) + if (ftruncate(mem_fd, nr_pages * page_size * 2)) err("ftruncate"); - if (fallocate(shm_fd, + if (fallocate(mem_fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, nr_pages * page_size * 2)) err("fallocate");
We already used memfd for shmem test, move it forward with hugetlb too so that we don't need user to specify the hugetlb file path explicitly when running hugetlb shared tests. Signed-off-by: Peter Xu <peterx@redhat.com> --- tools/testing/selftests/vm/userfaultfd.c | 60 ++++++++---------------- 1 file changed, 20 insertions(+), 40 deletions(-)