On Thu, Mar 30, 2023 at 12:07:23PM -0400, Peter Xu wrote:
> In many ways it's weird and unwanted to keep all the tests in the same
> userfaultfd.c at least when still in the current way.
>
> For example, it doesn't make much sense to run the stress test for each
> method we can create an userfaultfd handle (either via syscall or /dev/
> node). It's a waste of time running this twice for the whole stress as the
> stress paths are the same, only the open path is different.
>
> It's also just weird to need to manually specify different types of memory
> to run all unit tests for the userfaultfd interface. We should be able to
> just run a single program and that should go through all functional uffd
> tests without running the stress test at all. The stress test was more for
> torturing and finding race conditions. We don't want to wait for stress to
> finish just to regress test a functional test.
>
> When we start to pile up more things on top of the same file and same
> functions, things start to go a bit chaos and the code is just harder to
> maintain too with tons of global variables.
>
> This patch creates a new test uffd-unit-tests to keep userfaultfd unit
> tests in the future, currently empty.
>
> Meanwhile rename the old userfaultfd.c test to uffd-stress.c.
>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
Reviewed-by: Mike Rapoport (IBM) <rppt@xxxxxxxxxx>
> ---
> tools/testing/selftests/mm/.gitignore | 3 ++-
> tools/testing/selftests/mm/Makefile | 8 +++---
> tools/testing/selftests/mm/run_vmtests.sh | 10 ++++---
> .../mm/{userfaultfd.c => uffd-stress.c} | 0
> tools/testing/selftests/mm/uffd-unit-tests.c | 27 +++++++++++++++++++
> 5 files changed, 40 insertions(+), 8 deletions(-)
> rename tools/testing/selftests/mm/{userfaultfd.c => uffd-stress.c} (100%)
> create mode 100644 tools/testing/selftests/mm/uffd-unit-tests.c
>
> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> index 347277f2adc3..8917455f4f51 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -21,7 +21,8 @@ protection_keys
> protection_keys_32
> protection_keys_64
> madv_populate
> -userfaultfd
> +uffd-stress
> +uffd-unit-tests
> mlock-intersect-test
> mlock-random-test
> virtual_address_range
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 9c3737285f8a..ca8bc4ac71e6 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -20,7 +20,7 @@ MACHINE ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/' -e 's/ppc64.*/p
> # Avoid accidental wrong builds, due to built-in rules working just a little
> # bit too well--but not quite as well as required for our situation here.
> #
> -# In other words, "make userfaultfd" is supposed to fail to build at all,
> +# In other words, "make $SOME_TEST" is supposed to fail to build at all,
> # because this Makefile only supports either "make" (all), or "make /full/path".
> # However, the built-in rules, if not suppressed, will pick up CFLAGS and the
> # initial LDLIBS (but not the target-specific LDLIBS, because those are only
> @@ -56,7 +56,8 @@ TEST_GEN_PROGS += mremap_test
> TEST_GEN_PROGS += on-fault-limit
> TEST_GEN_PROGS += thuge-gen
> TEST_GEN_PROGS += transhuge-stress
> -TEST_GEN_PROGS += userfaultfd
> +TEST_GEN_PROGS += uffd-stress
> +TEST_GEN_PROGS += uffd-unit-tests
> TEST_GEN_PROGS += soft-dirty
> TEST_GEN_PROGS += split_huge_page_test
> TEST_GEN_PROGS += ksm_tests
> @@ -107,7 +108,8 @@ include ../lib.mk
>
> $(TEST_GEN_PROGS): vm_util.c
>
> -$(OUTPUT)/userfaultfd: uffd-common.c
> +$(OUTPUT)/uffd-stress: uffd-common.c
> +$(OUTPUT)/uffd-unit-tests: uffd-common.c
>
> ifeq ($(MACHINE),x86_64)
> BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index 9cc33984aa9f..af7bbc74cd83 100644
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -196,14 +196,16 @@ CATEGORY="gup_test" run_test ./gup_test -a
> # Dump pages 0, 19, and 4096, using pin_user_pages:
> CATEGORY="gup_test" run_test ./gup_test -ct -F 0x1 0 19 0x1000
>
> +CATEGORY="userfaultfd" run_test ./uffd-unit-tests
> uffd_mods=("" ":dev")
> +uffd_stress_bin=./uffd-stress
> for mod in "${uffd_mods[@]}"; do
> - CATEGORY="userfaultfd" run_test ./userfaultfd anon${mod} 20 16
> + CATEGORY="userfaultfd" run_test ${uffd_stress_bin} anon${mod} 20 16
> # Hugetlb tests require source and destination huge pages. Pass in half
> # the size ($half_ufd_size_MB), which is used for *each*.
> - CATEGORY="userfaultfd" run_test ./userfaultfd hugetlb${mod} "$half_ufd_size_MB" 32
> - CATEGORY="userfaultfd" run_test ./userfaultfd hugetlb_shared${mod} "$half_ufd_size_MB" 32
> - CATEGORY="userfaultfd" run_test ./userfaultfd shmem${mod} 20 16
> + CATEGORY="userfaultfd" run_test ${uffd_stress_bin} hugetlb${mod} "$half_ufd_size_MB" 32
> + CATEGORY="userfaultfd" run_test ${uffd_stress_bin} hugetlb_shared${mod} "$half_ufd_size_MB" 32
> + CATEGORY="userfaultfd" run_test ${uffd_stress_bin} shmem${mod} 20 16
> done
>
> #cleanup
> diff --git a/tools/testing/selftests/mm/userfaultfd.c b/tools/testing/selftests/mm/uffd-stress.c
> similarity index 100%
> rename from tools/testing/selftests/mm/userfaultfd.c
> rename to tools/testing/selftests/mm/uffd-stress.c
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> new file mode 100644
> index 000000000000..6857388783be
> --- /dev/null
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Userfaultfd unit tests.
> + *
> + * Copyright (C) 2015-2023 Red Hat, Inc.
> + */
> +
> +#include "uffd-common.h"
> +
> +#ifdef __NR_userfaultfd
> +
> +int main(int argc, char *argv[])
> +{
> + return KSFT_PASS;
> +}
> +
> +#else /* __NR_userfaultfd */
> +
> +#warning "missing __NR_userfaultfd definition"
> +
> +int main(void)
> +{
> + printf("Skipping %s (missing __NR_userfaultfd)\n", __file__);
> + return KSFT_SKIP;
> +}
> +
> +#endif /* __NR_userfaultfd */
> --
> 2.39.1
>
--
Sincerely yours,
Mike.
