On Tue, Oct 4, 2022 at 2:41 PM Peter Xu <peterx@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
