* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-01 19:10 ` David Hildenbrand
@ 2025-09-02 2:51 ` Wei Yang
2025-09-02 7:49 ` David Hildenbrand
2025-09-02 8:16 ` Wei Yang
2025-09-02 14:56 ` Zi Yan
2 siblings, 1 reply; 23+ messages in thread
From: Wei Yang @ 2025-09-02 2:51 UTC (permalink / raw)
To: David Hildenbrand
Cc: Zi Yan, Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On Mon, Sep 01, 2025 at 09:10:57PM +0200, David Hildenbrand wrote:
>> > > > (a) Will this actually do anything? Also, maybe it does now, but can't the kernel just optimize that out in the future?
>> > >
>> > > It remaps each subpage of 4 PMD THPs into a contiguous 2MB vaddr range and
>> > > perform split on that range.
>> >
>> > I'm afraid I am missing the "why".
>> >
>> > I would have thought that a "split_pte_mapped_thp" test would want to pte-map THPs
>> > to the see if they can be split.
>> >
>> > Why is the mremap required? IOW, what exactly is the test trying to test that
>> > exceeds "split_pte_mapped_thp" ?
>>
>> IMHO, it is an interesting test case for splitting a THP when only a subpage
>> is mapped into a vaddr range and in a contiguous vaddr each page comes from
>> different THPs.
>
>Right. Slightly similar to just MAV_DONTNEED'ing the other PTEs and trying to
>split the bigger range.
>
>Of course, if you involve more mremap, the RMAP logic of installing migration
>ptes will get stressed more.
>
>So yes, there are various ways on how to stress the RMAP walk when splitting.
>
>> The mprotect test case you are mentioning would still have all
>> subpages mapped by contiguous vaddrs.
>
>Right, it would not stress RMAP as much.
>
>>
>> But if you think both are just testing PTE-mapped THPs, feel free to replace the
>> existing one with the mprotect test case. In addition, is_backed_by_folio()
>> can be reverted back to its prior version, since it no longer needs to handle
>> the case where subpages from different THPs can be mapped into a vaddr range.
>
>Oh, the is_backed_by_folio() change is actually really valuable.
>
>
>I think I was confused by the implementation that works on a single virtual address
>range with multiple different variables, questioning why we mremap at all.
>
>I tried cleaning up that test myself and ended up with the following (it
>escalated a bit). If that looks cleaner to you as well, I can submit that as a
>patch.
>
>diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
>index 10ae65ea032f6..aa0f0502efa06 100644
>--- a/tools/testing/selftests/mm/split_huge_page_test.c
>+++ b/tools/testing/selftests/mm/split_huge_page_test.c
>@@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
> static void split_pte_mapped_thp(void)
> {
>- char *one_page, *pte_mapped, *pte_mapped2;
>- size_t len = 4 * pmd_pagesize;
>- uint64_t thp_size;
>+ const size_t nr_thps = 4;
>+ const size_t thp_area_size = nr_thps * pmd_pagesize;
>+ const size_t page_area_size = nr_thps * pagesize;
>+ char *thp_area, *page_area = NULL, *tmp;
> size_t i;
>- one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>+ thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>- if (one_page == MAP_FAILED)
>- ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>+ if (thp_area == MAP_FAILED) {
>+ ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>+ goto out;
>+ }
>- madvise(one_page, len, MADV_HUGEPAGE);
>+ madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>- for (i = 0; i < len; i++)
>- one_page[i] = (char)i;
>+ for (i = 0; i < thp_area_size; i++)
>+ thp_area[i] = (char)i;
>- if (!check_huge_anon(one_page, 4, pmd_pagesize))
>- ksft_exit_fail_msg("No THP is allocated\n");
>+ if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>+ ksft_test_result_skip("Not all THPs allocated\n");
>+ goto out;
>+ }
>- /* remap the first pagesize of first THP */
>- pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
>-
>- /* remap the Nth pagesize of Nth THP */
>- for (i = 1; i < 4; i++) {
>- pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
>- pagesize, pagesize,
>- MREMAP_MAYMOVE|MREMAP_FIXED,
>- pte_mapped + pagesize * i);
>- if (pte_mapped2 == MAP_FAILED)
>- ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
>- }
>-
>- /* smap does not show THPs after mremap, use kpageflags instead */
>- thp_size = 0;
>- for (i = 0; i < pagesize * 4; i++)
>- if (i % pagesize == 0 &&
>- is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
>- thp_size++;
>-
>- if (thp_size != 4)
>- ksft_exit_fail_msg("Some THPs are missing during mremap\n");
>-
>- /* split all remapped THPs */
>- write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
>- (uint64_t)pte_mapped + pagesize * 4, 0);
>-
>- /* smap does not show THPs after mremap, use kpageflags instead */
>- thp_size = 0;
>- for (i = 0; i < pagesize * 4; i++) {
>- if (pte_mapped[i] != (char)i)
>- ksft_exit_fail_msg("%ld byte corrupted\n", i);
>+ /*
>+ * To challenge spitting code, we will mremap page[x] of the
>+ * thp[x] into a smaller area, and trigger the split from that
>+ * smaller area. This will end up replacing the PMD mappings in
>+ * the thp_area by PTE mappings first, leaving the THPs unsplit.
>+ */
>+ page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>+ if (page_area == MAP_FAILED) {
>+ ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>+ goto out;
>+ }
>- if (i % pagesize == 0 &&
>- !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>- thp_size++;
>+ for (i = 0; i < nr_thps; i++) {
>+ tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>+ pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>+ page_area + pagesize * i);
Would this create one hole at the beginning of each 2M range and cause
splitting underlining THP?
>+ if (tmp != MAP_FAILED)
>+ continue;
>+ ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
>+ goto out;
>+ }
>+
>+ /*
>+ * Verify that our THPs were not split yet. Note that
>+ * check_huge_anon() cannot be used as it checks for PMD mappings.
>+ */
>+ for (i = 0; i < nr_thps; i++) {
>+ if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
>+ pagemap_fd, kpageflags_fd))
>+ continue;
>+ ksft_test_result_fail("THP %zu missing after mremap\n", i);
>+ goto out;
> }
>- if (thp_size)
>- ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
>+ /* Split all THPs through the remapped pages. */
>+ write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
>+ (uint64_t)page_area + page_area_size, 0);
>+
>+ /* Corruption during mremap or split? */
>+ for (i = 0; i < page_area_size; i++) {
>+ if (page_area[i] == (char)i)
>+ continue;
>+ ksft_test_result_fail("%zu byte corrupted\n", i);
>+ goto out;
>+ }
>+
>+ /* Split failed? */
>+ for (i = 0; i < nr_thps; i++) {
>+ if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
>+ continue;
>+ ksft_test_result_fail("THP %zu not split\n", i);
>+ }
> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
>- munmap(one_page, len);
>+out:
>+ munmap(thp_area, thp_area_size);
>+ if (page_area)
>+ munmap(page_area, page_area_size);
> }
> static void split_file_backed_thp(int order)
>--
>2.50.1
>
>
>--
>Cheers
>
>David / dhildenb
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 2:51 ` Wei Yang
@ 2025-09-02 7:49 ` David Hildenbrand
2025-09-02 8:13 ` Wei Yang
0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand @ 2025-09-02 7:49 UTC (permalink / raw)
To: Wei Yang
Cc: Zi Yan, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
>> + /*
>> + * To challenge spitting code, we will mremap page[x] of the
>> + * thp[x] into a smaller area, and trigger the split from that
>> + * smaller area. This will end up replacing the PMD mappings in
>> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>> + */
>> + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>> + if (page_area == MAP_FAILED) {
>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>> + goto out;
>> + }
>> - if (i % pagesize == 0 &&
>> - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>> - thp_size++;
>> + for (i = 0; i < nr_thps; i++) {
>> + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>> + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>> + page_area + pagesize * i);
>
> Would this create one hole at the beginning of each 2M range and cause
> splitting underlining THP?
Yes, it will create a hole (this also happens in the old code).
As the comment above it now states: "leaving the THPs unsplit"
There is a check verifying that after this mremap code.
--
Cheers
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 7:49 ` David Hildenbrand
@ 2025-09-02 8:13 ` Wei Yang
2025-09-02 8:23 ` David Hildenbrand
0 siblings, 1 reply; 23+ messages in thread
From: Wei Yang @ 2025-09-02 8:13 UTC (permalink / raw)
To: David Hildenbrand
Cc: Wei Yang, Zi Yan, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On Tue, Sep 02, 2025 at 09:49:48AM +0200, David Hildenbrand wrote:
>> > + /*
>> > + * To challenge spitting code, we will mremap page[x] of the
>> > + * thp[x] into a smaller area, and trigger the split from that
>> > + * smaller area. This will end up replacing the PMD mappings in
>> > + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>> > + */
This confuse me a little. How about move "and trigger the split from that
smaller area" to the end? So that I know mremap convert to pte-mapping and
leave thp unsplit. Then we do the split.
>> > + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>> > + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>> > + if (page_area == MAP_FAILED) {
>> > + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>> > + goto out;
>> > + }
>> > - if (i % pagesize == 0 &&
>> > - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>> > - thp_size++;
>> > + for (i = 0; i < nr_thps; i++) {
>> > + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>> > + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>> > + page_area + pagesize * i);
>>
>> Would this create one hole at the beginning of each 2M range and cause
>> splitting underlining THP?
>
>Yes, it will create a hole (this also happens in the old code).
>
>As the comment above it now states: "leaving the THPs unsplit"
>
>There is a check verifying that after this mremap code.
>
Thanks for clarifying. One nit to the comment above.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 8:13 ` Wei Yang
@ 2025-09-02 8:23 ` David Hildenbrand
2025-09-02 8:28 ` Wei Yang
0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand @ 2025-09-02 8:23 UTC (permalink / raw)
To: Wei Yang
Cc: Zi Yan, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On 02.09.25 10:13, Wei Yang wrote:
> On Tue, Sep 02, 2025 at 09:49:48AM +0200, David Hildenbrand wrote:
>>>> + /*
>>>> + * To challenge spitting code, we will mremap page[x] of the
>>>> + * thp[x] into a smaller area, and trigger the split from that
>>>> + * smaller area. This will end up replacing the PMD mappings in
>>>> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>>>> + */
>
> This confuse me a little. How about move "and trigger the split from that
> smaller area" to the end? So that I know mremap convert to pte-mapping and
> leave thp unsplit. Then we do the split.
>
After spleeping over it, what about the following?
+ /*
+ * To challenge spitting code, we will mremap a single page of each
+ * THP (page[i] of thp[i]) in the thp_area into page_area. This will
+ * replace the PMD mappings in the thp_area by PTE mappings first,
+ * but leaving the THP unsplit, to then create a page-sized hole in
+ * the thp_area.
+ * We will then manually trigger splitting of all THPs through the
+ * single mremap'ed pages of each THP in the page_area.
+ */
--
Cheers
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 8:23 ` David Hildenbrand
@ 2025-09-02 8:28 ` Wei Yang
0 siblings, 0 replies; 23+ messages in thread
From: Wei Yang @ 2025-09-02 8:28 UTC (permalink / raw)
To: David Hildenbrand
Cc: Wei Yang, Zi Yan, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On Tue, Sep 02, 2025 at 10:23:51AM +0200, David Hildenbrand wrote:
>On 02.09.25 10:13, Wei Yang wrote:
>> On Tue, Sep 02, 2025 at 09:49:48AM +0200, David Hildenbrand wrote:
>> > > > + /*
>> > > > + * To challenge spitting code, we will mremap page[x] of the
>> > > > + * thp[x] into a smaller area, and trigger the split from that
>> > > > + * smaller area. This will end up replacing the PMD mappings in
>> > > > + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>> > > > + */
>>
>> This confuse me a little. How about move "and trigger the split from that
>> smaller area" to the end? So that I know mremap convert to pte-mapping and
>> leave thp unsplit. Then we do the split.
>>
>
>After spleeping over it, what about the following?
>
>+ /*
>+ * To challenge spitting code, we will mremap a single page of each
>+ * THP (page[i] of thp[i]) in the thp_area into page_area. This will
>+ * replace the PMD mappings in the thp_area by PTE mappings first,
>+ * but leaving the THP unsplit, to then create a page-sized hole in
>+ * the thp_area.
>+ * We will then manually trigger splitting of all THPs through the
>+ * single mremap'ed pages of each THP in the page_area.
>+ */
>
Much better, thanks :-)
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-01 19:10 ` David Hildenbrand
2025-09-02 2:51 ` Wei Yang
@ 2025-09-02 8:16 ` Wei Yang
2025-09-02 8:26 ` David Hildenbrand
2025-09-02 14:56 ` Zi Yan
2 siblings, 1 reply; 23+ messages in thread
From: Wei Yang @ 2025-09-02 8:16 UTC (permalink / raw)
To: David Hildenbrand
Cc: Zi Yan, Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On Mon, Sep 01, 2025 at 09:10:57PM +0200, David Hildenbrand wrote:
[...]
>diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
>index 10ae65ea032f6..aa0f0502efa06 100644
>--- a/tools/testing/selftests/mm/split_huge_page_test.c
>+++ b/tools/testing/selftests/mm/split_huge_page_test.c
>@@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
> static void split_pte_mapped_thp(void)
> {
>- char *one_page, *pte_mapped, *pte_mapped2;
>- size_t len = 4 * pmd_pagesize;
>- uint64_t thp_size;
>+ const size_t nr_thps = 4;
>+ const size_t thp_area_size = nr_thps * pmd_pagesize;
>+ const size_t page_area_size = nr_thps * pagesize;
>+ char *thp_area, *page_area = NULL, *tmp;
> size_t i;
>- one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>+ thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>- if (one_page == MAP_FAILED)
>- ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>+ if (thp_area == MAP_FAILED) {
>+ ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>+ goto out;
>+ }
>- madvise(one_page, len, MADV_HUGEPAGE);
>+ madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>- for (i = 0; i < len; i++)
>- one_page[i] = (char)i;
>+ for (i = 0; i < thp_area_size; i++)
>+ thp_area[i] = (char)i;
>- if (!check_huge_anon(one_page, 4, pmd_pagesize))
>- ksft_exit_fail_msg("No THP is allocated\n");
>+ if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>+ ksft_test_result_skip("Not all THPs allocated\n");
Not sure why we skip here?
>+ goto out;
>+ }
>- /* remap the first pagesize of first THP */
>- pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
>-
>- /* remap the Nth pagesize of Nth THP */
>- for (i = 1; i < 4; i++) {
>- pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
>- pagesize, pagesize,
>- MREMAP_MAYMOVE|MREMAP_FIXED,
>- pte_mapped + pagesize * i);
>- if (pte_mapped2 == MAP_FAILED)
>- ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
>- }
>-
>- /* smap does not show THPs after mremap, use kpageflags instead */
>- thp_size = 0;
>- for (i = 0; i < pagesize * 4; i++)
>- if (i % pagesize == 0 &&
>- is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
>- thp_size++;
>-
>- if (thp_size != 4)
>- ksft_exit_fail_msg("Some THPs are missing during mremap\n");
>-
>- /* split all remapped THPs */
>- write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
>- (uint64_t)pte_mapped + pagesize * 4, 0);
>-
>- /* smap does not show THPs after mremap, use kpageflags instead */
>- thp_size = 0;
>- for (i = 0; i < pagesize * 4; i++) {
>- if (pte_mapped[i] != (char)i)
>- ksft_exit_fail_msg("%ld byte corrupted\n", i);
>+ /*
>+ * To challenge spitting code, we will mremap page[x] of the
>+ * thp[x] into a smaller area, and trigger the split from that
>+ * smaller area. This will end up replacing the PMD mappings in
>+ * the thp_area by PTE mappings first, leaving the THPs unsplit.
>+ */
>+ page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>+ if (page_area == MAP_FAILED) {
>+ ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>+ goto out;
>+ }
>- if (i % pagesize == 0 &&
>- !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>- thp_size++;
>+ for (i = 0; i < nr_thps; i++) {
>+ tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>+ pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>+ page_area + pagesize * i);
>+ if (tmp != MAP_FAILED)
>+ continue;
>+ ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
>+ goto out;
>+ }
>+
>+ /*
>+ * Verify that our THPs were not split yet. Note that
>+ * check_huge_anon() cannot be used as it checks for PMD mappings.
>+ */
>+ for (i = 0; i < nr_thps; i++) {
>+ if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
>+ pagemap_fd, kpageflags_fd))
>+ continue;
>+ ksft_test_result_fail("THP %zu missing after mremap\n", i);
>+ goto out;
> }
>- if (thp_size)
>- ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
>+ /* Split all THPs through the remapped pages. */
>+ write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
>+ (uint64_t)page_area + page_area_size, 0);
>+
>+ /* Corruption during mremap or split? */
>+ for (i = 0; i < page_area_size; i++) {
>+ if (page_area[i] == (char)i)
>+ continue;
>+ ksft_test_result_fail("%zu byte corrupted\n", i);
>+ goto out;
>+ }
>+
>+ /* Split failed? */
>+ for (i = 0; i < nr_thps; i++) {
>+ if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
I guess we want to check (page_area + i * pagesize) here?
>+ continue;
>+ ksft_test_result_fail("THP %zu not split\n", i);
>+ }
> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
>- munmap(one_page, len);
>+out:
>+ munmap(thp_area, thp_area_size);
>+ if (page_area)
>+ munmap(page_area, page_area_size);
> }
> static void split_file_backed_thp(int order)
>--
>2.50.1
>
>
>--
>Cheers
>
>David / dhildenb
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 8:16 ` Wei Yang
@ 2025-09-02 8:26 ` David Hildenbrand
0 siblings, 0 replies; 23+ messages in thread
From: David Hildenbrand @ 2025-09-02 8:26 UTC (permalink / raw)
To: Wei Yang
Cc: Zi Yan, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On 02.09.25 10:16, Wei Yang wrote:
> On Mon, Sep 01, 2025 at 09:10:57PM +0200, David Hildenbrand wrote:
> [...]
>> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
>> index 10ae65ea032f6..aa0f0502efa06 100644
>> --- a/tools/testing/selftests/mm/split_huge_page_test.c
>> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
>> @@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
>> static void split_pte_mapped_thp(void)
>> {
>> - char *one_page, *pte_mapped, *pte_mapped2;
>> - size_t len = 4 * pmd_pagesize;
>> - uint64_t thp_size;
>> + const size_t nr_thps = 4;
>> + const size_t thp_area_size = nr_thps * pmd_pagesize;
>> + const size_t page_area_size = nr_thps * pagesize;
>> + char *thp_area, *page_area = NULL, *tmp;
>> size_t i;
>> - one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>> + thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
>> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>> - if (one_page == MAP_FAILED)
>> - ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>> + if (thp_area == MAP_FAILED) {
>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>> + goto out;
>> + }
>> - madvise(one_page, len, MADV_HUGEPAGE);
>> + madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>> - for (i = 0; i < len; i++)
>> - one_page[i] = (char)i;
>> + for (i = 0; i < thp_area_size; i++)
>> + thp_area[i] = (char)i;
>> - if (!check_huge_anon(one_page, 4, pmd_pagesize))
>> - ksft_exit_fail_msg("No THP is allocated\n");
>> + if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>> + ksft_test_result_skip("Not all THPs allocated\n");
>
> Not sure why we skip here?
We usually don't want tests to fail simply because our memory is fragmented.
If I'm going to send this I'll comment on all changes in the patch
description.
[...]
>> +
>> + /* Split failed? */
>> + for (i = 0; i < nr_thps; i++) {
>> + if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
>
> I guess we want to check (page_area + i * pagesize) here?
yes!
--
Cheers
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-01 19:10 ` David Hildenbrand
2025-09-02 2:51 ` Wei Yang
2025-09-02 8:16 ` Wei Yang
@ 2025-09-02 14:56 ` Zi Yan
2025-09-02 15:28 ` David Hildenbrand
2 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2025-09-02 14:56 UTC (permalink / raw)
To: David Hildenbrand
Cc: Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On 1 Sep 2025, at 15:10, David Hildenbrand wrote:
>>>>> (a) Will this actually do anything? Also, maybe it does now, but can't the kernel just optimize that out in the future?
>>>>
>>>> It remaps each subpage of 4 PMD THPs into a contiguous 2MB vaddr range and
>>>> perform split on that range.
>>>
>>> I'm afraid I am missing the "why".
>>>
>>> I would have thought that a "split_pte_mapped_thp" test would want to pte-map THPs
>>> to the see if they can be split.
>>>
>>> Why is the mremap required? IOW, what exactly is the test trying to test that
>>> exceeds "split_pte_mapped_thp" ?
>>
>> IMHO, it is an interesting test case for splitting a THP when only a subpage
>> is mapped into a vaddr range and in a contiguous vaddr each page comes from
>> different THPs.
>
> Right. Slightly similar to just MAV_DONTNEED'ing the other PTEs and trying to
> split the bigger range.
>
> Of course, if you involve more mremap, the RMAP logic of installing migration
> ptes will get stressed more.
>
> So yes, there are various ways on how to stress the RMAP walk when splitting.
>
>> The mprotect test case you are mentioning would still have all
>> subpages mapped by contiguous vaddrs.
>
> Right, it would not stress RMAP as much.
>
>>
>> But if you think both are just testing PTE-mapped THPs, feel free to replace the
>> existing one with the mprotect test case. In addition, is_backed_by_folio()
>> can be reverted back to its prior version, since it no longer needs to handle
>> the case where subpages from different THPs can be mapped into a vaddr range.
>
> Oh, the is_backed_by_folio() change is actually really valuable.
>
>
> I think I was confused by the implementation that works on a single virtual address
> range with multiple different variables, questioning why we mremap at all.
>
> I tried cleaning up that test myself and ended up with the following (it
> escalated a bit). If that looks cleaner to you as well, I can submit that as a
> patch.
>
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index 10ae65ea032f6..aa0f0502efa06 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
> static void split_pte_mapped_thp(void)
> {
> - char *one_page, *pte_mapped, *pte_mapped2;
> - size_t len = 4 * pmd_pagesize;
> - uint64_t thp_size;
> + const size_t nr_thps = 4;
> + const size_t thp_area_size = nr_thps * pmd_pagesize;
> + const size_t page_area_size = nr_thps * pagesize;
> + char *thp_area, *page_area = NULL, *tmp;
> size_t i;
> - one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
> + thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> - if (one_page == MAP_FAILED)
> - ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
> + if (thp_area == MAP_FAILED) {
> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
> + goto out;
thp_area mmap failed and out label will try to munmap MAP_FAILED, which is
(void *) -1. munmap will fail with -EINVAL.
> + }
> - madvise(one_page, len, MADV_HUGEPAGE);
> + madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
> - for (i = 0; i < len; i++)
> - one_page[i] = (char)i;
> + for (i = 0; i < thp_area_size; i++)
> + thp_area[i] = (char)i;
> - if (!check_huge_anon(one_page, 4, pmd_pagesize))
> - ksft_exit_fail_msg("No THP is allocated\n");
> + if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
> + ksft_test_result_skip("Not all THPs allocated\n");
> + goto out;
> + }
> - /* remap the first pagesize of first THP */
> - pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
> -
> - /* remap the Nth pagesize of Nth THP */
> - for (i = 1; i < 4; i++) {
> - pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
> - pagesize, pagesize,
> - MREMAP_MAYMOVE|MREMAP_FIXED,
> - pte_mapped + pagesize * i);
> - if (pte_mapped2 == MAP_FAILED)
> - ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
> - }
> -
> - /* smap does not show THPs after mremap, use kpageflags instead */
> - thp_size = 0;
> - for (i = 0; i < pagesize * 4; i++)
> - if (i % pagesize == 0 &&
> - is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
> - thp_size++;
> -
> - if (thp_size != 4)
> - ksft_exit_fail_msg("Some THPs are missing during mremap\n");
> -
> - /* split all remapped THPs */
> - write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
> - (uint64_t)pte_mapped + pagesize * 4, 0);
> -
> - /* smap does not show THPs after mremap, use kpageflags instead */
> - thp_size = 0;
> - for (i = 0; i < pagesize * 4; i++) {
> - if (pte_mapped[i] != (char)i)
> - ksft_exit_fail_msg("%ld byte corrupted\n", i);
> + /*
> + * To challenge spitting code, we will mremap page[x] of the
> + * thp[x] into a smaller area, and trigger the split from that
> + * smaller area. This will end up replacing the PMD mappings in
> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
> + */
> + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> + if (page_area == MAP_FAILED) {
> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
> + goto out;
> + }
> - if (i % pagesize == 0 &&
> - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
> - thp_size++;
> + for (i = 0; i < nr_thps; i++) {
> + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
> + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
> + page_area + pagesize * i);
> + if (tmp != MAP_FAILED)
> + continue;
> + ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
> + goto out;
> + }
> +
> + /*
> + * Verify that our THPs were not split yet. Note that
> + * check_huge_anon() cannot be used as it checks for PMD mappings.
> + */
> + for (i = 0; i < nr_thps; i++) {
> + if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
> + pagemap_fd, kpageflags_fd))
> + continue;
> + ksft_test_result_fail("THP %zu missing after mremap\n", i);
> + goto out;
> }
> - if (thp_size)
> - ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
> + /* Split all THPs through the remapped pages. */
> + write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
> + (uint64_t)page_area + page_area_size, 0);
> +
> + /* Corruption during mremap or split? */
> + for (i = 0; i < page_area_size; i++) {
> + if (page_area[i] == (char)i)
> + continue;
> + ksft_test_result_fail("%zu byte corrupted\n", i);
> + goto out;
> + }
> +
> + /* Split failed? */
> + for (i = 0; i < nr_thps; i++) {
> + if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
page_area + i * pagesize, like Wei pointed out in another email.
> + continue;
> + ksft_test_result_fail("THP %zu not split\n", i);
> + }
> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
> - munmap(one_page, len);
> +out:
> + munmap(thp_area, thp_area_size);
> + if (page_area)
> + munmap(page_area, page_area_size);
> }
> static void split_file_backed_thp(int order)
> --
> 2.50.1
Otherwise, LGTM. With all the changes in this email and other email,
feel free to add Reviewed-by: Zi Yan <ziy@nvidia.com> when you send it
out formally.
Thank you for cleaning this up.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 14:56 ` Zi Yan
@ 2025-09-02 15:28 ` David Hildenbrand
2025-09-02 15:39 ` Zi Yan
2025-09-02 15:40 ` David Hildenbrand
0 siblings, 2 replies; 23+ messages in thread
From: David Hildenbrand @ 2025-09-02 15:28 UTC (permalink / raw)
To: Zi Yan
Cc: Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
[...]
>> @@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
>> static void split_pte_mapped_thp(void)
>> {
>> - char *one_page, *pte_mapped, *pte_mapped2;
>> - size_t len = 4 * pmd_pagesize;
>> - uint64_t thp_size;
>> + const size_t nr_thps = 4;
>> + const size_t thp_area_size = nr_thps * pmd_pagesize;
>> + const size_t page_area_size = nr_thps * pagesize;
>> + char *thp_area, *page_area = NULL, *tmp;
>> size_t i;
>> - one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>> + thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
>> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>> - if (one_page == MAP_FAILED)
>> - ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>> + if (thp_area == MAP_FAILED) {
>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>> + goto out;
>
> thp_area mmap failed and out label will try to munmap MAP_FAILED, which is
> (void *) -1. munmap will fail with -EINVAL.
Indeed, should just be a "return;"
>
>> + }
>> - madvise(one_page, len, MADV_HUGEPAGE);
>> + madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>> - for (i = 0; i < len; i++)
>> - one_page[i] = (char)i;
>> + for (i = 0; i < thp_area_size; i++)
>> + thp_area[i] = (char)i;
>> - if (!check_huge_anon(one_page, 4, pmd_pagesize))
>> - ksft_exit_fail_msg("No THP is allocated\n");
>> + if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>> + ksft_test_result_skip("Not all THPs allocated\n");
>> + goto out;
>> + }
>> - /* remap the first pagesize of first THP */
>> - pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
>> -
>> - /* remap the Nth pagesize of Nth THP */
>> - for (i = 1; i < 4; i++) {
>> - pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
>> - pagesize, pagesize,
>> - MREMAP_MAYMOVE|MREMAP_FIXED,
>> - pte_mapped + pagesize * i);
>> - if (pte_mapped2 == MAP_FAILED)
>> - ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
>> - }
>> -
>> - /* smap does not show THPs after mremap, use kpageflags instead */
>> - thp_size = 0;
>> - for (i = 0; i < pagesize * 4; i++)
>> - if (i % pagesize == 0 &&
>> - is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
>> - thp_size++;
>> -
>> - if (thp_size != 4)
>> - ksft_exit_fail_msg("Some THPs are missing during mremap\n");
>> -
>> - /* split all remapped THPs */
>> - write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
>> - (uint64_t)pte_mapped + pagesize * 4, 0);
>> -
>> - /* smap does not show THPs after mremap, use kpageflags instead */
>> - thp_size = 0;
>> - for (i = 0; i < pagesize * 4; i++) {
>> - if (pte_mapped[i] != (char)i)
>> - ksft_exit_fail_msg("%ld byte corrupted\n", i);
>> + /*
>> + * To challenge spitting code, we will mremap page[x] of the
>> + * thp[x] into a smaller area, and trigger the split from that
>> + * smaller area. This will end up replacing the PMD mappings in
>> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>> + */
>> + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>> + if (page_area == MAP_FAILED) {
>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>> + goto out;
>> + }
>> - if (i % pagesize == 0 &&
>> - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>> - thp_size++;
>> + for (i = 0; i < nr_thps; i++) {
>> + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>> + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>> + page_area + pagesize * i);
>> + if (tmp != MAP_FAILED)
>> + continue;
>> + ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
>> + goto out;
>> + }
>> +
>> + /*
>> + * Verify that our THPs were not split yet. Note that
>> + * check_huge_anon() cannot be used as it checks for PMD mappings.
>> + */
>> + for (i = 0; i < nr_thps; i++) {
>> + if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
>> + pagemap_fd, kpageflags_fd))
>> + continue;
>> + ksft_test_result_fail("THP %zu missing after mremap\n", i);
>> + goto out;
>> }
>> - if (thp_size)
>> - ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
>> + /* Split all THPs through the remapped pages. */
>> + write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
>> + (uint64_t)page_area + page_area_size, 0);
>> +
>> + /* Corruption during mremap or split? */
>> + for (i = 0; i < page_area_size; i++) {
>> + if (page_area[i] == (char)i)
>> + continue;
>> + ksft_test_result_fail("%zu byte corrupted\n", i);
>> + goto out;
>> + }
>> +
>> + /* Split failed? */
>> + for (i = 0; i < nr_thps; i++) {
>> + if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
>
> page_area + i * pagesize, like Wei pointed out in another email.
>
>> + continue;
>> + ksft_test_result_fail("THP %zu not split\n", i);
>> + }
>> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
>> - munmap(one_page, len);
>> +out:
>> + munmap(thp_area, thp_area_size);
>> + if (page_area)
>> + munmap(page_area, page_area_size);
>> }
>> static void split_file_backed_thp(int order)
>> --
>> 2.50.1
>
> Otherwise, LGTM. With all the changes in this email and other email,
> feel free to add Reviewed-by: Zi Yan <ziy@nvidia.com> when you send it
> out formally.
Thanks!
I'm currently chasing why I keep getting temporary
Bail out! Some THPs are missing during mremap
Already on the old test. Something doesn't work quite right as it seems.
--
Cheers
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 15:28 ` David Hildenbrand
@ 2025-09-02 15:39 ` Zi Yan
2025-09-02 15:40 ` David Hildenbrand
1 sibling, 0 replies; 23+ messages in thread
From: Zi Yan @ 2025-09-02 15:39 UTC (permalink / raw)
To: David Hildenbrand
Cc: Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On 2 Sep 2025, at 11:28, David Hildenbrand wrote:
> [...]
>
>>> @@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
>>> static void split_pte_mapped_thp(void)
>>> {
>>> - char *one_page, *pte_mapped, *pte_mapped2;
>>> - size_t len = 4 * pmd_pagesize;
>>> - uint64_t thp_size;
>>> + const size_t nr_thps = 4;
>>> + const size_t thp_area_size = nr_thps * pmd_pagesize;
>>> + const size_t page_area_size = nr_thps * pagesize;
>>> + char *thp_area, *page_area = NULL, *tmp;
>>> size_t i;
>>> - one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>>> + thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
>>> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>> - if (one_page == MAP_FAILED)
>>> - ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>>> + if (thp_area == MAP_FAILED) {
>>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>>> + goto out;
>>
>> thp_area mmap failed and out label will try to munmap MAP_FAILED, which is
>> (void *) -1. munmap will fail with -EINVAL.
>
> Indeed, should just be a "return;"
>
>>
>>> + }
>>> - madvise(one_page, len, MADV_HUGEPAGE);
>>> + madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>>> - for (i = 0; i < len; i++)
>>> - one_page[i] = (char)i;
>>> + for (i = 0; i < thp_area_size; i++)
>>> + thp_area[i] = (char)i;
>>> - if (!check_huge_anon(one_page, 4, pmd_pagesize))
>>> - ksft_exit_fail_msg("No THP is allocated\n");
>>> + if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>>> + ksft_test_result_skip("Not all THPs allocated\n");
>>> + goto out;
>>> + }
>>> - /* remap the first pagesize of first THP */
>>> - pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
>>> -
>>> - /* remap the Nth pagesize of Nth THP */
>>> - for (i = 1; i < 4; i++) {
>>> - pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
>>> - pagesize, pagesize,
>>> - MREMAP_MAYMOVE|MREMAP_FIXED,
>>> - pte_mapped + pagesize * i);
>>> - if (pte_mapped2 == MAP_FAILED)
>>> - ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
>>> - }
>>> -
>>> - /* smap does not show THPs after mremap, use kpageflags instead */
>>> - thp_size = 0;
>>> - for (i = 0; i < pagesize * 4; i++)
>>> - if (i % pagesize == 0 &&
>>> - is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
>>> - thp_size++;
>>> -
>>> - if (thp_size != 4)
>>> - ksft_exit_fail_msg("Some THPs are missing during mremap\n");
>>> -
>>> - /* split all remapped THPs */
>>> - write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
>>> - (uint64_t)pte_mapped + pagesize * 4, 0);
>>> -
>>> - /* smap does not show THPs after mremap, use kpageflags instead */
>>> - thp_size = 0;
>>> - for (i = 0; i < pagesize * 4; i++) {
>>> - if (pte_mapped[i] != (char)i)
>>> - ksft_exit_fail_msg("%ld byte corrupted\n", i);
>>> + /*
>>> + * To challenge spitting code, we will mremap page[x] of the
>>> + * thp[x] into a smaller area, and trigger the split from that
>>> + * smaller area. This will end up replacing the PMD mappings in
>>> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>>> + */
>>> + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>>> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>> + if (page_area == MAP_FAILED) {
>>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>>> + goto out;
>>> + }
>>> - if (i % pagesize == 0 &&
>>> - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>>> - thp_size++;
>>> + for (i = 0; i < nr_thps; i++) {
>>> + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>>> + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>>> + page_area + pagesize * i);
>>> + if (tmp != MAP_FAILED)
>>> + continue;
>>> + ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
>>> + goto out;
>>> + }
>>> +
>>> + /*
>>> + * Verify that our THPs were not split yet. Note that
>>> + * check_huge_anon() cannot be used as it checks for PMD mappings.
>>> + */
>>> + for (i = 0; i < nr_thps; i++) {
>>> + if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
>>> + pagemap_fd, kpageflags_fd))
>>> + continue;
>>> + ksft_test_result_fail("THP %zu missing after mremap\n", i);
>>> + goto out;
>>> }
>>> - if (thp_size)
>>> - ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
>>> + /* Split all THPs through the remapped pages. */
>>> + write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
>>> + (uint64_t)page_area + page_area_size, 0);
>>> +
>>> + /* Corruption during mremap or split? */
>>> + for (i = 0; i < page_area_size; i++) {
>>> + if (page_area[i] == (char)i)
>>> + continue;
>>> + ksft_test_result_fail("%zu byte corrupted\n", i);
>>> + goto out;
>>> + }
>>> +
>>> + /* Split failed? */
>>> + for (i = 0; i < nr_thps; i++) {
>>> + if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
>>
>> page_area + i * pagesize, like Wei pointed out in another email.
>>
>>> + continue;
>>> + ksft_test_result_fail("THP %zu not split\n", i);
>>> + }
>>> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
>>> - munmap(one_page, len);
>>> +out:
>>> + munmap(thp_area, thp_area_size);
>>> + if (page_area)
>>> + munmap(page_area, page_area_size);
>>> }
>>> static void split_file_backed_thp(int order)
>>> --
>>> 2.50.1
>>
>> Otherwise, LGTM. With all the changes in this email and other email,
>> feel free to add Reviewed-by: Zi Yan <ziy@nvidia.com> when you send it
>> out formally.
>
> Thanks!
>
> I'm currently chasing why I keep getting temporary
>
> Bail out! Some THPs are missing during mremap
>
> Already on the old test. Something doesn't work quite right as it seems.
Did you also see “Failed to get folio info”?
If pageflags_get() cannot read kpageflags content, is_backed_by_folio()
would not be able to detect a THP.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 15:28 ` David Hildenbrand
2025-09-02 15:39 ` Zi Yan
@ 2025-09-02 15:40 ` David Hildenbrand
2025-09-02 15:42 ` Zi Yan
1 sibling, 1 reply; 23+ messages in thread
From: David Hildenbrand @ 2025-09-02 15:40 UTC (permalink / raw)
To: Zi Yan
Cc: Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On 02.09.25 17:28, David Hildenbrand wrote:
> [...]
>
>>> @@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
>>> static void split_pte_mapped_thp(void)
>>> {
>>> - char *one_page, *pte_mapped, *pte_mapped2;
>>> - size_t len = 4 * pmd_pagesize;
>>> - uint64_t thp_size;
>>> + const size_t nr_thps = 4;
>>> + const size_t thp_area_size = nr_thps * pmd_pagesize;
>>> + const size_t page_area_size = nr_thps * pagesize;
>>> + char *thp_area, *page_area = NULL, *tmp;
>>> size_t i;
>>> - one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>>> + thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
>>> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>> - if (one_page == MAP_FAILED)
>>> - ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>>> + if (thp_area == MAP_FAILED) {
>>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>>> + goto out;
>>
>> thp_area mmap failed and out label will try to munmap MAP_FAILED, which is
>> (void *) -1. munmap will fail with -EINVAL.
>
> Indeed, should just be a "return;"
>
>>
>>> + }
>>> - madvise(one_page, len, MADV_HUGEPAGE);
>>> + madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>>> - for (i = 0; i < len; i++)
>>> - one_page[i] = (char)i;
>>> + for (i = 0; i < thp_area_size; i++)
>>> + thp_area[i] = (char)i;
>>> - if (!check_huge_anon(one_page, 4, pmd_pagesize))
>>> - ksft_exit_fail_msg("No THP is allocated\n");
>>> + if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>>> + ksft_test_result_skip("Not all THPs allocated\n");
>>> + goto out;
>>> + }
>>> - /* remap the first pagesize of first THP */
>>> - pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
>>> -
>>> - /* remap the Nth pagesize of Nth THP */
>>> - for (i = 1; i < 4; i++) {
>>> - pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
>>> - pagesize, pagesize,
>>> - MREMAP_MAYMOVE|MREMAP_FIXED,
>>> - pte_mapped + pagesize * i);
>>> - if (pte_mapped2 == MAP_FAILED)
>>> - ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
>>> - }
>>> -
>>> - /* smap does not show THPs after mremap, use kpageflags instead */
>>> - thp_size = 0;
>>> - for (i = 0; i < pagesize * 4; i++)
>>> - if (i % pagesize == 0 &&
>>> - is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
>>> - thp_size++;
>>> -
>>> - if (thp_size != 4)
>>> - ksft_exit_fail_msg("Some THPs are missing during mremap\n");
>>> -
>>> - /* split all remapped THPs */
>>> - write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
>>> - (uint64_t)pte_mapped + pagesize * 4, 0);
>>> -
>>> - /* smap does not show THPs after mremap, use kpageflags instead */
>>> - thp_size = 0;
>>> - for (i = 0; i < pagesize * 4; i++) {
>>> - if (pte_mapped[i] != (char)i)
>>> - ksft_exit_fail_msg("%ld byte corrupted\n", i);
>>> + /*
>>> + * To challenge spitting code, we will mremap page[x] of the
>>> + * thp[x] into a smaller area, and trigger the split from that
>>> + * smaller area. This will end up replacing the PMD mappings in
>>> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>>> + */
>>> + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>>> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>> + if (page_area == MAP_FAILED) {
>>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>>> + goto out;
>>> + }
>>> - if (i % pagesize == 0 &&
>>> - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>>> - thp_size++;
>>> + for (i = 0; i < nr_thps; i++) {
>>> + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>>> + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>>> + page_area + pagesize * i);
>>> + if (tmp != MAP_FAILED)
>>> + continue;
>>> + ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
>>> + goto out;
>>> + }
>>> +
>>> + /*
>>> + * Verify that our THPs were not split yet. Note that
>>> + * check_huge_anon() cannot be used as it checks for PMD mappings.
>>> + */
>>> + for (i = 0; i < nr_thps; i++) {
>>> + if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
>>> + pagemap_fd, kpageflags_fd))
>>> + continue;
>>> + ksft_test_result_fail("THP %zu missing after mremap\n", i);
>>> + goto out;
>>> }
>>> - if (thp_size)
>>> - ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
>>> + /* Split all THPs through the remapped pages. */
>>> + write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
>>> + (uint64_t)page_area + page_area_size, 0);
>>> +
>>> + /* Corruption during mremap or split? */
>>> + for (i = 0; i < page_area_size; i++) {
>>> + if (page_area[i] == (char)i)
>>> + continue;
>>> + ksft_test_result_fail("%zu byte corrupted\n", i);
>>> + goto out;
>>> + }
>>> +
>>> + /* Split failed? */
>>> + for (i = 0; i < nr_thps; i++) {
>>> + if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
>>
>> page_area + i * pagesize, like Wei pointed out in another email.
>>
>>> + continue;
>>> + ksft_test_result_fail("THP %zu not split\n", i);
>>> + }
>>> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
>>> - munmap(one_page, len);
>>> +out:
>>> + munmap(thp_area, thp_area_size);
>>> + if (page_area)
>>> + munmap(page_area, page_area_size);
>>> }
>>> static void split_file_backed_thp(int order)
>>> --
>>> 2.50.1
>>
>> Otherwise, LGTM. With all the changes in this email and other email,
>> feel free to add Reviewed-by: Zi Yan <ziy@nvidia.com> when you send it
>> out formally.
>
> Thanks!
>
> I'm currently chasing why I keep getting temporary
>
> Bail out! Some THPs are missing during mremap
>
> Already on the old test. Something doesn't work quite right as it seems.
>
Ah, I think I know why:
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index c94fd64066ef6..4051a5c98a97c 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -95,7 +95,7 @@ static bool is_backed_by_folio(char *vaddr, int order, int pagemap_fd,
return true;
/* this folio is bigger than the given order */
- if (pfn_flags & (KPF_THP | KPF_COMPOUND_TAIL))
+ if (pfn_flags & (KPF_THP | KPF_COMPOUND_TAIL) == KPF_THP | KPF_COMPOUND_TAIL)
return false;
return true;
Let me test with that.
--
Cheers
David / dhildenb
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Patch v2] selftests/mm: check content to see whether mremap corrupt data
2025-09-02 15:40 ` David Hildenbrand
@ 2025-09-02 15:42 ` Zi Yan
0 siblings, 0 replies; 23+ messages in thread
From: Zi Yan @ 2025-09-02 15:42 UTC (permalink / raw)
To: David Hildenbrand
Cc: Wei Yang, akpm, lorenzo.stoakes, baolin.wang, linux-mm,
linux-kselftest
On 2 Sep 2025, at 11:40, David Hildenbrand wrote:
> On 02.09.25 17:28, David Hildenbrand wrote:
>> [...]
>>
>>>> @@ -390,67 +390,88 @@ static void split_pmd_thp_to_order(int order)
>>>> static void split_pte_mapped_thp(void)
>>>> {
>>>> - char *one_page, *pte_mapped, *pte_mapped2;
>>>> - size_t len = 4 * pmd_pagesize;
>>>> - uint64_t thp_size;
>>>> + const size_t nr_thps = 4;
>>>> + const size_t thp_area_size = nr_thps * pmd_pagesize;
>>>> + const size_t page_area_size = nr_thps * pagesize;
>>>> + char *thp_area, *page_area = NULL, *tmp;
>>>> size_t i;
>>>> - one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
>>>> + thp_area = mmap((void *)(1UL << 30), thp_area_size, PROT_READ | PROT_WRITE,
>>>> MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>>> - if (one_page == MAP_FAILED)
>>>> - ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));
>>>> + if (thp_area == MAP_FAILED) {
>>>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>>>> + goto out;
>>>
>>> thp_area mmap failed and out label will try to munmap MAP_FAILED, which is
>>> (void *) -1. munmap will fail with -EINVAL.
>>
>> Indeed, should just be a "return;"
>>
>>>
>>>> + }
>>>> - madvise(one_page, len, MADV_HUGEPAGE);
>>>> + madvise(thp_area, thp_area_size, MADV_HUGEPAGE);
>>>> - for (i = 0; i < len; i++)
>>>> - one_page[i] = (char)i;
>>>> + for (i = 0; i < thp_area_size; i++)
>>>> + thp_area[i] = (char)i;
>>>> - if (!check_huge_anon(one_page, 4, pmd_pagesize))
>>>> - ksft_exit_fail_msg("No THP is allocated\n");
>>>> + if (!check_huge_anon(thp_area, nr_thps, pmd_pagesize)) {
>>>> + ksft_test_result_skip("Not all THPs allocated\n");
>>>> + goto out;
>>>> + }
>>>> - /* remap the first pagesize of first THP */
>>>> - pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
>>>> -
>>>> - /* remap the Nth pagesize of Nth THP */
>>>> - for (i = 1; i < 4; i++) {
>>>> - pte_mapped2 = mremap(one_page + pmd_pagesize * i + pagesize * i,
>>>> - pagesize, pagesize,
>>>> - MREMAP_MAYMOVE|MREMAP_FIXED,
>>>> - pte_mapped + pagesize * i);
>>>> - if (pte_mapped2 == MAP_FAILED)
>>>> - ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
>>>> - }
>>>> -
>>>> - /* smap does not show THPs after mremap, use kpageflags instead */
>>>> - thp_size = 0;
>>>> - for (i = 0; i < pagesize * 4; i++)
>>>> - if (i % pagesize == 0 &&
>>>> - is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd))
>>>> - thp_size++;
>>>> -
>>>> - if (thp_size != 4)
>>>> - ksft_exit_fail_msg("Some THPs are missing during mremap\n");
>>>> -
>>>> - /* split all remapped THPs */
>>>> - write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
>>>> - (uint64_t)pte_mapped + pagesize * 4, 0);
>>>> -
>>>> - /* smap does not show THPs after mremap, use kpageflags instead */
>>>> - thp_size = 0;
>>>> - for (i = 0; i < pagesize * 4; i++) {
>>>> - if (pte_mapped[i] != (char)i)
>>>> - ksft_exit_fail_msg("%ld byte corrupted\n", i);
>>>> + /*
>>>> + * To challenge spitting code, we will mremap page[x] of the
>>>> + * thp[x] into a smaller area, and trigger the split from that
>>>> + * smaller area. This will end up replacing the PMD mappings in
>>>> + * the thp_area by PTE mappings first, leaving the THPs unsplit.
>>>> + */
>>>> + page_area = mmap(NULL, page_area_size, PROT_READ | PROT_WRITE,
>>>> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>>> + if (page_area == MAP_FAILED) {
>>>> + ksft_test_result_fail("Fail to allocate memory: %s\n", strerror(errno));
>>>> + goto out;
>>>> + }
>>>> - if (i % pagesize == 0 &&
>>>> - !is_backed_by_folio(&pte_mapped[i], 0, pagemap_fd, kpageflags_fd))
>>>> - thp_size++;
>>>> + for (i = 0; i < nr_thps; i++) {
>>>> + tmp = mremap(thp_area + pmd_pagesize * i + pagesize * i,
>>>> + pagesize, pagesize, MREMAP_MAYMOVE|MREMAP_FIXED,
>>>> + page_area + pagesize * i);
>>>> + if (tmp != MAP_FAILED)
>>>> + continue;
>>>> + ksft_test_result_fail("mremap failed: %s\n", strerror(errno));
>>>> + goto out;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Verify that our THPs were not split yet. Note that
>>>> + * check_huge_anon() cannot be used as it checks for PMD mappings.
>>>> + */
>>>> + for (i = 0; i < nr_thps; i++) {
>>>> + if (is_backed_by_folio(page_area + i * pagesize, pmd_order,
>>>> + pagemap_fd, kpageflags_fd))
>>>> + continue;
>>>> + ksft_test_result_fail("THP %zu missing after mremap\n", i);
>>>> + goto out;
>>>> }
>>>> - if (thp_size)
>>>> - ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);
>>>> + /* Split all THPs through the remapped pages. */
>>>> + write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
>>>> + (uint64_t)page_area + page_area_size, 0);
>>>> +
>>>> + /* Corruption during mremap or split? */
>>>> + for (i = 0; i < page_area_size; i++) {
>>>> + if (page_area[i] == (char)i)
>>>> + continue;
>>>> + ksft_test_result_fail("%zu byte corrupted\n", i);
>>>> + goto out;
>>>> + }
>>>> +
>>>> + /* Split failed? */
>>>> + for (i = 0; i < nr_thps; i++) {
>>>> + if (is_backed_by_folio(&page_area[i], 0, pagemap_fd, kpageflags_fd))
>>>
>>> page_area + i * pagesize, like Wei pointed out in another email.
>>>
>>>> + continue;
>>>> + ksft_test_result_fail("THP %zu not split\n", i);
>>>> + }
>>>> ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
>>>> - munmap(one_page, len);
>>>> +out:
>>>> + munmap(thp_area, thp_area_size);
>>>> + if (page_area)
>>>> + munmap(page_area, page_area_size);
>>>> }
>>>> static void split_file_backed_thp(int order)
>>>> --
>>>> 2.50.1
>>>
>>> Otherwise, LGTM. With all the changes in this email and other email,
>>> feel free to add Reviewed-by: Zi Yan <ziy@nvidia.com> when you send it
>>> out formally.
>>
>> Thanks!
>>
>> I'm currently chasing why I keep getting temporary
>>
>> Bail out! Some THPs are missing during mremap
>>
>> Already on the old test. Something doesn't work quite right as it seems.
>>
>
> Ah, I think I know why:
>
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index c94fd64066ef6..4051a5c98a97c 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -95,7 +95,7 @@ static bool is_backed_by_folio(char *vaddr, int order, int pagemap_fd,
> return true;
> /* this folio is bigger than the given order */
> - if (pfn_flags & (KPF_THP | KPF_COMPOUND_TAIL))
> + if (pfn_flags & (KPF_THP | KPF_COMPOUND_TAIL) == KPF_THP | KPF_COMPOUND_TAIL)
> return false;
> return true;
>
> Let me test with that.
Oh, so if next page is a THP head, this will report a false negative result.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 23+ messages in thread