Re: [PATCH 2/3] selftests/mm: hmm-tests: don't hardcode THP size to 2MB
From: Sayali Patil
Date: Thu Apr 02 2026 - 02:35:57 EST
On 31/03/26 12:04, Alistair Popple wrote:
Several HMM tests hardcode TWOMEG as the THP size. This is wrong onCan we handle the case where read_pmd_pagesize() returns 0? Currently, it’s passed to ALIGN(), which could produce unexpected results if 0 is returned. Perhaps we can add a fallback to 2MB in that case.
architectures where the PMD size is not 2MB such as arm64 with 64K base
pages where THP is 512MB. Fix this by using read_pmd_pagesize() from
vm_util instead.
While here also replace the custom file_read_ulong() helper used to
parse the default hugetlbfs page size from /proc/meminfo with the
existing default_huge_page_size() from vm_util.
[1] https://lore.kernel.org/linux-mm/8bd0396a-8997-4d2e-a13f-5aac033083d7@xxxxxxxxx/
Fixes: fee9f6d1b8df ("mm/hmm/test: add selftests for HMM")
Fixes: 519071529d2a ("selftests/mm/hmm-tests: new tests for zone device THP migration")
Reported-by: Zenghui Yu <zenghui.yu@xxxxxxxxx>
Closes: https://lore.kernel.org/linux-mm/8bd0396a-8997-4d2e-a13f-5aac033083d7@xxxxxxxxx/
Signed-off-by: Alistair Popple <apopple@xxxxxxxxxx>
---
tools/testing/selftests/mm/hmm-tests.c | 83 +++++---------------------
1 file changed, 16 insertions(+), 67 deletions(-)
diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c
index e8328c89d855..788689497e92 100644
--- a/tools/testing/selftests/mm/hmm-tests.c
+++ b/tools/testing/selftests/mm/hmm-tests.c
@@ -34,6 +34,7 @@
*/
#include <lib/test_hmm_uapi.h>
#include <mm/gup_test.h>
+#include <mm/vm_util.h>
struct hmm_buffer {
void *ptr;
@@ -548,7 +549,7 @@ TEST_F(hmm, anon_write_child)
for (migrate = 0; migrate < 2; ++migrate) {
for (use_thp = 0; use_thp < 2; ++use_thp) {
- npages = ALIGN(use_thp ? TWOMEG : HMM_BUFFER_SIZE,
+ npages = ALIGN(use_thp ? read_pmd_pagesize() : HMM_BUFFER_SIZE,
self->page_size) >> self->page_shift;
ASSERT_NE(npages, 0);
size = npages << self->page_shift;
@@ -728,7 +729,7 @@ TEST_F(hmm, anon_write_huge)
int *ptr;
int ret;
- size = 2 * TWOMEG;
+ size = 2 * read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
@@ -744,7 +745,7 @@ TEST_F(hmm, anon_write_huge)
buffer->fd, 0);
ASSERT_NE(buffer->ptr, MAP_FAILED);
- size = TWOMEG;
+ size /= 2;
npages = size >> self->page_shift;
map = (void *)ALIGN((uintptr_t)buffer->ptr, size);
ret = madvise(map, size, MADV_HUGEPAGE);
@@ -770,54 +771,6 @@ TEST_F(hmm, anon_write_huge)
hmm_buffer_free(buffer);
}
-/*
- * Read numeric data from raw and tagged kernel status files. Used to read
- * /proc and /sys data (without a tag) and from /proc/meminfo (with a tag).
- */
-static long file_read_ulong(char *file, const char *tag)
-{
- int fd;
- char buf[2048];
- int len;
- char *p, *q;
- long val;
-
- fd = open(file, O_RDONLY);
- if (fd < 0) {
- /* Error opening the file */
- return -1;
- }
-
- len = read(fd, buf, sizeof(buf));
- close(fd);
- if (len < 0) {
- /* Error in reading the file */
- return -1;
- }
- if (len == sizeof(buf)) {
- /* Error file is too large */
- return -1;
- }
- buf[len] = '\0';
-
- /* Search for a tag if provided */
- if (tag) {
- p = strstr(buf, tag);
- if (!p)
- return -1; /* looks like the line we want isn't there */
- p += strlen(tag);
- } else
- p = buf;
-
- val = strtol(p, &q, 0);
- if (*q != ' ') {
- /* Error parsing the file */
- return -1;
- }
-
- return val;
-}
-
/*
* Write huge TLBFS page.
*/
@@ -826,15 +779,13 @@ TEST_F(hmm, anon_write_hugetlbfs)
struct hmm_buffer *buffer;
unsigned long npages;
unsigned long size;
- unsigned long default_hsize;
+ unsigned long default_hsize = default_huge_page_size();
unsigned long i;
int *ptr;
int ret;
- default_hsize = file_read_ulong("/proc/meminfo", "Hugepagesize:");
- if (default_hsize < 0 || default_hsize*1024 < default_hsize)
+ if (!default_hsize)
SKIP(return, "Huge page size could not be determined");
- default_hsize = default_hsize*1024; /* KB to B */
size = ALIGN(TWOMEG, default_hsize);
npages = size >> self->page_shift;
@@ -1606,7 +1557,7 @@ TEST_F(hmm, compound)
struct hmm_buffer *buffer;
unsigned long npages;
unsigned long size;
- unsigned long default_hsize;
+ unsigned long default_hsize = default_huge_page_size();
int *ptr;
unsigned char *m;
int ret;
@@ -1614,10 +1565,8 @@ TEST_F(hmm, compound)
/* Skip test if we can't allocate a hugetlbfs page. */
- default_hsize = file_read_ulong("/proc/meminfo", "Hugepagesize:");
- if (default_hsize < 0 || default_hsize*1024 < default_hsize)
+ if (!default_hsize)
SKIP(return, "Huge page size could not be determined");
- default_hsize = default_hsize*1024; /* KB to B */
size = ALIGN(TWOMEG, default_hsize);
npages = size >> self->page_shift;
@@ -2106,7 +2055,7 @@ TEST_F(hmm, migrate_anon_huge_empty)
int *ptr;
int ret;
- size = TWOMEG;
+ size = read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
@@ -2158,7 +2107,7 @@ TEST_F(hmm, migrate_anon_huge_zero)
int ret;
int val;
- size = TWOMEG;
+ size = read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
@@ -2221,7 +2170,7 @@ TEST_F(hmm, migrate_anon_huge_free)
int *ptr;
int ret;
- size = TWOMEG;
+ size = read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
@@ -2280,7 +2229,7 @@ TEST_F(hmm, migrate_anon_huge_fault)
int *ptr;
int ret;
- size = TWOMEG;
+ size = read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
@@ -2332,7 +2281,7 @@ TEST_F(hmm, migrate_partial_unmap_fault)
{
struct hmm_buffer *buffer;
unsigned long npages;
- unsigned long size = TWOMEG;
+ unsigned long size = read_pmd_pagesize();
unsigned long i;
void *old_ptr;
void *map;
@@ -2398,7 +2347,7 @@ TEST_F(hmm, migrate_remap_fault)
{
struct hmm_buffer *buffer;
unsigned long npages;
- unsigned long size = TWOMEG;
+ unsigned long size = read_pmd_pagesize();
unsigned long i;
void *old_ptr, *new_ptr = NULL;
void *map;
Can we please handle the case where read_pmd_pagesize() returns 0, wherever its used?
Also, looking at the code, since we are changing size, shouldn’t the offsets[] values in migrate_remap_fault and migrate_partial_unmap_fault be adjusted accordingly? Right now they are fixed at {0, 512KB, 1MB}.
@@ -2498,7 +2447,7 @@ TEST_F(hmm, migrate_anon_huge_err)
int *ptr;
int ret;
- size = TWOMEG;
+ size = read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
@@ -2593,7 +2542,7 @@ TEST_F(hmm, migrate_anon_huge_zero_err)
int *ptr;
int ret;
- size = TWOMEG;
+ size = read_pmd_pagesize();
buffer = malloc(sizeof(*buffer));
ASSERT_NE(buffer, NULL);
While reviewing the code, I noticed that the benchmark_thp_migration testcase also hardcodes the size to 2MB. Can we handle this as well?
I’ve updated it to use read_pmd_pagesize(), with a fallback to 2MB if it returns 0.
Additionally, I am capping the maximum number of THPs in benchmark tests to avoid integer overflow (e.g., when thp_size * 128 > INT_MAX on 16MB PMD-sized THP systems). Compute the maximum number of THPs at runtime as INT_MAX / thp_size and use the smaller of this value and 128 for the largest benchmark buffer.
diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c
index 788689497e92..c1c9204dc667 100644
--- a/tools/testing/selftests/mm/hmm-tests.c
+++ b/tools/testing/selftests/mm/hmm-tests.c
@@ -2756,8 +2756,15 @@ static inline int run_migration_benchmark(int fd, int use_thp, size_t buffer_siz
TEST_F_TIMEOUT(hmm, benchmark_thp_migration, 120)
{
struct benchmark_results thp_results, regular_results;
- size_t thp_size = 2 * 1024 * 1024; /* 2MB - typical THP size */
+ size_t thp_size = read_pmd_pagesize();
int iterations = 5;
+ size_t max_thps, num_thps;
+
+ if (!thp_size)
+ thp_size = TWOMEG;
+
+ max_thps = INT_MAX / thp_size;
+ num_thps = (max_thps > 128) ? 128 : max_thps;
printf("\nHMM THP Migration Benchmark\n");
printf("---------------------------\n");
@@ -2765,23 +2772,23 @@ TEST_F_TIMEOUT(hmm, benchmark_thp_migration, 120)
/* Test different buffer sizes */
size_t test_sizes[] = {
- thp_size / 4, /* 512KB - smaller than THP */
- thp_size / 2, /* 1MB - half THP */
- thp_size, /* 2MB - single THP */
- thp_size * 2, /* 4MB - two THPs */
- thp_size * 4, /* 8MB - four THPs */
- thp_size * 8, /* 16MB - eight THPs */
- thp_size * 128, /* 256MB - one twenty eight THPs */
+ thp_size / 4, /* Quarter THP */
+ thp_size / 2, /* half THP */
+ thp_size, /* single THP */
+ thp_size * 2, /* two THPs */
+ thp_size * 4, /* four THPs */
+ thp_size * 8, /* eight THPs */
+ thp_size * num_thps, /* max THPs without overflow */
};
static const char *const test_names[] = {
- "Small Buffer (512KB)",
- "Half THP Size (1MB)",
- "Single THP Size (2MB)",
- "Two THP Size (4MB)",
- "Four THP Size (8MB)",
- "Eight THP Size (16MB)",
- "One twenty eight THP Size (256MB)"
+ "Small Buffer",
+ "Half THP Size",
+ "Single THP Size",
+ "Two THP Size",
+ "Four THP Size",
+ "Eight THP Size",
+ "Large Buffer"
};
int num_tests = ARRAY_SIZE(test_sizes);
--
Thanks,
Sayali