On Thu, Nov 13, 2025 at 03:20:33PM +0100, Mehdi Ben Hadj Khelifa wrote: > The current shmem_allocate_area() implementation uses a hardcoded virtual > base address (BASE_PMD_ADDR) as a hint for mmap() when creating shmem-backed > test areas. This approach is fragile and may fail on systems with ASLR or > different virtual memory layouts, where the chosen address is unavailable. > > Replace the static base address with a dynamically reserved address range > obtained via mmap(NULL, ..., PROT_NONE). The memfd-backed areas and their > alias are then mapped into that reserved region using MAP_FIXED, preserving > the original layout and aliasing semantics while avoiding collisions with > unrelated mappings. > > This change improves robustness and portability of the test suite without > altering its behavior or coverage. > > Suggested-by: Mike Rapoport <[email protected]> > Signed-off-by: Mehdi Ben Hadj Khelifa <[email protected]>
Reviewed-by: Mike Rapoport (Microsoft) <[email protected]> > --- > Testing(Retested): > A diff between running the mm selftests on 6.18-rc5 from before and after > the change show no regression on x86_64 architecture with 32GB DDR5 RAM. > > ChangeLog: > > Changes from v1: > > -Implemented Mike's suggestions to make cleanup code more clear. > > Link:https://lore.kernel.org/all/[email protected]/ > > tools/testing/selftests/mm/uffd-common.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/mm/uffd-common.c > b/tools/testing/selftests/mm/uffd-common.c > index 994fe8c03923..edd02328f77b 100644 > --- a/tools/testing/selftests/mm/uffd-common.c > +++ b/tools/testing/selftests/mm/uffd-common.c > @@ -10,7 +10,6 @@ > uffd_test_ops_t *uffd_test_ops; > uffd_test_case_ops_t *uffd_test_case_ops; > > -#define BASE_PMD_ADDR ((void *)(1UL << 30)) > > /* pthread_mutex_t starts at page offset 0 */ > pthread_mutex_t *area_mutex(char *area, unsigned long nr, > uffd_global_test_opts_t *gopts) > @@ -142,30 +141,37 @@ static int shmem_allocate_area(uffd_global_test_opts_t > *gopts, void **alloc_area > 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; > > - /* TODO: clean this up. Use a static addr is ugly */ > - p = BASE_PMD_ADDR; > - if (!is_src) > - /* src map + alias + interleaved hpages */ > - p += 2 * (bytes + hpage_size); > + void *reserve = mmap(NULL, region_size, PROT_NONE, MAP_PRIVATE | > MAP_ANONYMOUS, > + -1, 0); > + if (reserve == MAP_FAILED) { > + close(mem_fd); > + return -errno; > + } > + > + p = reserve; > p_alias = p; > p_alias += bytes; > p_alias += hpage_size; /* Prevent src/dst VMA merge */ > > - *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, > + *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_FIXED | > MAP_SHARED, > mem_fd, offset); > if (*alloc_area == MAP_FAILED) { > *alloc_area = NULL; > + munmap(reserve, region_size); > + close(mem_fd); > return -errno; > } > if (*alloc_area != p) > err("mmap of memfd failed at %p", p); > > - area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, > + area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_FIXED | > MAP_SHARED, > mem_fd, offset); > if (area_alias == MAP_FAILED) { > - munmap(*alloc_area, bytes); > *alloc_area = NULL; > + munmap(reserve, region_size); > + close(mem_fd); > return -errno; > } > if (area_alias != p_alias) > -- > 2.51.2 > -- Sincerely yours, Mike.
