kselftest: mm: fix some failure of split_huge_page_test

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.

Reported-by Kevin Brodsky <[email protected]>

Reviewed-by Zi Yan <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
 tools/testing/selftests/mm/split_huge_page_test.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

tools/testing/selftests/mm/split_huge_page_test.c +0 -0

index 86a603692826..8f68bc94a5ca 100644

Quoted 18 lines

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.

Reported-by Kevin Brodsky <[email protected]>

Reviewed-by Zi Yan <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
 tools/testing/selftests/mm/split_huge_page_test.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

tools/testing/selftests/mm/split_huge_page_test.c +0 -0

index 86a603692826..8f68bc94a5ca 100644

tools/testing/selftests/mm/split_huge_page_test.c static void verify_rss_anon_split_huge_page_all_zeroes(char *one_page, int nr_hp · line 186

Quoted 7 lines

tools/testing/selftests/mm/split_huge_page_test.c +4 -0

@@ -180,6 +180,10 @@ 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");
 
+	/* Prevent khugepaged from collapsing the pages. */
+	if (madvise(one_page, len, MADV_NOHUGEPAGE))
+		ksft_print_msg("MADV_NOHUGEPAGE failed to prevent khugepaged from collapsing pages.\n");
+

Ok, this is really only expected to fail on extremely old kernels or kernels without CONFIG_TRANSPARENT_HUGEPAGE (where we should never get to that point :) ).

Can we just turn that into a

	ksft_exit_fail_perror("madvise(MADV_NOHUGEPAGE)");

I'm okay with a ksft_print_msg() as well, but would shorten the message quite a lot ("MADV_NOHUGEPAGE failed" or sth).

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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
  tools/testing/selftests/mm/vm_util.h |   1 +
  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +3 -32

index 4821a3563036..1d0959b3b9e8 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,97 @@ static bool check_large_folios(void *addr, size_t len, int nr_hpages,
  		ret = true;
 
  out:
-	close(pagemap_fd);
-	close(kpageflags_fd);
  	return ret;
  }

tools/testing/selftests/mm/vm_util.c +8 -1

  {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	char *start = addr;
+	char *end = start + len;
 
+	pmd_pagesize = read_pmd_pagesize();
  	if (!pmd_pagesize)
  		ksft_exit_fail_msg("reading PMD pagesize failed\n");

tools/testing/selftests/mm/vm_util.c +50 -18

-	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");
 
-bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
-{
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	kpageflags_fd = open(KPAGEFLAGS_PATH, O_RDONLY);
+	if (kpageflags_fd < 0) {
+		close(pagemap_fd);
+		ksft_exit_fail_msg("open kpageflags fail\n");
+	}
 
-	if (!pmd_pagesize)
-		ksft_exit_fail_msg("reading PMD pagesize failed\n");
+	if (check_large && !check_large_folios(pagemap_fd, kpageflags_fd,
+					       addr, len, nr_hpages, hpage_size))
+		goto out;
 
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "FilePmdMapped:", nr_hpages, hpage_size);
+	for (; start < end; start += granule) {
+		categories = pagemap_scan_get_categories(pagemap_fd, start);
+		pfn = pagemap_get_pfn(pagemap_fd, start);
+		if (pfn == -1UL) {
+			if (check_large)
+				goto out;
+			else
+				continue;
+		}
+		if (pageflags_get(pfn, kpageflags_fd, &kpf))
+			ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));
+		huge_mapped = categories & PAGE_IS_HUGE;
+		if (check_large != huge_mapped) {
+			if (!check_large || granule == pmd_pagesize)
+				goto out;
+		}
+		if (kpf & KPF_COMPOUND_TAIL)
+			continue;
+		if (!!(categories & PAGE_IS_FILE) != (type != CHECK_HUGE_ANON))
+			goto out;
+		if (type == CHECK_HUGE_ANON)
+			continue;
+		if (!!(kpf & KPF_SWAPBACKED) != (type != CHECK_HUGE_FILE))
+			goto out;
+	}
 
-	return check_large_folios(addr, len, nr_hpages, hpage_size);
+out:
+	close(pagemap_fd);
+	close(kpageflags_fd);
+	return start >= end;
  }
 
-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_pmd_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_pmd_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_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_SHMEM);
  }
 
  int64_t allocate_transhuge(void *ptr, int pagemap_fd)

tools/testing/selftests/mm/vm_util.h +1 -0

index 9a49af88702e..f19bd17817e5 100644
@@ -18,6 +18,7 @@
  #define PM_SWAP                       BIT_ULL(62)
  #define PM_PRESENT                    BIT_ULL(63)
 
+#define KPF_SWAPBACKED                BIT_ULL(14)
  #define KPF_COMPOUND_HEAD             BIT_ULL(15)
  #define KPF_COMPOUND_TAIL             BIT_ULL(16)
  #define KPF_HWPOISON                  BIT_ULL(19)

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
  tools/testing/selftests/mm/vm_util.h |   1 +
  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +8 -33

index 4821a3563036..1d0959b3b9e8 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,97 @@ 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)
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+	CHECK_HUGE_SHMEM,
+};

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 391

Quoted 3 lines

tools/testing/selftests/mm/vm_util.c +3 -0

+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+			     uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

Quoted 11 lines

tools/testing/selftests/mm/vm_util.c +8 -1

  {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	char *start = addr;
+	char *end = start + len;
+	pmd_pagesize = read_pmd_pagesize();
  	if (!pmd_pagesize)

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 411

Quoted 10 lines

tools/testing/selftests/mm/vm_util.c +7 -2

  		ksft_exit_fail_msg("reading PMD pagesize failed\n");
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	if (nr_hpages > 0) {
+		check_large = true;
+		granule = hpage_size;
+	} else {
+		check_large = false;
+		granule = psize();
+	}

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

[root@]./khugepaged -c 4 mthp_khugepaged:anon TAP version 13
# Save THP and khugepaged settings... OK 1..4
# Allocate huge page on fault... OK
# Split huge PMD on MADV_DONTNEED... OK
ok 1 allocate on fault and split
#
# Run test: collapse_full (mthp_khugepaged:anon) # Collapse multiple fully populated PTE table.... OK ok 2 collapse_full
#

# Run test: collapse_empty (mthp_khugepaged:anon) # Do not collapse empty PTE table.... OK
ok 3 collapse_empty
#
# Run test: collapse_single_mthp (mthp_khugepaged:anon) # Collapse PTE table with half PTE entries present.... Fail not ok 4 collapse_single_mthp
# Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
  tools/testing/selftests/mm/vm_util.h |   1 +
  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +8 -33

index 4821a3563036..1d0959b3b9e8 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,97 @@ 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)
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+	CHECK_HUGE_SHMEM,
+};

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 389

Quoted 3 lines

tools/testing/selftests/mm/vm_util.c +3 -0

+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+			     uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

Quoted 17 lines

tools/testing/selftests/mm/vm_util.c +10 -3

  {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	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, "AnonHugePages: ", nr_hpages, hpage_size);
+	if (nr_hpages > 0) {
+		check_large = true;

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 407

Quoted 5 lines

tools/testing/selftests/mm/vm_util.c +5 -0

+		granule = hpage_size;
+	} else {
+		check_large = false;
+		granule = psize();
+	}

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

Hi Baolin,

Quoted 90 lines · Yeoreum Yun

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
  tools/testing/selftests/mm/vm_util.h |   1 +
  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +8 -33

index 4821a3563036..1d0959b3b9e8 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,97 @@ 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)
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+	CHECK_HUGE_SHMEM,
+};

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 389

Quoted 3 lines

tools/testing/selftests/mm/vm_util.c +3 -0

+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+			     uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

But, It seems to valuable to check other page-flags than checking the large-folio only.

Quoted 17 lines

Quoted 17 lines

tools/testing/selftests/mm/vm_util.c +10 -3

  {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	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, "AnonHugePages: ", nr_hpages, hpage_size);
+	if (nr_hpages > 0) {
+		check_large = true;

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 407

Quoted 5 lines

tools/testing/selftests/mm/vm_util.c +5 -0

+		granule = hpage_size;
+	} else {
+		check_large = false;
+		granule = psize();
+	}

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

Since for a split case, large folio can be as-is but only remove the PMD mapping only, skipping the large_folio checking seems valid when nr_hpage is 0.

And the failure of test seems because of unmapped area after changinng the collapse-order. Therefore, it seems to fine with below patch:

---------------&<----------------------

tools/testing/selftests/mm/vm_util.c +3 -3

index 1d0959b3b9e8..f174a76d2310 100644
@@ -387,14 +387,14 @@ enum check_huge_type {
        CHECK_HUGE_SHMEM,
 };
 
-static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
-                            uint64_t hpage_size, enum check_huge_type type)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+                        uint64_t hpage_size, enum check_huge_type type)
 {
        int pagemap_fd, kpageflags_fd;
        uint64_t pmd_pagesize, granule;
        uint64_t categories, kpf;
        unsigned long pfn;
-       bool check_large, huge_mapped;
+       bool check_large, check_huge_mapped, allow_nomap;
        char *start = addr;
        char *end = start + len;
 

@@ -405,11 +405,20 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
        if (nr_hpages > 0) {

tools/testing/selftests/mm/vm_util.c +10 -8

+       if (nr_hpages * hpage_size < len)
+               allow_nomap = true;
+       else
+               allow_nomap = false;
+
        pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
        if (pagemap_fd < 0)
                ksft_exit_fail_msg("open pagemap fail\n");

@@ -428,18 +437,15 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
                categories = pagemap_scan_get_categories(pagemap_fd, start);
                pfn = pagemap_get_pfn(pagemap_fd, start);
                if (pfn == -1UL) {
-                       if (check_large)
+                       if (!allow_nomap)
                                goto out;
                        else
                                continue;
                }
                if (pageflags_get(pfn, kpageflags_fd, &kpf))
                        ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));
-               huge_mapped = categories & PAGE_IS_HUGE;
-               if (check_large != huge_mapped) {
-                       if (!check_large || granule == pmd_pagesize)
+               if (check_huge_mapped != !!(categories & PAGE_IS_HUGE))
                                goto out;
-               }
                if (kpf & KPF_COMPOUND_TAIL)
                        continue;
                if (!!(categories & PAGE_IS_FILE) != (type != CHECK_HUGE_ANON))

@@ -458,17 +464,17 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
 
 bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
-       return __check_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_ANON);
+       return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_ANON);
 }
 
 bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
-       return __check_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
+       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_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_SHMEM);
+       return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_SHMEM);
 }
 
 

If it's good for you, I'll repost again.

Thanks!

Quoted 130 lines · Yeoreum Yun

Hi Baolin,

Quoted 90 lines · Yeoreum Yun

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
  tools/testing/selftests/mm/vm_util.h |   1 +
  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +8 -33

index 4821a3563036..1d0959b3b9e8 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,97 @@ 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)
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+	CHECK_HUGE_SHMEM,
+};

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 389

Quoted 3 lines

tools/testing/selftests/mm/vm_util.c +3 -0

+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+			     uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

But, It seems to valuable to check other page-flags than checking the large-folio only.

Quoted 25 lines

Quoted 12 lines

tools/testing/selftests/mm/vm_util.c +8 -1

  {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	char *start = addr;
+	char *end = start + len;
+	pmd_pagesize = read_pmd_pagesize();
  	if (!pmd_pagesize)

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 407

Quoted 10 lines

tools/testing/selftests/mm/vm_util.c +7 -2

  		ksft_exit_fail_msg("reading PMD pagesize failed\n");
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	if (nr_hpages > 0) {
+		check_large = true;
+		granule = hpage_size;
+	} else {
+		check_large = false;
+		granule = psize();
+	}

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

Quoted 3 lines

Since for a split case, large folio can be as-is but only remove the PMD mapping only, skipping the large_folio checking seems valid when nr_hpage is 0.

What split care are you referring to? split_huge_page_test() always splits the folio.

Quoted 30 lines

And the failure of test seems because of unmapped area after changinng the collapse-order. Therefore, it seems to fine with below patch:

---------------&<----------------------

tools/testing/selftests/mm/vm_util.c +3 -3

index 1d0959b3b9e8..f174a76d2310 100644
@@ -387,14 +387,14 @@ enum check_huge_type {
        CHECK_HUGE_SHMEM,
 };
 
-static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
-                            uint64_t hpage_size, enum check_huge_type type)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+                        uint64_t hpage_size, enum check_huge_type type)
 {
        int pagemap_fd, kpageflags_fd;
        uint64_t pmd_pagesize, granule;
        uint64_t categories, kpf;
        unsigned long pfn;
-       bool check_large, huge_mapped;
+       bool check_large, check_huge_mapped, allow_nomap;
        char *start = addr;
        char *end = start + len;
 

tools/testing/selftests/mm/vm_util.c static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages, · line 415

Quoted 12 lines

tools/testing/selftests/mm/vm_util.c +4 -0

@@ -405,11 +405,20 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
        if (nr_hpages > 0) {
                check_large = true;
                granule = hpage_size;
+               if (granule == pmd_pagesize)
+                       check_huge_mapped = true;
+               else
+                       check_huge_mapped = false;
        } else {
                check_large = false;
                granule = psize();
        }

The else is for nr_hpages == 0? But it looks like we allow negative nr_hpages. Maybe add a bool expect_huge = nr_hpages > 0 to make it explicit.

granule is an optimization for PAGE_IS_HUGE scanning? When we expect a PMD mapping, we just scan at pmd_pagesize granularity, otherwise we check every single page?

At the high level, the function looks good to me, the rules are:

1. if hpage_size == pmd_pagesize, we need to check PAGE_IS_HUGE and check_large_folio() can be skipped, since we only care about mappings. This checks for PMD mappings.

2. in other cases, check_large_folios() is always needed. This is for mTHP checks.

I think the ifs at the beginning is confusing. Can we do something like below to get rid of the ifs? I also moved KPF_* checks in a separate function. Feel free to make changes if you find any issue there.

static bool check_huge_type(uint64_t categories, uint64_t kpageflags,
enum check_huge_type type)

{
	bool file = categories & PAGE_IS_FILE;
	bool swapbacked = kpageflags & KPF_SWAPBACKED;

	switch (type) {
	case CHECK_HUGE_ANON:
		return !file;
	case CHECK_HUGE_FILE:
		return file && !swapbacked;
	case CHECK_HUGE_SHMEM:
		return file && swapbacked;
	}

	return false;
}

static bool __check_huge(void *addr, size_t len, int nr_hpages,
uint64_t hpage_size, enum check_huge_type type)

{
	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;
	bool allow_nonpresent;
	bool ret = false;
	char *start = addr;
	char *end = start + len;

	pmd_pagesize = read_pmd_pagesize();
	if (!pmd_pagesize)
		ksft_exit_fail_msg("reading PMD pagesize failed\n");

	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;

	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) {
		close(pagemap_fd);
		ksft_exit_fail_msg("open kpageflags fail\n");
	}

	/* PTE-mapped large folios cannot be identified by PAGE_IS_HUGE. */
	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;
			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;
}

Quoted 59 lines

tools/testing/selftests/mm/vm_util.c +10 -8

+       if (nr_hpages * hpage_size < len)
+               allow_nomap = true;
+       else
+               allow_nomap = false;
+
        pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
        if (pagemap_fd < 0)
                ksft_exit_fail_msg("open pagemap fail\n");

@@ -428,18 +437,15 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
                categories = pagemap_scan_get_categories(pagemap_fd, start);
                pfn = pagemap_get_pfn(pagemap_fd, start);
                if (pfn == -1UL) {
-                       if (check_large)
+                       if (!allow_nomap)
                                goto out;
                        else
                                continue;
                }
                if (pageflags_get(pfn, kpageflags_fd, &kpf))
                        ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));
-               huge_mapped = categories & PAGE_IS_HUGE;
-               if (check_large != huge_mapped) {
-                       if (!check_large || granule == pmd_pagesize)
+               if (check_huge_mapped != !!(categories & PAGE_IS_HUGE))
                                goto out;
-               }
                if (kpf & KPF_COMPOUND_TAIL)
                        continue;
                if (!!(categories & PAGE_IS_FILE) != (type != CHECK_HUGE_ANON))

@@ -458,17 +464,17 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
 
 bool check_huge_anon(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
-       return __check_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_ANON);
+       return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_ANON);
 }
 
 bool check_huge_file(void *addr, size_t len, int nr_hpages, uint64_t hpage_size)
 {
-       return __check_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_FILE);
+       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_pmd_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_SHMEM);
+       return __check_huge(addr, len, nr_hpages, hpage_size, CHECK_HUGE_SHMEM);
 }
 
 

If it's good for you, I'll repost again.

Thanks!

Signature · 2 lines

Best Regards,
Yan, Zi

Quoted 131 lines · Zi Yan

Quoted 130 lines · Yeoreum Yun

Hi Baolin,

Quoted 90 lines · Yeoreum Yun

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
   tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
   tools/testing/selftests/mm/vm_util.h |   1 +
   2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +8 -33

index 4821a3563036..1d0959b3b9e8 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,97 @@ 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)
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+	CHECK_HUGE_SHMEM,
+};

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 389

Quoted 3 lines

tools/testing/selftests/mm/vm_util.c +3 -0

+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+			     uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

But, It seems to valuable to check other page-flags than checking the large-folio only.

Quoted 25 lines

Quoted 12 lines

tools/testing/selftests/mm/vm_util.c +8 -1

   {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	char *start = addr;
+	char *end = start + len;
+	pmd_pagesize = read_pmd_pagesize();
   	if (!pmd_pagesize)

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 407

Quoted 10 lines

tools/testing/selftests/mm/vm_util.c +7 -2

   		ksft_exit_fail_msg("reading PMD pagesize failed\n");
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	if (nr_hpages > 0) {
+		check_large = true;
+		granule = hpage_size;
+	} else {
+		check_large = false;
+		granule = psize();
+	}

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

Quoted 6 lines

Quoted 3 lines

Since for a split case, large folio can be as-is but only remove the PMD mapping only, skipping the large_folio checking seems valid when nr_hpage is 0.

What split care are you referring to? split_huge_page_test() always splits the folio.

Not for split_huge_page_test case but for khugepage testcase like collapse_full_of_compound() test.

Quoted 30 lines

Quoted 30 lines

And the failure of test seems because of unmapped area after changinng the collapse-order. Therefore, it seems to fine with below patch:

---------------&<----------------------

tools/testing/selftests/mm/vm_util.c +3 -3

index 1d0959b3b9e8..f174a76d2310 100644
@@ -387,14 +387,14 @@ enum check_huge_type {
         CHECK_HUGE_SHMEM,
  };
 
-static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
-                            uint64_t hpage_size, enum check_huge_type type)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+                        uint64_t hpage_size, enum check_huge_type type)
  {
         int pagemap_fd, kpageflags_fd;
         uint64_t pmd_pagesize, granule;
         uint64_t categories, kpf;
         unsigned long pfn;
-       bool check_large, huge_mapped;
+       bool check_large, check_huge_mapped, allow_nomap;
         char *start = addr;
         char *end = start + len;
 

Quoted 16 lines

tools/testing/selftests/mm/vm_util.c static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages, · line 415

Quoted 12 lines

tools/testing/selftests/mm/vm_util.c +4 -0

@@ -405,11 +405,20 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
         if (nr_hpages > 0) {
                 check_large = true;
                 granule = hpage_size;
+               if (granule == pmd_pagesize)
+                       check_huge_mapped = true;
+               else
+                       check_huge_mapped = false;
         } else {
                 check_large = false;
                 granule = psize();
         }

The else is for nr_hpages == 0? But it looks like we allow negative nr_hpages. Maybe add a bool expect_huge = nr_hpages > 0 to make it explicit.

Fair enough. I'll change.

Quoted 13 lines

granule is an optimization for PAGE_IS_HUGE scanning? When we expect a PMD mapping, we just scan at pmd_pagesize granularity, otherwise we check every single page?

At the high level, the function looks good to me, the rules are:

1. if hpage_size == pmd_pagesize, we need to check PAGE_IS_HUGE and check_large_folio() can be skipped, since we only care about mappings. This checks for PMD mappings.

2. in other cases, check_large_folios() is always needed. This is for mTHP checks.

Exactly, but for some testcase where use this function, doesn't trigger split of large _folio but only check the HUGE MAP is removed (nr_hpage == 0), it skips the chekc_large_folio().

Quoted 73 lines

I think the ifs at the beginning is confusing. Can we do something like below to get rid of the ifs? I also moved KPF_* checks in a separate function. Feel free to make changes if you find any issue there.

static bool check_huge_type(uint64_t categories, uint64_t kpageflags,
enum check_huge_type type)

{
	bool file = categories & PAGE_IS_FILE;
	bool swapbacked = kpageflags & KPF_SWAPBACKED;

	switch (type) {
	case CHECK_HUGE_ANON:
		return !file;
	case CHECK_HUGE_FILE:
		return file && !swapbacked;
	case CHECK_HUGE_SHMEM:
		return file && swapbacked;
	}

	return false;
}

static bool __check_huge(void *addr, size_t len, int nr_hpages,
uint64_t hpage_size, enum check_huge_type type)

{
	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;
	bool allow_nonpresent;
	bool ret = false;
	char *start = addr;
	char *end = start + len;

	pmd_pagesize = read_pmd_pagesize();
	if (!pmd_pagesize)
		ksft_exit_fail_msg("reading PMD pagesize failed\n");

	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;

	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) {
		close(pagemap_fd);
		ksft_exit_fail_msg("open kpageflags fail\n");
	}

	/* PTE-mapped large folios cannot be identified by PAGE_IS_HUGE. */
	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;
			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))

Quoted 8 lines

			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;

Again, because of collapse_full_of_compound() testcase, it would be failed for this. so, it would better to skip check_large_folioes() when nr_hpages is 0.

Quoted 189 lines · Zi Yan

Quoted 131 lines · Yeoreum Yun

Quoted 130 lines

Hi Baolin,

Quoted 90 lines · Yeoreum Yun

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
   tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++---------------
   tools/testing/selftests/mm/vm_util.h |   1 +
   2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/mm/vm_util.c +8 -33

index 4821a3563036..1d0959b3b9e8 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,97 @@ 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)
+enum check_huge_type {
+	CHECK_HUGE_ANON,
+	CHECK_HUGE_FILE,
+	CHECK_HUGE_SHMEM,
+};

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 389

Quoted 3 lines

tools/testing/selftests/mm/vm_util.c +3 -0

+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+			     uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

But, It seems to valuable to check other page-flags than checking the large-folio only.

Quoted 25 lines

Quoted 12 lines

tools/testing/selftests/mm/vm_util.c +8 -1

   {
-	uint64_t pmd_pagesize = read_pmd_pagesize();
+	int pagemap_fd, kpageflags_fd;
+	uint64_t pmd_pagesize, granule;
+	uint64_t categories, kpf;
+	unsigned long pfn;
+	bool check_large, huge_mapped;
+	char *start = addr;
+	char *end = start + len;
+	pmd_pagesize = read_pmd_pagesize();
   	if (!pmd_pagesize)

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 407

Quoted 10 lines

tools/testing/selftests/mm/vm_util.c +7 -2

   		ksft_exit_fail_msg("reading PMD pagesize failed\n");
-	if (hpage_size == pmd_pagesize)
-		return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size);
+	if (nr_hpages > 0) {
+		check_large = true;
+		granule = hpage_size;
+	} else {
+		check_large = false;
+		granule = psize();
+	}

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

Quoted 6 lines

Quoted 3 lines

Since for a split case, large folio can be as-is but only remove the PMD mapping only, skipping the large_folio checking seems valid when nr_hpage is 0.

What split care are you referring to? split_huge_page_test() always splits the folio.

Not for split_huge_page_test case but for khugepage testcase like collapse_full_of_compound() test.

Quoted 30 lines

Quoted 30 lines

And the failure of test seems because of unmapped area after changinng the collapse-order. Therefore, it seems to fine with below patch:

---------------&<----------------------

tools/testing/selftests/mm/vm_util.c +3 -3

index 1d0959b3b9e8..f174a76d2310 100644
@@ -387,14 +387,14 @@ enum check_huge_type {
         CHECK_HUGE_SHMEM,
  };
 
-static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
-                            uint64_t hpage_size, enum check_huge_type type)
+static bool __check_huge(void *addr, size_t len, int nr_hpages,
+                        uint64_t hpage_size, enum check_huge_type type)
  {
         int pagemap_fd, kpageflags_fd;
         uint64_t pmd_pagesize, granule;
         uint64_t categories, kpf;
         unsigned long pfn;
-       bool check_large, huge_mapped;
+       bool check_large, check_huge_mapped, allow_nomap;
         char *start = addr;
         char *end = start + len;
 

Quoted 16 lines

tools/testing/selftests/mm/vm_util.c static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages, · line 415

Quoted 12 lines

tools/testing/selftests/mm/vm_util.c +4 -0

@@ -405,11 +405,20 @@ static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
         if (nr_hpages > 0) {
                 check_large = true;
                 granule = hpage_size;
+               if (granule == pmd_pagesize)
+                       check_huge_mapped = true;
+               else
+                       check_huge_mapped = false;
         } else {
                 check_large = false;
                 granule = psize();
         }

The else is for nr_hpages == 0? But it looks like we allow negative nr_hpages. Maybe add a bool expect_huge = nr_hpages > 0 to make it explicit.

Quoted 15 lines

Fair enough. I'll change.

Quoted 13 lines

granule is an optimization for PAGE_IS_HUGE scanning? When we expect a PMD mapping, we just scan at pmd_pagesize granularity, otherwise we check every single page?

At the high level, the function looks good to me, the rules are:

1. if hpage_size == pmd_pagesize, we need to check PAGE_IS_HUGE and check_large_folio() can be skipped, since we only care about mappings. This checks for PMD mappings.

2. in other cases, check_large_folios() is always needed. This is for mTHP checks.

Yes, this is also what I thought. And the following code looks good to me. Thanks.

Quoted 86 lines

Exactly, but for some testcase where use this function, doesn't trigger split of large _folio but only check the HUGE MAP is removed (nr_hpage == 0), it skips the chekc_large_folio().

Quoted 73 lines

I think the ifs at the beginning is confusing. Can we do something like below to get rid of the ifs? I also moved KPF_* checks in a separate function. Feel free to make changes if you find any issue there.

static bool check_huge_type(uint64_t categories, uint64_t kpageflags,
enum check_huge_type type)

{
	bool file = categories & PAGE_IS_FILE;
	bool swapbacked = kpageflags & KPF_SWAPBACKED;

	switch (type) {
	case CHECK_HUGE_ANON:
		return !file;
	case CHECK_HUGE_FILE:
		return file && !swapbacked;
	case CHECK_HUGE_SHMEM:
		return file && swapbacked;
	}

	return false;
}

static bool __check_huge(void *addr, size_t len, int nr_hpages,
uint64_t hpage_size, enum check_huge_type type)

{
	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;
	bool allow_nonpresent;
	bool ret = false;
	char *start = addr;
	char *end = start + len;

	pmd_pagesize = read_pmd_pagesize();
	if (!pmd_pagesize)
		ksft_exit_fail_msg("reading PMD pagesize failed\n");

	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;

	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) {
		close(pagemap_fd);
		ksft_exit_fail_msg("open kpageflags fail\n");
	}

	/* PTE-mapped large folios cannot be identified by PAGE_IS_HUGE. */
	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;
			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))

Quoted 8 lines

			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;

Quoted 3 lines

Again, because of collapse_full_of_compound() testcase, it would be failed for this. so, it would better to skip check_large_folioes() when nr_hpages is 0.

IIUC, when nr_hpages is 0, we should also call check_large_folios() to verify that there are no large folios within this address range, which is also what the previous code did.

Also, I tried Zi's code, and it passed all my khugepaged selftests. I'm not sure why you saw the collapse_full_of_compound() test case fail.

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++--------------- tools/testing/selftests/mm/vm_util.h |   1 +

  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/ renamed from tools/testing/selftests/mm/vm_util.c +0 -0

mm/vm_util.c
index 4821a3563036..1d0959b3b9e8 100644

tools/testing/selftests/mm/vm_util.c +3 -29

@@ -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,97 @@ 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)

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 391

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c +8 -0

+enum check_huge_type {
+    CHECK_HUGE_ANON,
+    CHECK_HUGE_FILE,
+    CHECK_HUGE_SHMEM,
+};
+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+                 uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

Quoted 13 lines

  {
-    uint64_t pmd_pagesize = read_pmd_pagesize();
+    int pagemap_fd, kpageflags_fd;
+    uint64_t pmd_pagesize, granule;
+    uint64_t categories, kpf;
+    unsigned long pfn;
+    bool check_large, huge_mapped;
+    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)

Quoted 8 lines

-        return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size); +    if (nr_hpages > 0) {
+        check_large = true;
+        granule = hpage_size;
+    } else {
+        check_large = false;
+        granule = psize();
+    }

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

[root@]./khugepaged -c 4 mthp_khugepaged:anon TAP version 13
# Save THP and khugepaged settings... OK 1..4
# Allocate huge page on fault... OK
# Split huge PMD on MADV_DONTNEED... OK
ok 1 allocate on fault and split
#
# Run test: collapse_full (mthp_khugepaged:anon) # Collapse multiple fully populated PTE table.... OK ok 2 collapse_full
#

# Run test: collapse_empty (mthp_khugepaged:anon) # Do not collapse empty PTE table.... OK
ok 3 collapse_empty
#
# Run test: collapse_single_mthp (mthp_khugepaged:anon) # Collapse PTE table with half PTE entries present.... Fail not ok 4 collapse_single_mthp
# Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

Quoted 141 lines · Baolin Wang

Quoted 87 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.

Fix this by using /proc/self/pagemap and /proc/kpageflags instead of /proc/self/smaps to detect huge pages.

Reported-by David Hildenbrand (Arm) <[email protected]>

Signed-off-by Yeoreum Yun <[email protected]>

---
  tools/testing/selftests/mm/vm_util.c | 130 ++++++++++++++++++++--------------- tools/testing/selftests/mm/vm_util.h |   1 +

  2 files changed, 77 insertions(+), 54 deletions(-)

tools/testing/selftests/ renamed from tools/testing/selftests/mm/vm_util.c +0 -0

mm/vm_util.c
index 4821a3563036..1d0959b3b9e8 100644

tools/testing/selftests/mm/vm_util.c +3 -29

@@ -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,97 @@ 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)

tools/testing/selftests/mm/vm_util.c static bool check_large_folios(void *addr, size_t len, int nr_hpages, · line 391

Quoted 8 lines

tools/testing/selftests/mm/vm_util.c +8 -0

+enum check_huge_type {
+    CHECK_HUGE_ANON,
+    CHECK_HUGE_FILE,
+    CHECK_HUGE_SHMEM,
+};
+
+static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
+                 uint64_t hpage_size, enum check_huge_type type)

The original __check_pmd_huge() is only for PMD-sized large folios, but now it not only checks PMD-sized large folios but also mTHP large folios, which I find confusing. Please keep its original semantics, and only check PMD-sized large folios.

Quoted 13 lines

  {
-    uint64_t pmd_pagesize = read_pmd_pagesize();
+    int pagemap_fd, kpageflags_fd;
+    uint64_t pmd_pagesize, granule;
+    uint64_t categories, kpf;
+    unsigned long pfn;
+    bool check_large, huge_mapped;
+    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)

Quoted 8 lines

-        return __check_pmd_huge(addr, "AnonHugePages: ", nr_hpages, hpage_size); +    if (nr_hpages > 0) {
+        check_large = true;
+        granule = hpage_size;
+    } else {
+        check_large = false;
+        granule = psize();
+    }

This is incorrect for the mTHP large folio check. I already hit a selftest failure. Please test your patches before sending them out.

[root@]./khugepaged -c 4 mthp_khugepaged:anon TAP version 13
# Save THP and khugepaged settings... OK 1..4
# Allocate huge page on fault... OK
# Split huge PMD on MADV_DONTNEED... OK
ok 1 allocate on fault and split
#
# Run test: collapse_full (mthp_khugepaged:anon) # Collapse multiple fully populated PTE table.... OK ok 2 collapse_full
#

Quoted 8 lines

# Run test: collapse_empty (mthp_khugepaged:anon) # Do not collapse empty PTE table.... OK
ok 3 collapse_empty
#
# Run test: collapse_single_mthp (mthp_khugepaged:anon) # Collapse PTE table with half PTE entries present.... Fail not ok 4 collapse_single_mthp
# Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

FWIW, the CI flags this as well:

https://github.com/linux-mm/linux-mm/actions/runs/33028595020/job/98375618491

Signature · 4 lines

-- 
Cheers,

David