Message-ID: <20250814091647.6prozsywma7qlugm@master>
Date: Thu, 14 Aug 2025 09:16:47 +0000
From: Wei Yang <richard.weiyang@...il.com>
To: Zi Yan <ziy@...dia.com>, g@...ter.smtp.subspace.kernel.org
Cc: Wei Yang <richard.weiyang@...il.com>, wang lian <lianux.mm@...il.com>,
	Baolin Wang <baolin.wang@...ux.alibaba.com>,
	David Hildenbrand <david@...hat.com>, linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Nico Pache <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
	Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
	Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Michal Hocko <mhocko@...e.com>, Shuah Khan <shuah@...nel.org>,
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v3 4/4] selftests/mm: check after-split folio orders in
 split_huge_page_test.

On Tue, Aug 12, 2025 at 11:55:12AM -0400, Zi Yan wrote:
>Instead of just checking the existence of PMD folios before and after folio
>split tests, use check_folio_orders() to check after-split folio orders.
>
>The following tests are not changed:
>1. split_pte_mapped_thp: the test already uses kpageflags to check;
>2. split_file_backed_thp: no vaddr available.
>
>Signed-off-by: Zi Yan <ziy@...dia.com>
>---
> .../selftests/mm/split_huge_page_test.c       | 85 +++++++++++++------
> 1 file changed, 61 insertions(+), 24 deletions(-)
>
>diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
>index 3aaf783f339f..1ea2c7f22962 100644
>--- a/tools/testing/selftests/mm/split_huge_page_test.c
>+++ b/tools/testing/selftests/mm/split_huge_page_test.c
>@@ -26,6 +26,7 @@ uint64_t pagesize;
> unsigned int pageshift;
> uint64_t pmd_pagesize;
> unsigned int pmd_order;
>+int *expected_orders;
> 
> #define SPLIT_DEBUGFS "/sys/kernel/debug/split_huge_pages"
> #define SMAP_PATH "/proc/self/smaps"
>@@ -37,6 +38,11 @@ unsigned int pmd_order;
> 
> #define GET_ORDER(nr_pages)    (31 - __builtin_clz(nr_pages))
> 
>+const char *pagemap_proc = "/proc/self/pagemap";
>+const char *kpageflags_proc = "/proc/kpageflags";
>+int pagemap_fd;
>+int kpageflags_fd;
>+
> int is_backed_by_folio(char *vaddr, int order, int pagemap_fd, int kpageflags_fd)
> {
> 	unsigned long pfn_head;
>@@ -49,18 +55,21 @@ int is_backed_by_folio(char *vaddr, int order, int pagemap_fd, int kpageflags_fd
> 
> 	pfn = pagemap_get_pfn(pagemap_fd, vaddr);
> 
>+	/* non present page */
> 	if (pfn == -1UL)
> 		return 0;
> 
> 	if (get_pfn_flags(pfn, kpageflags_fd, &pfn_flags))
> 		return 0;
> 
>+	/* check for order-0 pages */
> 	if (!order) {
> 		if (pfn_flags & (KPF_THP | KPF_COMPOUND_HEAD | KPF_COMPOUND_TAIL))
> 			return 0;
> 		return 1;
> 	}
> 
>+	/* non THP folio */
> 	if (!(pfn_flags & KPF_THP))
> 		return 0;
> 
>@@ -69,9 +78,11 @@ int is_backed_by_folio(char *vaddr, int order, int pagemap_fd, int kpageflags_fd
> 	if (get_pfn_flags(pfn_head, kpageflags_fd, &pfn_flags))
> 		return 0;
> 
>+	/* head PFN has no compound_head flag set */
> 	if (!(pfn_flags & (KPF_THP | KPF_COMPOUND_HEAD)))
> 		return 0;
> 
>+	/* check all tail PFN flags */
> 	for (i = 1; i < (1UL << order) - 1; i++) {
> 		if (get_pfn_flags(pfn_head + i, kpageflags_fd, &pfn_flags))
> 			return 0;

The comment in is_backed_by_folio() is more proper to be in previous patch?

>@@ -198,6 +209,12 @@ void split_pmd_thp_to_order(int order)
> 		if (one_page[i] != (char)i)
> 			ksft_exit_fail_msg("%ld byte corrupted\n", i);
> 
>+	memset(expected_orders, 0, sizeof(int) * (pmd_order + 1));
>+	expected_orders[order] = 4 << (pmd_order - order);
>+
>+	if (check_folio_orders(one_page, len, pagemap_fd, kpageflags_fd,
>+			       expected_orders, (pmd_order + 1)))
>+		ksft_exit_fail_msg("Unexpected THP split\n");
> 
> 	if (!check_huge_anon(one_page, 0, pmd_pagesize))
> 		ksft_exit_fail_msg("Still AnonHugePages not split\n");
>@@ -212,22 +229,6 @@ void split_pte_mapped_thp(void)
> 	size_t len = 4 * pmd_pagesize;
> 	uint64_t thp_size;
> 	size_t i;
>-	const char *pagemap_template = "/proc/%d/pagemap";
>-	const char *kpageflags_proc = "/proc/kpageflags";
>-	char pagemap_proc[255];
>-	int pagemap_fd;
>-	int kpageflags_fd;
>-
>-	if (snprintf(pagemap_proc, 255, pagemap_template, getpid()) < 0)
>-		ksft_exit_fail_msg("get pagemap proc error: %s\n", strerror(errno));
>-
>-	pagemap_fd = open(pagemap_proc, O_RDONLY);
>-	if (pagemap_fd == -1)
>-		ksft_exit_fail_msg("read pagemap: %s\n", strerror(errno));
>-
>-	kpageflags_fd = open(kpageflags_proc, O_RDONLY);
>-	if (kpageflags_fd == -1)
>-		ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));
> 
> 	one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
> 			MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>@@ -285,8 +286,6 @@ void split_pte_mapped_thp(void)
> 
> 	ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
> 	munmap(one_page, len);
>-	close(pagemap_fd);
>-	close(kpageflags_fd);
> }
> 
> void split_file_backed_thp(int order)
>@@ -489,6 +488,7 @@ void split_thp_in_pagecache_to_order_at(size_t fd_size, const char *fs_loc,
> 		int order, int offset)
> {
> 	int fd;
>+	char *split_addr;
> 	char *addr;
> 	size_t i;
> 	char testfile[INPUT_MAX];
>@@ -502,14 +502,27 @@ void split_thp_in_pagecache_to_order_at(size_t fd_size, const char *fs_loc,
> 	err = create_pagecache_thp_and_fd(testfile, fd_size, &fd, &addr);
> 	if (err)
> 		return;
>+
> 	err = 0;
> 
>-	if (offset == -1)
>-		write_debugfs(PID_FMT, getpid(), (uint64_t)addr,
>-			      (uint64_t)addr + fd_size, order);
>-	else
>-		write_debugfs(PID_FMT_OFFSET, getpid(), (uint64_t)addr,
>-			      (uint64_t)addr + fd_size, order, offset);
>+	memset(expected_orders, 0, sizeof(int) * (pmd_order + 1));

I am not familiar with split, you change it to split on each pmd_pagesize from
4 pmd_pagesize. Is there any difference?

>+	if (offset == -1) {
>+		for (split_addr = addr; split_addr < addr + fd_size; split_addr += pmd_pagesize)
>+			write_debugfs(PID_FMT, getpid(), (uint64_t)split_addr,
>+				      (uint64_t)split_addr + pagesize, order);

                                                ^--- here should be vaddr_end

Curious why not (uint64_t)split_addr + pmd_pagesize?

>+
>+		expected_orders[order] = fd_size / (pagesize << order);
>+	} else {
>+		int times = fd_size / pmd_pagesize;
>+
>+		for (split_addr = addr; split_addr < addr + fd_size; split_addr += pmd_pagesize)
>+			write_debugfs(PID_FMT_OFFSET, getpid(), (uint64_t)split_addr,
>+				      (uint64_t)split_addr + pagesize, order, offset);

As above.

>+
>+		for (i = order + 1; i < pmd_order; i++)
>+			expected_orders[i] = times;
>+		expected_orders[order] = 2 * times;
>+	}
> 
> 	for (i = 0; i < fd_size; i++)
> 		if (*(addr + i) != (char)i) {
>@@ -518,6 +531,13 @@ void split_thp_in_pagecache_to_order_at(size_t fd_size, const char *fs_loc,
> 			goto out;
> 		}
> 
>+	if (check_folio_orders(addr, fd_size, pagemap_fd, kpageflags_fd,
>+			       expected_orders, (pmd_order + 1))) {
>+		ksft_print_msg("Unexpected THP split\n");
>+		err = 1;
>+		goto out;
>+	}
>+
> 	if (!check_huge_file(addr, 0, pmd_pagesize)) {
> 		ksft_print_msg("Still FilePmdMapped not split\n");
> 		err = EXIT_FAILURE;
>@@ -569,9 +589,22 @@ int main(int argc, char **argv)
> 
> 	nr_pages = pmd_pagesize / pagesize;
> 	pmd_order = GET_ORDER(nr_pages);
>+
>+	expected_orders = (int *)malloc(sizeof(int) * (pmd_order + 1));
>+	if (!expected_orders)
>+		ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>+
> 	tests = 2 + (pmd_order - 1) + (2 * pmd_order) + (pmd_order - 1) * 4 + 2;
> 	ksft_set_plan(tests);
> 
>+	pagemap_fd = open(pagemap_proc, O_RDONLY);
>+	if (pagemap_fd == -1)
>+		ksft_exit_fail_msg("read pagemap: %s\n", strerror(errno));
>+
>+	kpageflags_fd = open(kpageflags_proc, O_RDONLY);
>+	if (kpageflags_fd == -1)
>+		ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));
>+
> 	fd_size = 2 * pmd_pagesize;
> 
> 	split_pmd_zero_pages();
>@@ -596,6 +629,10 @@ int main(int argc, char **argv)
> 			split_thp_in_pagecache_to_order_at(fd_size, fs_loc, i, offset);
> 	cleanup_thp_fs(fs_loc, created_tmp);
> 
>+	close(pagemap_fd);
>+	close(kpageflags_fd);
>+	free(expected_orders);
>+
> 	ksft_finished();
> 
> 	return 0;
>-- 
>2.47.2

-- 
Wei Yang
Help you, Help me

Powered by blists - more mailing lists