Skip to contentSkip to conversation

kselftest: mm: fix some failure of split_huge_page_test

PATCH Superseded Yeoreum Yun 5 participants 12 messages Sep 21 – Sep 23

Subsystems (5)

v7 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

v7 cover kselftest: mm: fix some failure of split_huge_page_test

split_huge_page_test can fail for the following reasons:

1. During the test, khugepaged may collapse previously split pages again, causing intermittent failures.

2. Since glibc commit 321e1fc73f (“malloc: Enable 2MB THP by default on AArch64”), glibc may call madvise(MADV_HUGEPAGE) for sufficiently large allocations made by memalign(). The underlying VMA may start at a different address from the aligned address returned by memalign(). Moreover, a subsequent madvise(MADV_HUGEPAGE) call does not split the VMA because it already has the same advice.

This causes the test to fail because the check_huge_xxx() helpers incorrectly require the address returned by memalign() to match the VMA start address reported in /proc/self/smaps.

Address these issues by applying MADV_NOHUGEPAGE after faulting in the huge page, preventing khugepaged from collapsing it again, and instead of relying on /proc/self/smaps, use /proc/self/pagemap and /proc/kpageflags to detect huge-page mappings and large folios:

1. If hpage_size == pmd_pagesize, check PAGE_IS_HUGE instead of using check_large_folios(), since only the mapping type matters. This identifies PMD-mapped huge pages.
2. Otherwise, use check_large_folios() to detect large folios. This covers mTHP cases.
3. Check the folio flags according to the type of huge page.

Since check_huge_shmem() was required to distinguish shmem huge pages because /proc/self/smaps reports them using a dedicated “ShmemPmdMapped” entry, as opposed to “FilePmdMapped” for file-backed huge pages.

Now that /proc/self/smaps is no longer used to detect huge pages and /proc/kpageflags is used instead, it is sufficient to distinguish
between file-backed and anonymous pages since the ShmemPmdMapped is also
kind of file-backed.

Therefore, remove check_huge_shmem() and use check_huge_file() instead.

This patch based on mm-unstable

---
Changes in v7:
- change khugepaged's check_huge() callback.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- drop alloc_isolated_mem() patch.
- remove check_huge_shmem().
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- rename madv_nohuge() to disable_khugepaged().
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- reparse commit message.
- introduce alloc_isolated_mem().
- make madv_nohuge() helper according to suggestion.
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- change message in case of failure of madvise() with MADV_NOHUGEPAGE.
- fix collapse_single_mthp (mthp_khugepaged:anon) test case failure.
- Link to v2: https://lore.kernel.org/all/[email protected]/

Changes in v2:
- rebase to mm-unstable.
- add message in case of failure of madvise() with MADV_NOHUGEPAGE.
- fix wrong setup expected_huge in check_huge_shmem().
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Yeoreum Yun (3):
kselftest: mm: prevent random failure of huge page split for khugepaged
kselftest: mm: replace usage of /proc/self/smaps for check_huge_xxx() helper
kselftest: mm: remove check_huge_shmem()

 tools/testing/selftests/mm/folio_split_race_test.c |   2 +-
 tools/testing/selftests/mm/khugepaged.c            |  38 +-----
 tools/testing/selftests/mm/split_huge_page_test.c  |  19 +++
 tools/testing/selftests/mm/uffd-common.c           |   4 +-
 tools/testing/selftests/mm/vm_util.c               | 134 ++++++++++++---------
 tools/testing/selftests/mm/vm_util.h               |   1 -
 6 files changed, 108 insertions(+), 90 deletions(-)

---
base-commit: 8d61431ed2607386b427752505379536eb634ce8
change-id: 20260820-fix_split-f44939ec44b8

Best regards,

Signature · 2 lines
Sincerely,
Yeoreum Yun
#1 lore raw

PATCH v7 1/3 kselftest: mm: prevent random failure of huge page split for khugepaged

There're some random failure for split_huge_page_test when khugepaged collapses pages into pmd again which had split by the test.

Prevent the khugepaged's collapses for split page by setting the mapped pmd-huge-page with MADV_NOHUGEPAGE before split.

---
 tools/testing/selftests/mm/split_huge_page_test.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
tools/testing/selftests/mm/split_huge_page_test.c +19 -0
index c01d227d7fd6d..cba327f778734 100644
@@ -167,6 +167,18 @@ static char *allocate_zero_filled_hugepage(size_t len)
 	return result;
 }
 
+static void disable_khugepaged(void *addr, size_t len)
+{
+	/*
+	 * Disables khugepaged from collapsing THPs in range, existing THP
+	 * pages remain.
+	 */
+	if (!madvise(addr, len, MADV_NOHUGEPAGE))
+		return;
+
+	ksft_exit_fail_msg("MADV_NOHUGEPAGE failed, err=%d\n", errno);
+}
+
 static void verify_rss_anon_split_huge_page_all_zeroes(char *one_page, int nr_hpages, size_t len)
 {
 	unsigned long rss_anon_before, rss_anon_after;
@@ -179,6 +191,8 @@ static void verify_rss_anon_split_huge_page_all_zeroes(char *one_page, int nr_hp
 	if (!rss_anon_before)
 		ksft_exit_fail_msg("No RssAnon is allocated before split\n");
 
+	disable_khugepaged(one_page, len);
+
 	/* split all THPs */
 	write_debugfs(PID_FMT, getpid(), (uint64_t)one_page,
 		      (uint64_t)one_page + len, 0);
@@ -226,6 +240,8 @@ static void split_pmd_thp_to_order(int order)
 	if (!check_huge_anon(one_page, 4 * pmd_pagesize, 4, pmd_pagesize))
 		ksft_exit_fail_msg("No THP is allocated\n");
 
+	disable_khugepaged(one_page, len);
+
 	/* split all THPs */
 	write_debugfs(PID_FMT, getpid(), (uint64_t)one_page,
 		(uint64_t)one_page + len, order);
@@ -274,6 +290,8 @@ static void split_pte_mapped_thp(void)
 		goto out;
 	}
 
+	disable_khugepaged(thp_area, thp_area_size);
+
 	/*
 	 * To challenge spitting code, we will mremap a single page of each
 	 * THP (page[i] of thp[i]) in the thp_area into page_area. This will
@@ -541,6 +559,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
 		ksft_test_result_skip("Pagecache folio split skipped\n");
 		return -2;
 	}
+	disable_khugepaged(*addr, fd_size);
 	return 0;
 err_out_close:
 	close(*fd);
Signature · 1 line
2.43.0
#2 lore raw

PATCH v7 2/3 kselftest: mm: replace usage of /proc/self/smaps for check_huge_xxx() helper

In reply to Yeoreum Yun

Since glibc commit 321e1fc73f (“malloc: Enable 2MB THP by default on AArch64”), glibc may call madvise(MADV_HUGEPAGE) for sufficiently large allocations made by memalign().

The underlying VMA may start at a different address from the aligned address returned by memalign(). Furthermore, a subsequent madvise(MADV_HUGEPAGE) call does not split the VMA because the flag is already set.

This causes split_huge_page_test to fail because the check_huge_xxx() helpers incorrectly require the address returned by memalign() to match the VMA start address reported in /proc/self/smaps.

Instead of relying on /proc/self/smaps, use /proc/self/pagemap and /proc/kpageflags to detect huge-page mappings and large folios:

1. If hpage_size == pmd_pagesize, check PAGE_IS_HUGE instead of using check_large_folios(), since only the mapping type matters. This identifies PMD-mapped huge pages.
2. Otherwise, use check_large_folios() to detect large folios. This covers mTHP cases.
3. Check the folio flags according to the type of huge page.

Since shmem pages are also file-backed, simply check whether the page is file-backed.

---
 tools/testing/selftests/mm/vm_util.c | 137 +++++++++++++++++++++--------------
 1 file changed, 84 insertions(+), 53 deletions(-)
tools/testing/selftests/mm/vm_util.c +84 -53
index 4821a35630363..a7c5caf6925e0 100644
@@ -351,31 +351,13 @@ char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)
 	return entry;
 }
 
-static bool __check_pmd_huge(void *addr, char *pattern, int nr_hpages,
-		  uint64_t hpage_size)
-{
-	char buffer[MAX_LINE_LENGTH];
-	uint64_t thp = -1;
-	char *entry;
-
-	entry = __get_smap_entry(addr, pattern, buffer, sizeof(buffer));
-	if (!entry)
-		goto err_out;
-
-	if (sscanf(entry, "%9" SCNu64 " kB", &thp) != 1)
-		ksft_exit_fail_msg("Reading smap error\n");
-
-err_out:
-	return thp == (nr_hpages * (hpage_size >> 10));
-}
-
-static bool check_large_folios(void *addr, size_t len, int nr_hpages,
-		uint64_t hpage_size)
+static bool check_large_folios(int pagemap_fd, int kpageflags_fd,
+			       void *addr, size_t len, int nr_hpages,
+			       uint64_t hpage_size)
 {
 	int order = 0, pagesize = getpagesize();
 	unsigned int nr_pages = hpage_size / pagesize;
 	int orders[MAX_NR_ORDERS], status;
-	int pagemap_fd, kpageflags_fd;
 	bool ret = false;
 
 	if (!nr_pages)
@@ -386,15 +368,6 @@ static bool check_large_folios(void *addr, size_t len, int nr_hpages,
 		ksft_exit_fail_msg("invalid order\n");
 
 	memset(orders, 0, sizeof(int) * MAX_NR_ORDERS);
-	pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
-	if (pagemap_fd == -1)
-		ksft_exit_fail_msg("read pagemap fail\n");
-
-	kpageflags_fd = open(KPAGEFLAGS_PATH, O_RDONLY);
-	if (kpageflags_fd == -1) {
-		close(pagemap_fd);
-		ksft_exit_fail_msg("read kpageflags fail\n");
-	}
 
 	status = gather_folio_orders(addr, len, pagemap_fd,
 			kpageflags_fd, orders, MAX_NR_ORDERS);
@@ -405,48 +378,106 @@ static bool check_large_folios(void *addr, size_t len, int nr_hpages,
 		ret = true;
 
 out:
-	close(pagemap_fd);
-	close(kpageflags_fd);
 	return ret;
 }
 
-bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
-{
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+};
 
-	if (!pmd_pagesize)
-		ksft_exit_fail_msg("reading PMD pagesize failed\n");
+static bool check_huge_type(uint64_t categories, uint64_t kpageflags,
+			    enum check_huge_type type)
+{
+	const bool file = categories & PAGE_IS_FILE;
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	switch (type) {
+	case CHECK_HUGE_ANON:
+		return !file;
+	case CHECK_HUGE_FILE:
+		return file;
+	}
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+	return false;
 }
 
-bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+			 uint64_t hpage_size, enum check_huge_type type)
 {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	bool ret = false;
+	int pagemap_fd, kpageflags_fd;
+	int nr_pmd_mappings = 0;
+	uint64_t pmd_pagesize, scan_mapping_size;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_pmd_mapping, allow_nonpresent;
+	char *start = addr;
+	char *end = start + len;
 
+	pmd_pagesize = read_pmd_pagesize();
 	if (!pmd_pagesize)
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "FilePmdMapped:", nr_hpages, hpage_size);
+	check_pmd_mapping = (hpage_size == pmd_pagesize);
+	scan_mapping_size = (nr_hpages > 0) ? hpage_size : psize();
+	/* Some mTHP tests check a partially populated PMD-sized range. */
+	allow_nonpresent = (uint64_t)nr_hpages * hpage_size < len;
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+	pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
+	if (pagemap_fd < 0)
+		ksft_exit_fail_msg("open pagemap fail\n");
+
+	kpageflags_fd = open(KPAGEFLAGS_PATH, O_RDONLY);
+	if (kpageflags_fd < 0)
+		ksft_exit_fail_msg("open kpageflags fail\n");
+
+	if (!check_pmd_mapping &&
+	    !check_large_folios(pagemap_fd, kpageflags_fd,
+				addr, len, nr_hpages, hpage_size))
+		goto out;
+
+	for (; start < end; start += scan_mapping_size) {
+		categories = pagemap_scan_get_categories(pagemap_fd, start);
+		pfn = pagemap_get_pfn(pagemap_fd, start);
+		if (pfn == -1UL) {
+			if (!allow_nonpresent)
+				goto out;
+			else
+				continue;
+		}
+		if (pageflags_get(pfn, kpageflags_fd, &kpf))
+			ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));
+		if (check_pmd_mapping && (categories & PAGE_IS_HUGE))
+			nr_pmd_mappings++;
+		if (kpf & KPF_COMPOUND_TAIL)
+			continue;
+		if (!check_huge_type(categories, kpf, type))
+			goto out;
+	}
+
+	if (check_pmd_mapping && (nr_pmd_mappings != nr_hpages))
+		goto out;
+	ret = true;
+
+out:
+	close(pagemap_fd);
+	close(kpageflags_fd);
+	return ret;
 }
 
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
-
-	if (!pmd_pagesize)
-		ksft_exit_fail_msg("reading PMD pagesize failed\n");
+	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_ANON);
+}
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "ShmemPmdMapped:", nr_hpages, hpage_size);
+bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+{
+	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
+}
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+{
+	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
 }
 
 int64_t allocate_transhuge(void *ptr, int pagemap_fd)
Signature · 1 line
2.43.0
#3 lore raw

Re: [PATCH v7 2/3] kselftest: mm: replace usage of /proc/self/smaps for check_huge_xxx() helper

Quoted 23 lines · Yeoreum Yun

Since glibc commit 321e1fc73f (“malloc: Enable 2MB THP by default on AArch64”), glibc may call madvise(MADV_HUGEPAGE) for sufficiently large allocations made by memalign().

The underlying VMA may start at a different address from the aligned address returned by memalign(). Furthermore, a subsequent madvise(MADV_HUGEPAGE) call does not split the VMA because the flag is already set.

This causes split_huge_page_test to fail because the check_huge_xxx() helpers incorrectly require the address returned by memalign() to match the VMA start address reported in /proc/self/smaps.

Instead of relying on /proc/self/smaps, use /proc/self/pagemap and /proc/kpageflags to detect huge-page mappings and large folios:

1. If hpage_size == pmd_pagesize, check PAGE_IS_HUGE instead of using check_large_folios(), since only the mapping type matters. This identifies PMD-mapped huge pages.
2. Otherwise, use check_large_folios() to detect large folios. This covers mTHP cases.
3. Check the folio flags according to the type of huge page.

Quoted 5 lines

Since shmem pages are also file-backed, simply check whether the page is file-backed.

Do we need a Fixes: tag?

Quoted 26 lines
---
 tools/testing/selftests/mm/vm_util.c | 137 +++++++++++++++++++++--------------
 1 file changed, 84 insertions(+), 53 deletions(-)
tools/testing/selftests/mm/vm_util.c +0 -13
index 4821a35630363..a7c5caf6925e0 100644
@@ -351,31 +351,13 @@ char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)
 	return entry;
 }
 
-static bool __check_pmd_huge(void *addr, char *pattern, int nr_hpages,
-		  uint64_t hpage_size)
-{
-	char buffer[MAX_LINE_LENGTH];
-	uint64_t thp = -1;
-	char *entry;
-
-	entry = __get_smap_entry(addr, pattern, buffer, sizeof(buffer));
-	if (!entry)
-		goto err_out;
-
-	if (sscanf(entry, "%9" SCNu64 " kB", &thp) != 1)
-		ksft_exit_fail_msg("Reading smap error\n");

tools/testing/selftests/mm/vm_util.c char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len) · line 356

Quoted 10 lines
tools/testing/selftests/mm/vm_util.c +3 -7
-
-err_out:
-	return thp == (nr_hpages * (hpage_size >> 10));
-}
-
-static bool check_large_folios(void *addr, size_t len, int nr_hpages,
-		uint64_t hpage_size)
+static bool check_large_folios(int pagemap_fd, int kpageflags_fd,
+			       void *addr, size_t len, int nr_hpages,
+			       uint64_t hpage_size)

The function used two tab indent for a reason ;)

Use two tabs also in the functions you change below.

[...]

Quoted 15 lines
tools/testing/selftests/mm/vm_util.c +9 -2
-bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+			 uint64_t hpage_size, enum check_huge_type type)
 
 
 {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	bool ret = false;
+	int pagemap_fd, kpageflags_fd;
+	int nr_pmd_mappings = 0;
+	uint64_t pmd_pagesize, scan_mapping_size;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_pmd_mapping, allow_nonpresent;

tools/testing/selftests/mm/vm_util.c char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)

Quoted 10 lines
tools/testing/selftests/mm/vm_util.c +4 -2
+	char *start = addr;
+	char *end = start + len;
 
+	pmd_pagesize = read_pmd_pagesize();
 	if (!pmd_pagesize)
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "FilePmdMapped:", nr_hpages, hpage_size);
+	check_pmd_mapping = (hpage_size == pmd_pagesize);

No need for ()

Quoted 29 lines
tools/testing/selftests/mm/vm_util.c +27 -1
+	scan_mapping_size = (nr_hpages > 0) ? hpage_size : psize();
+	/* Some mTHP tests check a partially populated PMD-sized range. */
+	allow_nonpresent = (uint64_t)nr_hpages * hpage_size < len;
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+	pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
+	if (pagemap_fd < 0)
+		ksft_exit_fail_msg("open pagemap fail\n");
+
+	kpageflags_fd = open(KPAGEFLAGS_PATH, O_RDONLY);
+	if (kpageflags_fd < 0)
+		ksft_exit_fail_msg("open kpageflags fail\n");
+
+	if (!check_pmd_mapping &&
+	    !check_large_folios(pagemap_fd, kpageflags_fd,
+				addr, len, nr_hpages, hpage_size))
+		goto out;
+
+	for (; start < end; start += scan_mapping_size) {
+		categories = pagemap_scan_get_categories(pagemap_fd, start);
+		pfn = pagemap_get_pfn(pagemap_fd, start);
+		if (pfn == -1UL) {
+			if (!allow_nonpresent)
+				goto out;
+			else
+				continue;
+		}
+		if (pageflags_get(pfn, kpageflags_fd, &kpf))
+			ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));

tools/testing/selftests/mm/vm_util.c

Quoted 10 lines
tools/testing/selftests/mm/vm_util.c +10 -0
+		if (check_pmd_mapping && (categories & PAGE_IS_HUGE))
+			nr_pmd_mappings++;
+		if (kpf & KPF_COMPOUND_TAIL)
+			continue;
+		if (!check_huge_type(categories, kpf, type))
+			goto out;
+	}
+
+	if (check_pmd_mapping && (nr_pmd_mappings != nr_hpages))
+		goto out;

There is way too much happening here, which makes this rather hard to review.

Why can't we just focus on the smaps change for now?

I.e., something like the following (untested)

tools/testing/selftests/mm/vm_util.c +30 -29
index 4821a35630363..5d348d6985026 100644
@@ -351,24 +351,6 @@ char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)
 	return entry;
 }
 
-static bool __check_pmd_huge(void *addr, char *pattern, int nr_hpages,
-		  uint64_t hpage_size)
-{
-	char buffer[MAX_LINE_LENGTH];
-	uint64_t thp = -1;
-	char *entry;
-
-	entry = __get_smap_entry(addr, pattern, buffer, sizeof(buffer));
-	if (!entry)
-		goto err_out;
-
-	if (sscanf(entry, "%9" SCNu64 " kB", &thp) != 1)
-		ksft_exit_fail_msg("Reading smap error\n");
-
-err_out:
-	return thp == (nr_hpages * (hpage_size >> 10));
-}
-
 static bool check_large_folios(void *addr, size_t len, int nr_hpages,
 		uint64_t hpage_size)
 {
@@ -410,20 +392,34 @@ static bool check_large_folios(void *addr, size_t len, int nr_hpages,
 	return ret;
 }
 
-bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+static bool __check_pmds(void *start, size_t len, int expected_file_pmds,
+		int expected_anon_pmds, uint64_t pmd_pagesize)
 {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int found_anon_pmds = 0, found_file_pmds = 0;
+	int pagemap_fd;
+	void *addr;
 
-	if (!pmd_pagesize)
-		ksft_exit_fail_msg("reading PMD pagesize failed\n");
+	pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
+	if (pagemap_fd < 0)
+		ksft_exit_fail_msg("open pagemap fail\n");
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	for (addr = start; addr < start + len; addr += pmd_pagesize) {
+		const uint64_t categories = pagemap_scan_get_categories(pagemap_fd, addr);
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+		if (!(categories & PAGE_IS_HUGE))
+			continue;
+		if (categories & PAGE_IS_FILE)
+			found_file_pmds++;
+		else
+			found_anon_pmds++;
+	}
+
+	close(pagemap_fd);
+	return found_anon_pmds == expected_anon_pmds &&
+	       found_file_pmds == expected_file_pmds;
 }
 
-bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
 	uint64_t pmd_pagesize = read_pmd_pagesize();
 
@@ -431,12 +427,12 @@ bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
 
 	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "FilePmdMapped:", nr_hpages, hpage_size);
+		return __check_pmds(addr, len, 0, nr_hpages, pmd_pagesize);
 
 	return check_large_folios(addr, len, nr_hpages, hpage_size);
 }
 
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
 	uint64_t pmd_pagesize = read_pmd_pagesize();
 
@@ -444,11 +440,16 @@ bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
 
 	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "ShmemPmdMapped:", nr_hpages, hpage_size);
+		return __check_pmds(addr, len, nr_hpages, 0, pmd_pagesize);
 
 	return check_large_folios(addr, len, nr_hpages, hpage_size);
 }
 
+bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+{
+	return check_huge_file(addr, len, nr_hpages, hpage_size);
+}
+
 int64_t allocate_transhuge(void *ptr, int pagemap_fd)
 {
 	uint64_t ent[2];
Signature · 5 lines
2.43.0
-- 
Cheers,

David
#4 lore raw

Re: [PATCH v7 2/3] kselftest: mm: replace usage of /proc/self/smaps for check_huge_xxx() helper

Hi,

Quoted 24 lines · Yeoreum Yun
Quoted 23 lines

Since glibc commit 321e1fc73f (“malloc: Enable 2MB THP by default on AArch64”), glibc may call madvise(MADV_HUGEPAGE) for sufficiently large allocations made by memalign().

The underlying VMA may start at a different address from the aligned address returned by memalign(). Furthermore, a subsequent madvise(MADV_HUGEPAGE) call does not split the VMA because the flag is already set.

This causes split_huge_page_test to fail because the check_huge_xxx() helpers incorrectly require the address returned by memalign() to match the VMA start address reported in /proc/self/smaps.

Instead of relying on /proc/self/smaps, use /proc/self/pagemap and /proc/kpageflags to detect huge-page mappings and large folios:

1. If hpage_size == pmd_pagesize, check PAGE_IS_HUGE instead of using check_large_folios(), since only the mapping type matters. This identifies PMD-mapped huge pages.
2. Otherwise, use check_large_folios() to detect large folios. This covers mTHP cases.
3. Check the folio flags according to the type of huge page.

Quoted 7 lines
Quoted 5 lines

Since shmem pages are also file-backed, simply check whether the page is file-backed.

Do we need a Fixes: tag?

Since this change is required for the chaning of behavior of glibc, I don't think we don't need to add Fixes tag in here?

Quoted 40 lines
Quoted 27 lines
---
 tools/testing/selftests/mm/vm_util.c | 137 +++++++++++++++++++++--------------
 1 file changed, 84 insertions(+), 53 deletions(-)
tools/testing/selftests/mm/vm_util.c +0 -13
index 4821a35630363..a7c5caf6925e0 100644
@@ -351,31 +351,13 @@ char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)
 	return entry;
 }
 
-static bool __check_pmd_huge(void *addr, char *pattern, int nr_hpages,
-		  uint64_t hpage_size)
-{
-	char buffer[MAX_LINE_LENGTH];
-	uint64_t thp = -1;
-	char *entry;
-
-	entry = __get_smap_entry(addr, pattern, buffer, sizeof(buffer));
-	if (!entry)
-		goto err_out;
-
-	if (sscanf(entry, "%9" SCNu64 " kB", &thp) != 1)
-		ksft_exit_fail_msg("Reading smap error\n");

tools/testing/selftests/mm/vm_util.c char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len) · line 356

Quoted 10 lines
tools/testing/selftests/mm/vm_util.c +3 -7
-
-err_out:
-	return thp == (nr_hpages * (hpage_size >> 10));
-}
-
-static bool check_large_folios(void *addr, size_t len, int nr_hpages,
-		uint64_t hpage_size)
+static bool check_large_folios(int pagemap_fd, int kpageflags_fd,
+			       void *addr, size_t len, int nr_hpages,
+			       uint64_t hpage_size)

The function used two tab indent for a reason ;)

Quoted 1 line

Use two tabs also in the functions you change below.

Okay.

Quoted 23 lines
Quoted 23 lines
tools/testing/selftests/mm/vm_util.c +12 -2
-bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+			 uint64_t hpage_size, enum check_huge_type type)
 
 
 {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	bool ret = false;
+	int pagemap_fd, kpageflags_fd;
+	int nr_pmd_mappings = 0;
+	uint64_t pmd_pagesize, scan_mapping_size;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_pmd_mapping, allow_nonpresent;
+	char *start = addr;
+	char *end = start + len;
 
+	pmd_pagesize = read_pmd_pagesize();
 	if (!pmd_pagesize)
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
Quoted 5 lines

tools/testing/selftests/mm/vm_util.c

Quoted 3 lines
tools/testing/selftests/mm/vm_util.c +1 -2
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "FilePmdMapped:", nr_hpages, hpage_size);
+	check_pmd_mapping = (hpage_size == pmd_pagesize);

No need for ()

I see.

Quoted 44 lines
Quoted 30 lines
tools/testing/selftests/mm/vm_util.c +27 -1
+	scan_mapping_size = (nr_hpages > 0) ? hpage_size : psize();
+	/* Some mTHP tests check a partially populated PMD-sized range. */
+	allow_nonpresent = (uint64_t)nr_hpages * hpage_size < len;
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+	pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
+	if (pagemap_fd < 0)
+		ksft_exit_fail_msg("open pagemap fail\n");
+
+	kpageflags_fd = open(KPAGEFLAGS_PATH, O_RDONLY);
+	if (kpageflags_fd < 0)
+		ksft_exit_fail_msg("open kpageflags fail\n");
+
+	if (!check_pmd_mapping &&
+	    !check_large_folios(pagemap_fd, kpageflags_fd,
+				addr, len, nr_hpages, hpage_size))
+		goto out;
+
+	for (; start < end; start += scan_mapping_size) {
+		categories = pagemap_scan_get_categories(pagemap_fd, start);
+		pfn = pagemap_get_pfn(pagemap_fd, start);
+		if (pfn == -1UL) {
+			if (!allow_nonpresent)
+				goto out;
+			else
+				continue;
+		}
+		if (pageflags_get(pfn, kpageflags_fd, &kpf))
+			ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));

tools/testing/selftests/mm/vm_util.c

Quoted 10 lines
tools/testing/selftests/mm/vm_util.c +10 -0
+		if (check_pmd_mapping && (categories & PAGE_IS_HUGE))
+			nr_pmd_mappings++;
+		if (kpf & KPF_COMPOUND_TAIL)
+			continue;
+		if (!check_huge_type(categories, kpf, type))
+			goto out;
+	}
+
+	if (check_pmd_mapping && (nr_pmd_mappings != nr_hpages))
+		goto out;

There is way too much happening here, which makes this rather hard to review.

Quoted 1 line

Why can't we just focus on the smaps change for now?

Yeap. I think we can simplify as your suggestion with a little bit change. I'll spin again.

Thanks.

Quoted 115 lines

I.e., something like the following (untested)

tools/testing/selftests/mm/vm_util.c +30 -29
index 4821a35630363..5d348d6985026 100644
@@ -351,24 +351,6 @@ char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)
 	return entry;
 }
 
-static bool __check_pmd_huge(void *addr, char *pattern, int nr_hpages,
-		  uint64_t hpage_size)
-{
-	char buffer[MAX_LINE_LENGTH];
-	uint64_t thp = -1;
-	char *entry;
-
-	entry = __get_smap_entry(addr, pattern, buffer, sizeof(buffer));
-	if (!entry)
-		goto err_out;
-
-	if (sscanf(entry, "%9" SCNu64 " kB", &thp) != 1)
-		ksft_exit_fail_msg("Reading smap error\n");
-
-err_out:
-	return thp == (nr_hpages * (hpage_size >> 10));
-}
-
 static bool check_large_folios(void *addr, size_t len, int nr_hpages,
 		uint64_t hpage_size)
 {
@@ -410,20 +392,34 @@ static bool check_large_folios(void *addr, size_t len, int nr_hpages,
 	return ret;
 }
 
-bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+static bool __check_pmds(void *start, size_t len, int expected_file_pmds,
+		int expected_anon_pmds, uint64_t pmd_pagesize)
 {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int found_anon_pmds = 0, found_file_pmds = 0;
+	int pagemap_fd;
+	void *addr;
 
-	if (!pmd_pagesize)
-		ksft_exit_fail_msg("reading PMD pagesize failed\n");
+	pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
+	if (pagemap_fd < 0)
+		ksft_exit_fail_msg("open pagemap fail\n");
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	for (addr = start; addr < start + len; addr += pmd_pagesize) {
+		const uint64_t categories = pagemap_scan_get_categories(pagemap_fd, addr);
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+		if (!(categories & PAGE_IS_HUGE))
+			continue;
+		if (categories & PAGE_IS_FILE)
+			found_file_pmds++;
+		else
+			found_anon_pmds++;
+	}
+
+	close(pagemap_fd);
+	return found_anon_pmds == expected_anon_pmds &&
+	       found_file_pmds == expected_file_pmds;
 }
 
-bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
 	uint64_t pmd_pagesize = read_pmd_pagesize();
 
@@ -431,12 +427,12 @@ bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
 
 	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "FilePmdMapped:", nr_hpages, hpage_size);
+		return __check_pmds(addr, len, 0, nr_hpages, pmd_pagesize);
 
 	return check_large_folios(addr, len, nr_hpages, hpage_size);
 }
 
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
 	uint64_t pmd_pagesize = read_pmd_pagesize();
 
@@ -444,11 +440,16 @@ bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size
 		ksft_exit_fail_msg("reading PMD pagesize failed\n");
 
 	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "ShmemPmdMapped:", nr_hpages, hpage_size);
+		return __check_pmds(addr, len, nr_hpages, 0, pmd_pagesize);
 
 	return check_large_folios(addr, len, nr_hpages, hpage_size);
 }
 
+bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
+{
+	return check_huge_file(addr, len, nr_hpages, hpage_size);
+}
+
 int64_t allocate_transhuge(void *ptr, int pagemap_fd)
 {
 	uint64_t ent[2];
Signature · 5 lines
2.43.0
-- 
Cheers,

David
Signature · 2 lines
Sincerely,
Yeoreum Yun
#5 lore raw

Re: [PATCH v7 2/3] kselftest: mm: replace usage of /proc/self/smaps for check_huge_xxx() helper

Quoted 29 lines · Yeoreum Yun

Hi,

Quoted 27 lines · Yeoreum Yun
Quoted 26 lines

Since glibc commit 321e1fc73f (“malloc: Enable 2MB THP by default on AArch64”), glibc may call madvise(MADV_HUGEPAGE) for sufficiently large allocations made by memalign().

The underlying VMA may start at a different address from the aligned address returned by memalign(). Furthermore, a subsequent madvise(MADV_HUGEPAGE) call does not split the VMA because the flag is already set.

This causes split_huge_page_test to fail because the check_huge_xxx() helpers incorrectly require the address returned by memalign() to match the VMA start address reported in /proc/self/smaps.

Instead of relying on /proc/self/smaps, use /proc/self/pagemap and /proc/kpageflags to detect huge-page mappings and large folios:

1. If hpage_size == pmd_pagesize, check PAGE_IS_HUGE instead of using check_large_folios(), since only the mapping type matters. This identifies PMD-mapped huge pages.
2. Otherwise, use check_large_folios() to detect large folios. This covers mTHP cases.
3. Check the folio flags according to the type of huge page.

Since shmem pages are also file-backed, simply check whether the page is file-backed.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

Do we need a Fixes: tag?

Since this change is required for the chaning of behavior of glibc, I don't think we don't need to add Fixes tag in here?

Our test made wrong assumptions on the underlying behavior of the user space allocator (no VMA merging would happen). So I'd argue that we should add a
Fixes: for the test.

Signature · 3 lines
Cheers,

David
#6 lore raw

Re: [PATCH v7 2/3] kselftest: mm: replace usage of /proc/self/smaps for check_huge_xxx() helper

Quoted 33 lines · David Hildenbrand (Arm)
Quoted 32 lines · Yeoreum Yun

Hi,

Quoted 30 lines · Yeoreum Yun
Quoted 28 lines

Since glibc commit 321e1fc73f (“malloc: Enable 2MB THP by default on AArch64”), glibc may call madvise(MADV_HUGEPAGE) for sufficiently large allocations made by memalign().

The underlying VMA may start at a different address from the aligned address returned by memalign(). Furthermore, a subsequent madvise(MADV_HUGEPAGE) call does not split the VMA because the flag is already set.

This causes split_huge_page_test to fail because the check_huge_xxx() helpers incorrectly require the address returned by memalign() to match the VMA start address reported in /proc/self/smaps.

Instead of relying on /proc/self/smaps, use /proc/self/pagemap and /proc/kpageflags to detect huge-page mappings and large folios:

1. If hpage_size == pmd_pagesize, check PAGE_IS_HUGE instead of using check_large_folios(), since only the mapping type matters. This identifies PMD-mapped huge pages.
2. Otherwise, use check_large_folios() to detect large folios. This covers mTHP cases.
3. Check the folio flags according to the type of huge page.

Since shmem pages are also file-backed, simply check whether the page is file-backed.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Do we need a Fixes: tag?

Since this change is required for the chaning of behavior of glibc, I don't think we don't need to add Fixes tag in here?

Our test made wrong assumptions on the underlying behavior of the user space allocator (no VMA merging would happen). So I'd argue that we should add a
Fixes: for the test.

Okay. while I do
- add intermediate patch before this as you suggeeted.
- on the intermediate patch, add fix tag.
- based the patch, rebase this patch to add anon/file check for other check_large_folios() type.

Thanks.

Quoted 5 lines
Signature · 4 lines
-- 
Cheers,

David
Signature · 2 lines
Sincerely,
Yeoreum Yun
#7 lore raw

PATCH v7 3/3 kselftest: mm: remove check_huge_shmem()

In reply to Yeoreum Yun

check_huge_shmem() was required to distinguish shmem huge pages because /proc/self/smaps reports them using a dedicated “ShmemPmdMapped” entry, as opposed to “FilePmdMapped” for file-backed huge pages.

Now that /proc/self/smaps is no longer used to detect huge pages and /proc/kpageflags is used instead, it is sufficient to distinguish between file-backed and anonymous pages since the ShmemPmdMapped is also kind of file-backed.

Therefore, remove check_huge_shmem() and use check_huge_file() instead and cleanup khugepaged's check_huge operation in mem_ops.

---
 tools/testing/selftests/mm/folio_split_race_test.c |  2 +-
 tools/testing/selftests/mm/khugepaged.c            | 38 ++++------------------
 tools/testing/selftests/mm/uffd-common.c           |  4 +--
 tools/testing/selftests/mm/vm_util.c               |  5 ---
 tools/testing/selftests/mm/vm_util.h               |  1 -
 5 files changed, 9 insertions(+), 41 deletions(-)
tools/testing/selftests/mm/folio_split_race_test.c +1 -1
index 1960635a953eb..4c0d3b6a411b8 100644
@@ -182,7 +182,7 @@ static uint64_t run_iteration(void)
 	for (i = 0; i < TOTAL_PAGES; i++)
 		fill_page(mmap_base, i);
 
-	if (!check_huge_shmem(mmap_base, FILE_SIZE, NR_PMD_PAGE, pmd_pagesize))
+	if (!check_huge_file(mmap_base, FILE_SIZE, NR_PMD_PAGE, pmd_pagesize))
 		ksft_exit_fail_msg("No shmem THP is allocated\n");
 
 	if (pthread_barrier_init(&ctl.barrier, NULL, NUM_READER_THREADS + 1) != 0)
tools/testing/selftests/mm/khugepaged.c +6 -32
index f82673f5f6b47..f448278823678 100644
@@ -53,7 +53,7 @@ struct mem_ops {
 	void *(*setup_area)(int nr_hpages);
 	void (*cleanup_area)(void *p, unsigned long size);
 	void (*fault)(void *p, unsigned long start, unsigned long end);
-	bool (*check_huge)(void *addr, size_t len, int nr_hpages, unsigned long hpage_size);
+	bool (*check_huge)(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
 	const char *name;
 };
 
@@ -314,12 +314,6 @@ static void anon_fault(void *p, unsigned long start, unsigned long end)
 	fill_memory(p, start, end);
 }
 
-static bool anon_check_huge(void *addr, size_t len, int nr_hpages,
-		unsigned long hpage_size)
-{
-	return check_huge_anon(addr, len, nr_hpages, hpage_size);
-}
-
 static void *file_setup_area_common(int nr_hpages, enum file_setup_ops setup)
 {
 	const int open_opt = setup == FILE_SETUP_READ_ONLY_FS ? O_RDONLY : O_RDWR;
@@ -411,20 +405,6 @@ static void file_fault_write(void *p, unsigned long start, unsigned long end)
 		ksft_exit_fail_perror("madvise(MADV_POPULATE_WRITE)");
 }
 
-static bool file_check_huge(void *addr, size_t len, int nr_hpages,
-		unsigned long hpage_size)
-{
-	switch (finfo.type) {
-	case VMA_FILE:
-		return check_huge_file(addr, len, nr_hpages, hpage_size);
-	case VMA_SHMEM:
-		return check_huge_shmem(addr, len, nr_hpages, hpage_size);
-	default:
-		ksft_exit_fail_msg("Unknown VMA type\n");
-		return false;
-	}
-}
-
 static void *shmem_setup_area(int nr_hpages)
 {
 	void *p;
@@ -448,17 +428,11 @@ static void shmem_cleanup_area(void *p, unsigned long size)
 	close(finfo.fd);
 }
 
-static bool shmem_check_huge(void *addr, size_t len, int nr_hpages,
-		unsigned long hpage_size)
-{
-	return check_huge_shmem(addr, len, nr_hpages, hpage_size);
-}
-
 static struct mem_ops __anon_ops = {
 	.setup_area = &anon_setup_area,
 	.cleanup_area = &anon_cleanup_area,
 	.fault = &anon_fault,
-	.check_huge = &anon_check_huge,
+	.check_huge = &check_huge_anon,
 	.name = "anon",
 };
 
@@ -466,7 +440,7 @@ static struct mem_ops __read_only_file_ops = {
 	.setup_area = &file_setup_read_only_area,
 	.cleanup_area = &file_cleanup_area,
 	.fault = &file_fault_read,
-	.check_huge = &file_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "file",
 };
 
@@ -474,7 +448,7 @@ static struct mem_ops __read_write_file_read_ops = {
 	.setup_area = &file_setup_read_write_fs_read_area,
 	.cleanup_area = &file_cleanup_area,
 	.fault = &file_fault_read_and_flush,
-	.check_huge = &file_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "file",
 };
 
@@ -482,7 +456,7 @@ static struct mem_ops __read_write_file_write_ops = {
 	.setup_area = &file_setup_read_write_fs_write_area,
 	.cleanup_area = &file_cleanup_area,
 	.fault = &file_fault_write,
-	.check_huge = &file_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "file",
 };
 
@@ -490,7 +464,7 @@ static struct mem_ops __shmem_ops = {
 	.setup_area = &shmem_setup_area,
 	.cleanup_area = &shmem_cleanup_area,
 	.fault = &anon_fault,
-	.check_huge = &shmem_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "shmem",
 };
 
tools/testing/selftests/mm/uffd-common.c +2 -2
index 1fb967ef49854..a76851aced4d0 100644
@@ -196,8 +196,8 @@ static void shmem_check_pmd_mapping(uffd_global_test_opts_t *gopts, void *p, int
 {
 	size_t len = expect_nr_hpages * read_pmd_pagesize();
 
-	if (!check_huge_shmem(gopts->area_dst_alias, len, expect_nr_hpages,
-			      read_pmd_pagesize()))
+	if (!check_huge_file(gopts->area_dst_alias, len, expect_nr_hpages,
+			     read_pmd_pagesize()))
 		err("Did not find expected %d number of hugepages",
 		    expect_nr_hpages);
 }
tools/testing/selftests/mm/vm_util.c +0 -5
index a7c5caf6925e0..e31e88d5af76d 100644
@@ -475,11 +475,6 @@ bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
 }
 
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
-{
-	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
-}
-
 int64_t allocate_transhuge(void *ptr, int pagemap_fd)
 {
 	uint64_t ent[2];
tools/testing/selftests/mm/vm_util.h +0 -1
index 9a49af88702e4..fc9c93235f6fa 100644
@@ -92,7 +92,6 @@ uint64_t read_pmd_pagesize(void);
 unsigned long rss_anon(void);
 bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
 bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
 int64_t allocate_transhuge(void *ptr, int pagemap_fd);
 int pageflags_get(unsigned long pfn, int kpageflags_fd, uint64_t *flags);
 int gather_folio_orders(char *vaddr_start, size_t len,
Signature · 1 line
2.43.0
maintainer Acked-by on v7 3/3
#8 lore raw

Re: [PATCH v7 3/3] kselftest: mm: remove check_huge_shmem()

Quoted 9 lines · Yeoreum Yun

check_huge_shmem() was required to distinguish shmem huge pages because /proc/self/smaps reports them using a dedicated “ShmemPmdMapped” entry, as opposed to “FilePmdMapped” for file-backed huge pages.

Now that /proc/self/smaps is no longer used to detect huge pages and /proc/kpageflags is used instead, it is sufficient to distinguish between file-backed and anonymous pages since the ShmemPmdMapped is also kind of file-backed.

Quoted 6 lines

Therefore, remove check_huge_shmem() and use check_huge_file() instead and cleanup khugepaged's check_huge operation in mem_ops.

---

Signature · 3 lines
Cheers,

David
reviewer Acked-by on v7 3/3
#9 lore raw

Re: [PATCH v7 3/3] kselftest: mm: remove check_huge_shmem()

In reply to Yeoreum Yun
Quoted 9 lines · Yeoreum Yun

check_huge_shmem() was required to distinguish shmem huge pages because /proc/self/smaps reports them using a dedicated “ShmemPmdMapped” entry, as opposed to “FilePmdMapped” for file-backed huge pages.

Now that /proc/self/smaps is no longer used to detect huge pages and /proc/kpageflags is used instead, it is sufficient to distinguish between file-backed and anonymous pages since the ShmemPmdMapped is also kind of file-backed.

Quoted 5 lines

Therefore, remove check_huge_shmem() and use check_huge_file() instead and cleanup khugepaged's check_huge operation in mem_ops.

LGTM so:

Quoted 168 lines
---
 tools/testing/selftests/mm/folio_split_race_test.c |  2 +-
 tools/testing/selftests/mm/khugepaged.c            | 38 ++++------------------
 tools/testing/selftests/mm/uffd-common.c           |  4 +--
 tools/testing/selftests/mm/vm_util.c               |  5 ---
 tools/testing/selftests/mm/vm_util.h               |  1 -
 5 files changed, 9 insertions(+), 41 deletions(-)
tools/testing/selftests/mm/folio_split_race_test.c +1 -1
index 1960635a953eb..4c0d3b6a411b8 100644
@@ -182,7 +182,7 @@ static uint64_t run_iteration(void)
 	for (i = 0; i < TOTAL_PAGES; i++)
 		fill_page(mmap_base, i);
 
-	if (!check_huge_shmem(mmap_base, FILE_SIZE, NR_PMD_PAGE, pmd_pagesize))
+	if (!check_huge_file(mmap_base, FILE_SIZE, NR_PMD_PAGE, pmd_pagesize))
 		ksft_exit_fail_msg("No shmem THP is allocated\n");
 
 	if (pthread_barrier_init(&ctl.barrier, NULL, NUM_READER_THREADS + 1) != 0)
tools/testing/selftests/mm/khugepaged.c +6 -32
index f82673f5f6b47..f448278823678 100644
@@ -53,7 +53,7 @@ struct mem_ops {
 	void *(*setup_area)(int nr_hpages);
 	void (*cleanup_area)(void *p, unsigned long size);
 	void (*fault)(void *p, unsigned long start, unsigned long end);
-	bool (*check_huge)(void *addr, size_t len, int nr_hpages, unsigned long hpage_size);
+	bool (*check_huge)(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
 	const char *name;
 };
 
@@ -314,12 +314,6 @@ static void anon_fault(void *p, unsigned long start, unsigned long end)
 	fill_memory(p, start, end);
 }
 
-static bool anon_check_huge(void *addr, size_t len, int nr_hpages,
-		unsigned long hpage_size)
-{
-	return check_huge_anon(addr, len, nr_hpages, hpage_size);
-}
-
 static void *file_setup_area_common(int nr_hpages, enum file_setup_ops setup)
 {
 	const int open_opt = setup == FILE_SETUP_READ_ONLY_FS ? O_RDONLY : O_RDWR;
@@ -411,20 +405,6 @@ static void file_fault_write(void *p, unsigned long start, unsigned long end)
 		ksft_exit_fail_perror("madvise(MADV_POPULATE_WRITE)");
 }
 
-static bool file_check_huge(void *addr, size_t len, int nr_hpages,
-		unsigned long hpage_size)
-{
-	switch (finfo.type) {
-	case VMA_FILE:
-		return check_huge_file(addr, len, nr_hpages, hpage_size);
-	case VMA_SHMEM:
-		return check_huge_shmem(addr, len, nr_hpages, hpage_size);
-	default:
-		ksft_exit_fail_msg("Unknown VMA type\n");
-		return false;
-	}
-}
-
 static void *shmem_setup_area(int nr_hpages)
 {
 	void *p;
@@ -448,17 +428,11 @@ static void shmem_cleanup_area(void *p, unsigned long size)
 	close(finfo.fd);
 }
 
-static bool shmem_check_huge(void *addr, size_t len, int nr_hpages,
-		unsigned long hpage_size)
-{
-	return check_huge_shmem(addr, len, nr_hpages, hpage_size);
-}
-
 static struct mem_ops __anon_ops = {
 	.setup_area = &anon_setup_area,
 	.cleanup_area = &anon_cleanup_area,
 	.fault = &anon_fault,
-	.check_huge = &anon_check_huge,
+	.check_huge = &check_huge_anon,
 	.name = "anon",
 };
 
@@ -466,7 +440,7 @@ static struct mem_ops __read_only_file_ops = {
 	.setup_area = &file_setup_read_only_area,
 	.cleanup_area = &file_cleanup_area,
 	.fault = &file_fault_read,
-	.check_huge = &file_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "file",
 };
 
@@ -474,7 +448,7 @@ static struct mem_ops __read_write_file_read_ops = {
 	.setup_area = &file_setup_read_write_fs_read_area,
 	.cleanup_area = &file_cleanup_area,
 	.fault = &file_fault_read_and_flush,
-	.check_huge = &file_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "file",
 };
 
@@ -482,7 +456,7 @@ static struct mem_ops __read_write_file_write_ops = {
 	.setup_area = &file_setup_read_write_fs_write_area,
 	.cleanup_area = &file_cleanup_area,
 	.fault = &file_fault_write,
-	.check_huge = &file_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "file",
 };
 
@@ -490,7 +464,7 @@ static struct mem_ops __shmem_ops = {
 	.setup_area = &shmem_setup_area,
 	.cleanup_area = &shmem_cleanup_area,
 	.fault = &anon_fault,
-	.check_huge = &shmem_check_huge,
+	.check_huge = &check_huge_file,
 	.name = "shmem",
 };
 
tools/testing/selftests/mm/uffd-common.c +2 -2
index 1fb967ef49854..a76851aced4d0 100644
@@ -196,8 +196,8 @@ static void shmem_check_pmd_mapping(uffd_global_test_opts_t *gopts, void *p, int
 {
 	size_t len = expect_nr_hpages * read_pmd_pagesize();
 
-	if (!check_huge_shmem(gopts->area_dst_alias, len, expect_nr_hpages,
-			      read_pmd_pagesize()))
+	if (!check_huge_file(gopts->area_dst_alias, len, expect_nr_hpages,
+			     read_pmd_pagesize()))
 		err("Did not find expected %d number of hugepages",
 		    expect_nr_hpages);
 }
tools/testing/selftests/mm/vm_util.c +0 -5
index a7c5caf6925e0..e31e88d5af76d 100644
@@ -475,11 +475,6 @@ bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
 }
 
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
-{
-	return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
-}
-
 int64_t allocate_transhuge(void *ptr, int pagemap_fd)
 {
 	uint64_t ent[2];
tools/testing/selftests/mm/vm_util.h +0 -2
index 9a49af88702e4..fc9c93235f6fa 100644
@@ -92,7 +92,6 @@ uint64_t read_pmd_pagesize(void);
 unsigned long rss_anon(void);
 bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
 bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
-bool check_huge_shmem(void *addr, size_t len, int nr_hpages, uint64_t hpage_size);
 int64_t allocate_transhuge(void *ptr, int pagemap_fd);
 int pageflags_get(unsigned long pfn, int kpageflags_fd, uint64_t *flags);
 int gather_folio_orders(char *vaddr_start, size_t len,
 
--
Quoted 2 lines

2.43.0

--
Cheers, Lorenzo

reviewer Acked-by on v7 3/3
#10 lore raw

Re: [PATCH v7 3/3] kselftest: mm: remove check_huge_shmem()

In reply to Yeoreum Yun
Quoted 14 lines · Yeoreum Yun

check_huge_shmem() was required to distinguish shmem huge pages because /proc/self/smaps reports them using a dedicated “ShmemPmdMapped” entry, as opposed to “FilePmdMapped” for file-backed huge pages.

Now that /proc/self/smaps is no longer used to detect huge pages and /proc/kpageflags is used instead, it is sufficient to distinguish between file-backed and anonymous pages since the ShmemPmdMapped is also kind of file-backed.

Therefore, remove check_huge_shmem() and use check_huge_file() instead and cleanup khugepaged's check_huge operation in mem_ops.

Quoted 8 lines
---
 tools/testing/selftests/mm/folio_split_race_test.c |  2 +-
 tools/testing/selftests/mm/khugepaged.c            | 38 ++++------------------
 tools/testing/selftests/mm/uffd-common.c           |  4 +--
 tools/testing/selftests/mm/vm_util.c               |  5 ---
 tools/testing/selftests/mm/vm_util.h               |  1 -
 5 files changed, 9 insertions(+), 41 deletions(-)

Makes sense.

Signature · 2 lines
Best Regards,
Yan, Zi
reviewer Reviewed-by on v7 3/3
#11 lore raw

Re: [PATCH v7 3/3] kselftest: mm: remove check_huge_shmem()

In reply to Yeoreum Yun
Quoted 9 lines · Yeoreum Yun

check_huge_shmem() was required to distinguish shmem huge pages because /proc/self/smaps reports them using a dedicated “ShmemPmdMapped” entry, as opposed to “FilePmdMapped” for file-backed huge pages.

Now that /proc/self/smaps is no longer used to detect huge pages and /proc/kpageflags is used instead, it is sufficient to distinguish between file-backed and anonymous pages since the ShmemPmdMapped is also kind of file-backed.

Quoted 6 lines

Therefore, remove check_huge_shmem() and use check_huge_file() instead and cleanup khugepaged's check_huge operation in mem_ops.

---

LGTM.