Re: [PATCH v2] selftests/mm: scale uffd memory region contingent upon base page size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] selftests/mm: global UFFD_TEST_MEM_SIZE scaling inflates all
  tests to 1 GiB
- [Medium] selftests/mm: 32-bit off_t and address space overflow in
  shmem_allocate_area()
--

commit a5aec08279d23de437082c277d91963fe8283b3a
Author: John J. Coleman <jocolema@redhat.com>

selftests/mm: scale uffd memory region contingent upon base page size

This commit scales UFFD_TEST_MEM_SIZE by the architecture base page size in
uffd-unit-tests.c so that PMD-granularity tests have a large enough memory
region on 64K base page systems.

> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 4d9df714d5018..99eda4eefb51d 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -13,8 +13,13 @@
>  
>  #ifdef __NR_userfaultfd
>  
> -/* The unit test doesn't need a large or random size, make it 32MB for now */
> -#define  UFFD_TEST_MEM_SIZE               (32UL << 20)
> +/*
> + * The unit test does not need a large or random size, but the allocation size
> + * needs to scale with base page size per architecture otherwise pmd alignment
> + * tests will fail for architectures like aarch64+64k.  For standard 4k pages, 
> + * this will allocate 64MiB, while allocating 1GiB for 64k page sizes.
> + */
> +#define  UFFD_TEST_MEM_SIZE               (psize() * (16UL << 10))

[Severity: Medium]
As reported by Sashiko on v1 [1], does scaling UFFD_TEST_MEM_SIZE globally
inflate memory consumption and runtime for the entire test suite and reserve
2 GiB of HugeTLB pages on 64K page systems?

In main(), UFFD_TEST_MEM_SIZE is used for every test case and memory type:

tools/testing/selftests/mm/uffd-unit-tests.c:main() {
    ...
    /* Ensure we have at least 2 pages */
    gopts.nr_pages = MAX(UFFD_TEST_MEM_SIZE, gopts.page_size * 2)
        / gopts.page_size;
    ...
}

On 64K base page systems, this increases each test area from 32 MiB (512
pages) to 1 GiB (16,384 pages), allocating 2 GiB (area_src and area_dst) for
every test case, populating all 16,384 pages in uffd_test_ctx_init(), and
scanning 1 GiB byte-by-byte in tests that fault and verify area_dst. On 4K
base page systems, it doubles UFFD_TEST_MEM_SIZE from 32 MiB to 64 MiB.

In addition, uffd_setup_hugetlb() reserves twice UFFD_TEST_MEM_SIZE in
HugeTLB pages for the entire test run:

tools/testing/selftests/mm/uffd-unit-tests.c:uffd_setup_hugetlb() {
    ...
    /* need twice UFFD_TEST_MEM_SIZE, one for src area and one for dst */
    nr_hugepages = 2 * MAX(UFFD_TEST_MEM_SIZE, hp_size * 2) / hp_size;
    hugetlb_set_nr_default_pages(nr_hugepages);
    ...
}

On 64K base page systems where the default hugepage size is smaller than
512 MiB (such as powerpc64 with 2 MiB or 16 MiB hugepages, or arm64 with
2 MiB cont-PTE hugepages), the required HugeTLB reservation jumps from
64 MiB to 2 GiB. If 2 GiB of HugeTLB pages cannot be reserved, all hugetlb
and hugetlb-private tests are skipped; if it succeeds, pinning 2 GiB in
HugeTLB throughout the test run can starve the contiguous 512 MiB THP
allocations needed by move-pmd.

Only the PMD-granularity tests (such as move-pmd and move-pmd-split in
uffd_move_test_common()) require a buffer of at least 2 * pmd_psize() to
accommodate aligning area_src and area_dst up to chunk_size. Would it be
better to keep UFFD_TEST_MEM_SIZE at 32 MiB and instead use the pre_alloc
callback in struct uffd_test_case_ops (for example on
uffd_move_test_pmd_case_ops) to adjust gopts->nr_pages based on
2 * pmd_psize() for the PMD tests?

[Severity: Medium]
As reported by Sashiko on v1 [1], can setting UFFD_TEST_MEM_SIZE to
(psize() * (16UL << 10)) overflow 32-bit signed off_t and the 32-bit virtual
address space when running a 32-bit build on a 64K or 256K page kernel?

Neither tools/testing/selftests/mm/Makefile nor
tools/testing/selftests/lib.mk defines _FILE_OFFSET_BITS=64, so on 32-bit
builds unsigned long, size_t, and off_t are 32-bit. When psize() is 64K,
UFFD_TEST_MEM_SIZE is 0x40000000UL (1 GiB), and on 256K-page kernels
(CONFIG_PPC_256K_PAGES) 262144U * 16384UL overflows 32-bit unsigned long to
0.

In shmem_allocate_area(), bytes * 2 is passed to uffd_mem_fd_create():

tools/testing/selftests/mm/uffd-common.c:shmem_allocate_area() {
    size_t bytes = gopts->nr_pages * gopts->page_size, hpage_size = pmd_psize();
    unsigned long offset = is_src ? 0 : bytes;
    char *p = NULL, *p_alias = NULL;
    int mem_fd = uffd_mem_fd_create(bytes * 2, false);
    size_t region_size = bytes * 2 + hpage_size;
    ...
}

tools/testing/selftests/mm/uffd-common.c:uffd_mem_fd_create() {
    ...
    if (ftruncate(mem_fd, mem_size))
        err("ftruncate");
    ...
}

When bytes is 1 GiB (0x40000000UL), bytes * 2 (0x80000000UL) overflows the
32-bit signed off_t parameter mem_size in uffd_mem_fd_create() to
-2147483648 (and similarly for size * 2 in hugetlb_allocate_area()), causing
ftruncate() to fail with EINVAL and abort the test suite via
err("ftruncate"). Furthermore, allocating both area_src and area_dst in
shmem_allocate_area() reserves two virtual memory regions of size
bytes * 2 + hpage_size (>= 4 GiB total), which cannot fit in a 32-bit
virtual address space.

[1] https://lore.kernel.org/r/20261103202049.499206-1-jocolema@redhat.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105175407.638242-1-jocolema@redhat.com?part=1