Re: [PATCH v3 4/4] selftests/mm: check after-split folio orders in split_huge_page_test.

[Zi Yan <ziy@nvidia.com>]

On 14 Aug 2025, at 5:16, Wei Yang wrote:

> 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@nvidia.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?

Oops, these should be in the prior 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?

It is in V1->V2 changelog. split_huge_pages_pid() always step in PAGESIZE
to be able to split mremapped PTE-mapped THPs. Using [addr, addr + fd_size)
makes the PMD THP be split multiple times. The goal of this test is to
check the result of one non-uniform split,
so use [split_addr, split_addr + pagesize) to achieve that.

I will add the above to commit message and a comment here.

>
>> +
>> +		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.
>

See above.

--
Best Regards,
Yan, Zi