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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 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()
- [Low] selftests/mm: PAGE_SIZE macro duplicates psize() and risks redefinition
--

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

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

This patch scales UFFD_TEST_MEM_SIZE in uffd-unit-tests.c based on the
architecture base page size so that move-pmd tests on aarch64 with 64K base
pages have a large enough memory region instead of being skipped.

> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 298609544445e..4037a7160b13d 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -13,8 +13,14 @@
>  
>  #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  PAGE_SIZE                        sysconf(_SC_PAGESIZE)

[Severity: Low]
This isn't a bug, but uffd-unit-tests.c already includes vm_util.h (via
uffd-common.h) and calls psize() in main():

tools/testing/selftests/mm/vm_util.h:psize() {
	if (!__page_size)
		__page_size = sysconf(_SC_PAGESIZE);
	return __page_size;
}

An unguarded PAGE_SIZE macro can also collide with system headers on libcs
such as musl (which defines PAGE_SIZE in limits.h under _GNU_SOURCE), and
sysconf(_SC_PAGESIZE) returns a signed long rather than an unsigned value.

Could this reuse psize() instead of defining a new PAGE_SIZE macro?

> +#define  UFFD_TEST_MEM_SIZE               (PAGE_SIZE * (16 << 10))

[Severity: Medium]
Does scaling UFFD_TEST_MEM_SIZE globally inflate memory allocation, runtime,
and HugeTLB reservations across the entire test suite?

In main(), UFFD_TEST_MEM_SIZE is applied to 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;
    ...
}

And uffd_setup_hugetlb() reserves twice UFFD_TEST_MEM_SIZE in HugeTLB pages
up front for the entire test binary execution:

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, only the PMD-granularity tests (such as move-pmd
and move-pmd-split in uffd_move_test_common(), which align area_src and
area_dst up to pmd_psize()) need a buffer of at least 2 * pmd_psize().
Scaling UFFD_TEST_MEM_SIZE globally increases every test on 64K base pages
from 32 MiB (512 pages) to 1 GiB (16,384 pages) per area (2 GiB total for
area_src and area_dst), forcing every base-page test to fault in 16,384
pages and scan 1 GiB byte-by-byte in helpers like check_memory_contents().

It also raises the HugeTLB reservation in uffd_setup_hugetlb() from 64 MiB
to 2 GiB on 64K-page systems with 2 MiB or 16 MiB default hugepages (such as
powerpc64 or arm64 cont-PTE hugepages), which causes all hugetlb tests to be
skipped if 2 GiB of hugepages cannot be reserved, or pins 2 GiB of RAM in
 the HugeTLB pool while subsequent anon/shmem tests allocate another 2 GiB.
Additionally, PMD size is not a fixed multiple of base page size across
architectures (powerpc64 with 64K pages has a 2 MiB or 16 MiB PMD size, and
4K-page systems double from 32 MiB to 64 MiB).

Since struct uffd_test_case_ops provides a pre_alloc hook invoked in
uffd_test_ctx_init() before allocate_area(), would it be better to keep
UFFD_TEST_MEM_SIZE at 32 MiB and adjust gopts->nr_pages based on
2 * pmd_psize() in uffd_move_test_pmd_case_ops (and any other PMD-sized test)?

[Severity: Medium]
Can setting UFFD_TEST_MEM_SIZE to 1 GiB on 64K-page kernels overflow 32-bit
signed off_t and the 32-bit virtual address space when uffd-unit-tests is
built as a 32-bit binary (for example, a 32-bit compat binary on an
aarch64 or powerpc64 64K-page kernel, or 32-bit powerpc with 64K/256K pages)?

In main() -> uffd_test_ctx_init() -> shmem_allocate_area():

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;
    ...
}

Because tools/testing/selftests/mm/Makefile does not define
_FILE_OFFSET_BITS=64, off_t and size_t are 32-bit on 32-bit builds. When
bytes is 1 GiB (0x40000000), bytes * 2 is 0x80000000, which overflows the
32-bit signed off_t parameter of uffd_mem_fd_create(off_t mem_size, bool hugetlb)
to -2147483648 and causes ftruncate() to fail with EINVAL, while mapping two
regions of size bytes * 2 + hpage_size exceeds the 32-bit address space. On
256K base page kernels (CONFIG_PPC_256K_PAGES), PAGE_SIZE * (16 << 10) also
overflows a 32-bit signed long directly.

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