Re: [PATCH 22/29] selftests/mm: Move uffd pagemap test to unit test

Mike Rapoport
2023-04-11T12:41:58Z

On Thu, Mar 30, 2023 at 12:08:21PM -0400, Peter Xu wrote:
> Move it over and make it split into two tests, one for pagemap and one for
> the new WP_UNPOPULATED (to be a separate one).
>
> The thp pagemap test wasn't really working (with MADV_HUGEPAGE). Let's
> just drop it (since it never really worked anyway..) and leave that for
> later.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>

> ---
> tools/testing/selftests/mm/uffd-stress.c | 166 -------------------
> tools/testing/selftests/mm/uffd-unit-tests.c | 145 ++++++++++++++++
> 2 files changed, 145 insertions(+), 166 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
> index 70cb0619354e..50738a993afc 100644
> --- a/tools/testing/selftests/mm/uffd-stress.c
> +++ b/tools/testing/selftests/mm/uffd-stress.c
> @@ -671,157 +671,6 @@ static int userfaultfd_minor_test(void)
> return stats.missing_faults != 0 || stats.minor_faults != nr_pages;
> }
>
> -static int pagemap_open(void)
> -{
> - int fd = open("/proc/self/pagemap", O_RDONLY);
> -
> - if (fd < 0)
> - err("open pagemap");
> -
> - return fd;
> -}
> -
> -/* This macro let __LINE__ works in err() */
> -#define pagemap_check_wp(value, wp) do { \
> - if (!!(value & PM_UFFD_WP) != wp) \
> - err("pagemap uffd-wp bit error: 0x%"PRIx64, value); \
> - } while (0)
> -
> -static int pagemap_test_fork(bool present)
> -{
> - pid_t child = fork();
> - uint64_t value;
> - int fd, result;
> -
> - if (!child) {
> - /* Open the pagemap fd of the child itself */
> - fd = pagemap_open();
> - value = pagemap_get_entry(fd, area_dst);
> - /*
> - * After fork() uffd-wp bit should be gone as long as we're
> - * without UFFD_FEATURE_EVENT_FORK
> - */
> - pagemap_check_wp(value, false);
> - /* Succeed */
> - exit(0);
> - }
> - waitpid(child, &result, 0);
> - return result;
> -}
> -
> -static void userfaultfd_wp_unpopulated_test(int pagemap_fd)
> -{
> - uint64_t value;
> -
> - /* Test applying pte marker to anon unpopulated */
> - wp_range(uffd, (uint64_t)area_dst, page_size, true);
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, true);
> -
> - /* Test unprotect on anon pte marker */
> - wp_range(uffd, (uint64_t)area_dst, page_size, false);
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, false);
> -
> - /* Test zap on anon marker */
> - wp_range(uffd, (uint64_t)area_dst, page_size, true);
> - if (madvise(area_dst, page_size, MADV_DONTNEED))
> - err("madvise(MADV_DONTNEED) failed");
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, false);
> -
> - /* Test fault in after marker removed */
> - *area_dst = 1;
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, false);
> - /* Drop it to make pte none again */
> - if (madvise(area_dst, page_size, MADV_DONTNEED))
> - err("madvise(MADV_DONTNEED) failed");
> -
> - /* Test read-zero-page upon pte marker */
> - wp_range(uffd, (uint64_t)area_dst, page_size, true);
> - *(volatile char *)area_dst;
> - /* Drop it to make pte none again */
> - if (madvise(area_dst, page_size, MADV_DONTNEED))
> - err("madvise(MADV_DONTNEED) failed");
> -}
> -
> -static void userfaultfd_pagemap_test(unsigned int test_pgsize)
> -{
> - int pagemap_fd;
> - uint64_t value;
> -
> - /* Pagemap tests uffd-wp only */
> - if (!test_uffdio_wp)
> - return;
> -
> - /* Not enough memory to test this page size */
> - if (test_pgsize > nr_pages * page_size)
> - return;
> -
> - printf("testing uffd-wp with pagemap (pgsize=%u): ", test_pgsize);
> - /* Flush so it doesn't flush twice in parent/child later */
> - fflush(stdout);
> -
> - uffd_test_ctx_init(UFFD_FEATURE_WP_UNPOPULATED);
> -
> - if (test_pgsize > page_size) {
> - /* This is a thp test */
> - if (madvise(area_dst, nr_pages * page_size, MADV_HUGEPAGE))
> - err("madvise(MADV_HUGEPAGE) failed");
> - } else if (test_pgsize == page_size) {
> - /* This is normal page test; force no thp */
> - if (madvise(area_dst, nr_pages * page_size, MADV_NOHUGEPAGE))
> - err("madvise(MADV_NOHUGEPAGE) failed");
> - }
> -
> - if (uffd_register(uffd, area_dst, nr_pages * page_size,
> - false, true, false))
> - err("register failed");
> -
> - pagemap_fd = pagemap_open();
> -
> - /* Smoke test WP_UNPOPULATED first when it's still empty */
> - if (test_pgsize == page_size)
> - userfaultfd_wp_unpopulated_test(pagemap_fd);
> -
> - /* Touch the page */
> - *area_dst = 1;
> - wp_range(uffd, (uint64_t)area_dst, test_pgsize, true);
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, true);
> - /* Make sure uffd-wp bit dropped when fork */
> - if (pagemap_test_fork(true))
> - err("Detected stall uffd-wp bit in child");
> -
> - /* Exclusive required or PAGEOUT won't work */
> - if (!(value & PM_MMAP_EXCLUSIVE))
> - err("multiple mapping detected: 0x%"PRIx64, value);
> -
> - if (madvise(area_dst, test_pgsize, MADV_PAGEOUT))
> - err("madvise(MADV_PAGEOUT) failed");
> -
> - /* Uffd-wp should persist even swapped out */
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, true);
> - /* Make sure uffd-wp bit dropped when fork */
> - if (pagemap_test_fork(false))
> - err("Detected stall uffd-wp bit in child");
> -
> - /* Unprotect; this tests swap pte modifications */
> - wp_range(uffd, (uint64_t)area_dst, page_size, false);
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, false);
> -
> - /* Fault in the page from disk */
> - *area_dst = 2;
> - value = pagemap_get_entry(pagemap_fd, area_dst);
> - pagemap_check_wp(value, false);
> -
> - close(pagemap_fd);
> - printf("done\n");
> -}
> -
> static int userfaultfd_stress(void)
> {
> void *area;
> @@ -933,21 +782,6 @@ static int userfaultfd_stress(void)
> uffd_stats_report(uffd_stats, nr_cpus);
> }
>
> - if (test_type == TEST_ANON) {
> - /*
> - * shmem/hugetlb won't be able to run since they have different
> - * behavior on fork() (file-backed memory normally drops ptes
> - * directly when fork), meanwhile the pagemap test will verify
> - * pgtable entry of fork()ed child.
> - */
> - userfaultfd_pagemap_test(page_size);
> - /*
> - * Hard-code for x86_64 for now for 2M THP, as x86_64 is
> - * currently the only one that supports uffd-wp
> - */
> - userfaultfd_pagemap_test(page_size * 512);
> - }
> -
> return userfaultfd_zeropage_test() || userfaultfd_sig_test()
> || userfaultfd_events_test() || userfaultfd_minor_test();
> }
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 007145063363..bcd67bd4ec90 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -200,7 +200,152 @@ static bool uffd_feature_supported(uffd_test_case_t *test)
> test->uffd_feature_required;
> }
>
> +static int pagemap_open(void)
> +{
> + int fd = open("/proc/self/pagemap", O_RDONLY);
> +
> + if (fd < 0)
> + err("open pagemap");
> +
> + return fd;
> +}
> +
> +/* This macro let __LINE__ works in err() */
> +#define pagemap_check_wp(value, wp) do { \
> + if (!!(value & PM_UFFD_WP) != wp) \
> + err("pagemap uffd-wp bit error: 0x%"PRIx64, value); \
> + } while (0)
> +
> +static int pagemap_test_fork(bool present)
> +{
> + pid_t child = fork();
> + uint64_t value;
> + int fd, result;
> +
> + if (!child) {
> + /* Open the pagemap fd of the child itself */
> + fd = pagemap_open();
> + value = pagemap_get_entry(fd, area_dst);
> + /*
> + * After fork() uffd-wp bit should be gone as long as we're
> + * without UFFD_FEATURE_EVENT_FORK
> + */
> + pagemap_check_wp(value, false);
> + /* Succeed */
> + exit(0);
> + }
> + waitpid(child, &result, 0);
> + return result;
> +}
> +
> +static void uffd_wp_unpopulated_test(void)
> +{
> + uint64_t value;
> + int pagemap_fd;
> +
> + if (uffd_register(uffd, area_dst, nr_pages * page_size,
> + false, true, false))
> + err("register failed");
> +
> + pagemap_fd = pagemap_open();
> +
> + /* Test applying pte marker to anon unpopulated */
> + wp_range(uffd, (uint64_t)area_dst, page_size, true);
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, true);
> +
> + /* Test unprotect on anon pte marker */
> + wp_range(uffd, (uint64_t)area_dst, page_size, false);
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, false);
> +
> + /* Test zap on anon marker */
> + wp_range(uffd, (uint64_t)area_dst, page_size, true);
> + if (madvise(area_dst, page_size, MADV_DONTNEED))
> + err("madvise(MADV_DONTNEED) failed");
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, false);
> +
> + /* Test fault in after marker removed */
> + *area_dst = 1;
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, false);
> + /* Drop it to make pte none again */
> + if (madvise(area_dst, page_size, MADV_DONTNEED))
> + err("madvise(MADV_DONTNEED) failed");
> +
> + /* Test read-zero-page upon pte marker */
> + wp_range(uffd, (uint64_t)area_dst, page_size, true);
> + *(volatile char *)area_dst;
> + /* Drop it to make pte none again */
> + if (madvise(area_dst, page_size, MADV_DONTNEED))
> + err("madvise(MADV_DONTNEED) failed");
> +
> + uffd_test_pass();
> +}
> +
> +static void uffd_pagemap_test(void)
> +{
> + int pagemap_fd;
> + uint64_t value;
> +
> + if (uffd_register(uffd, area_dst, nr_pages * page_size,
> + false, true, false))
> + err("register failed");
> +
> + pagemap_fd = pagemap_open();
> +
> + /* Touch the page */
> + *area_dst = 1;
> + wp_range(uffd, (uint64_t)area_dst, page_size, true);
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, true);
> + /* Make sure uffd-wp bit dropped when fork */
> + if (pagemap_test_fork(true))
> + err("Detected stall uffd-wp bit in child");
> +
> + /* Exclusive required or PAGEOUT won't work */
> + if (!(value & PM_MMAP_EXCLUSIVE))
> + err("multiple mapping detected: 0x%"PRIx64, value);
> +
> + if (madvise(area_dst, page_size, MADV_PAGEOUT))
> + err("madvise(MADV_PAGEOUT) failed");
> +
> + /* Uffd-wp should persist even swapped out */
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, true);
> + /* Make sure uffd-wp bit dropped when fork */
> + if (pagemap_test_fork(false))
> + err("Detected stall uffd-wp bit in child");
> +
> + /* Unprotect; this tests swap pte modifications */
> + wp_range(uffd, (uint64_t)area_dst, page_size, false);
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, false);
> +
> + /* Fault in the page from disk */
> + *area_dst = 2;
> + value = pagemap_get_entry(pagemap_fd, area_dst);
> + pagemap_check_wp(value, false);
> +
> + close(pagemap_fd);
> + uffd_test_pass();
> +}
> +
> uffd_test_case_t uffd_tests[] = {
> + {
> + .name = "pagemap",
> + .uffd_fn = uffd_pagemap_test,
> + .mem_targets = MEM_ANON,
> + .uffd_feature_required = UFFD_FEATURE_PAGEFAULT_FLAG_WP,
> + },
> + {
> + .name = "wp-unpopulated",
> + .uffd_fn = uffd_wp_unpopulated_test,
> + .mem_targets = MEM_ANON,
> + .uffd_feature_required =
> + UFFD_FEATURE_PAGEFAULT_FLAG_WP | UFFD_FEATURE_WP_UNPOPULATED,
> + },
> };
>
> int main(int argc, char *argv[])
> --
> 2.39.1
>

--
Sincerely yours,
Mike.