Just a friendly bump for review. :) Peter, any objections to this
version? I think it fairly closely matches your suggestions from v1.
On Thu, Sep 30, 2021 at 2:23 PM Axel Rasmussen <axelrasmussen@xxxxxxxxxx> wrote:
>
> Before any tests are run, in set_test_type, we decide what feature(s) we
> are going to be testing, based upon our command line arguments. However,
> the supported features are not just a function of the memory type being
> used, so this is broken.
>
> For instance, consider writeprotect support. It is "normally" supported
> for anonymous memory, but furthermore it requires that the kernel has
> CONFIG_HAVE_ARCH_USERFAULTFD_WP. So, it is *not* supported at all on
> aarch64, for example.
>
> So, this commit fixes this by querying the kernel for the set of
> features it supports in set_test_type, by opening a userfaultfd and
> issuing a UFFDIO_API ioctl. Based upon the reported features, we toggle
> what tests are enabled.
>
> Signed-off-by: Axel Rasmussen <axelrasmussen@xxxxxxxxxx>
> ---
> tools/testing/selftests/vm/userfaultfd.c | 54 ++++++++++++++----------
> 1 file changed, 31 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 2a71a91559a7..00d1b7555865 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -346,6 +346,16 @@ static struct uffd_test_ops hugetlb_uffd_test_ops = {
>
> static struct uffd_test_ops *uffd_test_ops;
>
> +static inline uint64_t uffd_minor_feature(void)
> +{
> + if (test_type == TEST_HUGETLB && map_shared)
> + return UFFD_FEATURE_MINOR_HUGETLBFS;
> + else if (test_type == TEST_SHMEM)
> + return UFFD_FEATURE_MINOR_SHMEM;
> + else
> + return 0;
> +}
> +
> static void userfaultfd_open(uint64_t *features)
> {
> struct uffdio_api uffdio_api;
> @@ -406,7 +416,7 @@ static void uffd_test_ctx_clear(void)
> munmap_area((void **)&area_dst_alias);
> }
>
> -static void uffd_test_ctx_init_ext(uint64_t *features)
> +static void uffd_test_ctx_init(uint64_t features)
> {
> unsigned long nr, cpu;
>
> @@ -418,7 +428,7 @@ static void uffd_test_ctx_init_ext(uint64_t *features)
> uffd_test_ops->release_pages(area_src);
> uffd_test_ops->release_pages(area_dst);
>
> - userfaultfd_open(features);
> + userfaultfd_open(&features);
>
> count_verify = malloc(nr_pages * sizeof(unsigned long long));
> if (!count_verify)
> @@ -446,11 +456,6 @@ static void uffd_test_ctx_init_ext(uint64_t *features)
> err("pipe");
> }
>
> -static inline void uffd_test_ctx_init(uint64_t features)
> -{
> - uffd_test_ctx_init_ext(&features);
> -}
> -
> static int my_bcmp(char *str1, char *str2, size_t n)
> {
> unsigned long i;
> @@ -1191,7 +1196,6 @@ static int userfaultfd_minor_test(void)
> void *expected_page;
> char c;
> struct uffd_stats stats = { 0 };
> - uint64_t req_features, features_out;
>
> if (!test_uffdio_minor)
> return 0;
> @@ -1199,21 +1203,7 @@ static int userfaultfd_minor_test(void)
> printf("testing minor faults: ");
> fflush(stdout);
>
> - if (test_type == TEST_HUGETLB)
> - req_features = UFFD_FEATURE_MINOR_HUGETLBFS;
> - else if (test_type == TEST_SHMEM)
> - req_features = UFFD_FEATURE_MINOR_SHMEM;
> - else
> - return 1;
> -
> - features_out = req_features;
> - uffd_test_ctx_init_ext(&features_out);
> - /* If kernel reports required features aren't supported, skip test. */
> - if ((features_out & req_features) != req_features) {
> - printf("skipping test due to lack of feature support\n");
> - fflush(stdout);
> - return 0;
> - }
> + uffd_test_ctx_init(uffd_minor_feature());
>
> uffdio_register.range.start = (unsigned long)area_dst_alias;
> uffdio_register.range.len = nr_pages * page_size;
> @@ -1574,6 +1564,8 @@ unsigned long default_huge_page_size(void)
>
> static void set_test_type(const char *type)
> {
> + uint64_t features = UFFD_API_FEATURES;
> +
> if (!strcmp(type, "anon")) {
> test_type = TEST_ANON;
> uffd_test_ops = &anon_uffd_test_ops;
> @@ -1607,6 +1599,22 @@ static void set_test_type(const char *type)
> if ((unsigned long) area_count(NULL, 0) + sizeof(unsigned long long) * 2
> > page_size)
> err("Impossible to run this test");
> +
> + /*
> + * Whether we can test certain features depends not just on test type,
> + * but also on whether or not this particular kernel supports the
> + * feature.
> + */
> +
> + userfaultfd_open(&features);
> +
> + test_uffdio_wp = test_uffdio_wp &&
> + (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
> + test_uffdio_minor = test_uffdio_minor &&
> + (features & uffd_minor_feature());
> +
> + close(uffd);
> + uffd = -1;
> }
>
> static void sigalrm(int sig)
> --
> 2.33.0.800.g4c38ced690-goog
>
