[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5981F8F3-9935-4570-B5CA-DD8B0FF43EDE@nvidia.com>
Date: Thu, 14 Aug 2025 09:35:08 -0400
From: Zi Yan <ziy@...dia.com>
To: Wei Yang <richard.weiyang@...il.com>
Cc: 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 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@...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?
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
Powered by blists - more mailing lists