linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Wei Yang <richard.weiyang@gmail.com>
To: Zi Yan <ziy@nvidia.com>, g@master.kvack.org
Cc: Wei Yang <richard.weiyang@gmail.com>,
	wang lian <lianux.mm@gmail.com>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	David Hildenbrand <david@redhat.com>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Nico Pache <npache@redhat.com>,
	Ryan Roberts <ryan.roberts@arm.com>, Dev Jain <dev.jain@arm.com>,
	Barry Song <baohua@kernel.org>, Vlastimil Babka <vbabka@suse.cz>,
	Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>, Shuah Khan <shuah@kernel.org>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v3 4/4] selftests/mm: check after-split folio orders in split_huge_page_test.
Date: Thu, 14 Aug 2025 09:16:47 +0000	[thread overview]
Message-ID: <20250814091647.6prozsywma7qlugm@master> (raw)
In-Reply-To: <20250812155512.926011-5-ziy@nvidia.com>

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?

>@@ -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


  reply	other threads:[~2025-08-14  9:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-12 15:55 [PATCH v3 0/4] Better split_huge_page_test result check Zi Yan
2025-08-12 15:55 ` [PATCH v3 1/4] mm/huge_memory: add new_order and offset to split_huge_pages*() pr_debug Zi Yan
2025-08-12 15:55 ` [PATCH v3 2/4] selftests/mm: add check_folio_orders() helper Zi Yan
2025-08-13  3:38   ` wang lian
2025-08-14 17:50     ` Zi Yan
2025-08-13 21:12   ` Wei Yang
2025-08-13 21:56     ` Zi Yan
2025-08-12 15:55 ` [PATCH v3 3/4] selftests/mm: reimplement is_backed_by_thp() with more precise check Zi Yan
2025-08-13 21:41   ` Wei Yang
2025-08-13 21:58     ` Zi Yan
2025-08-12 15:55 ` [PATCH v3 4/4] selftests/mm: check after-split folio orders in split_huge_page_test Zi Yan
2025-08-14  9:16   ` Wei Yang [this message]
2025-08-14 13:35     ` Zi Yan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250814091647.6prozsywma7qlugm@master \
    --to=richard.weiyang@gmail.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@redhat.com \
    --cc=dev.jain@arm.com \
    --cc=g@master.kvack.org \
    --cc=lianux.mm@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mhocko@suse.com \
    --cc=npache@redhat.com \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox