The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com --- tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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)) + for (i = 0; i < pagesize * 4; i += pagesize) + if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;
if (thp_size != 4)
On Sat, 30 Aug 2025 02:31:02 +0000 Wei Yang richard.weiyang@gmail.com wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Why?
--- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void) /* 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))
- for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;
Looks like we're doing more work. Is there something wrong with the existing code?
On Fri, Aug 29, 2025 at 08:09:53PM -0700, Andrew Morton wrote:
On Sat, 30 Aug 2025 02:31:02 +0000 Wei Yang richard.weiyang@gmail.com wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Why?
--- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void) /* 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))
- for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;Looks like we're doing more work. Is there something wrong with the existing code?
Excuse me if I misunderstand the code.
Originally, i iterate from 0 to pagesize * 4 one by one and call is_backed_by_folio() when i is 0, 4096, 8192, 12288.
The change makes i just iterate on 0, 4096, 8192, 12288 and call is_backed_by_folio() respectively.
Current code is not wrong, but not necessary to iterate one by one.
On 29 Aug 2025, at 22:31, Wei Yang wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com
tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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))
for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;if (thp_size != 4)
It might be better to add
if (pte_mapped[i] != (char)i) ksft_exit_fail_msg("%ld byte corrupted\n", i);
instead to make sure mremap() does not change pte_mapped[] values.
-- Best Regards, Yan, Zi
On Sat, Aug 30, 2025 at 09:32:58PM -0400, Zi Yan wrote:
On 29 Aug 2025, at 22:31, Wei Yang wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com
tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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))
for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;if (thp_size != 4)
It might be better to add
if (pte_mapped[i] != (char)i) ksft_exit_fail_msg("%ld byte corrupted\n", i);
instead to make sure mremap() does not change pte_mapped[] values.
Thanks, will use this.
-- Best Regards, Yan, Zi
On 31.08.25 03:32, Zi Yan wrote:
On 29 Aug 2025, at 22:31, Wei Yang wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com
tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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))
for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;if (thp_size != 4)
It might be better to add
if (pte_mapped[i] != (char)i) ksft_exit_fail_msg("%ld byte corrupted\n", i);
instead to make sure mremap() does not change pte_mapped[] values.
We do have a corruption check later in that function, so I think we can just keep it simple here.
So this as is LGTM
Acked-by: David Hildenbrand david@redhat.com
As noted, we should just move from mremap() to mprotect() or sth like that which has clearer semantics.
On Mon, Sep 01, 2025 at 11:32:11AM +0200, David Hildenbrand wrote:
On 31.08.25 03:32, Zi Yan wrote:
On 29 Aug 2025, at 22:31, Wei Yang wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com
tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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))
for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;if (thp_size != 4)
It might be better to add
if (pte_mapped[i] != (char)i) ksft_exit_fail_msg("%ld byte corrupted\n", i);
instead to make sure mremap() does not change pte_mapped[] values.
We do have a corruption check later in that function, so I think we can just keep it simple here.
So this as is LGTM
Acked-by: David Hildenbrand david@redhat.com
As noted, we should just move from mremap() to mprotect() or sth like that which has clearer semantics.
If my understanding is correct, we should
mmap 4 PMD_SIZE region with RW madvise and fault in to allocate pmd-mapped thp mprotect first page of each PMD to read-only to split to pte-mapped thp check whether the page is backed by pmd-order folio
Is this the correct way?
On 01.09.25 14:51, Wei Yang wrote:
On Mon, Sep 01, 2025 at 11:32:11AM +0200, David Hildenbrand wrote:
On 31.08.25 03:32, Zi Yan wrote:
On 29 Aug 2025, at 22:31, Wei Yang wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com
tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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))
for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++;if (thp_size != 4)
It might be better to add
if (pte_mapped[i] != (char)i) ksft_exit_fail_msg("%ld byte corrupted\n", i);
instead to make sure mremap() does not change pte_mapped[] values.
We do have a corruption check later in that function, so I think we can just keep it simple here.
So this as is LGTM
Acked-by: David Hildenbrand david@redhat.com
As noted, we should just move from mremap() to mprotect() or sth like that which has clearer semantics.
If my understanding is correct, we should
mmap 4 PMD_SIZE region with RW madvise and fault in to allocate pmd-mapped thp mprotect first page of each PMD to read-only to split to pte-mapped thp check whether the page is backed by pmd-order folioIs this the correct way?
Yeah, I would just mprotect(PROT_READ) the first page of each PMD. That will trigger a PTE-mapping of the THP reliably.
On 01.09.25 14:54, David Hildenbrand wrote:
On 01.09.25 14:51, Wei Yang wrote:
On Mon, Sep 01, 2025 at 11:32:11AM +0200, David Hildenbrand wrote:
On 31.08.25 03:32, Zi Yan wrote:
On 29 Aug 2025, at 22:31, Wei Yang wrote:
The check of is_backed_by_folio() is done on each page.
Directly move pointer to next page instead of increase one and check if it is page size aligned.
Signed-off-by: Wei Yang richard.weiyang@gmail.com
tools/testing/selftests/mm/split_huge_page_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 10ae65ea032f..7f7016ba4054 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -423,9 +423,8 @@ static void split_pte_mapped_thp(void)
/* 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))
for (i = 0; i < pagesize * 4; i += pagesize)
if (is_backed_by_folio(&pte_mapped[i], pmd_order, pagemap_fd, kpageflags_fd)) thp_size++; if (thp_size != 4)It might be better to add
if (pte_mapped[i] != (char)i) ksft_exit_fail_msg("%ld byte corrupted\n", i);
instead to make sure mremap() does not change pte_mapped[] values.
We do have a corruption check later in that function, so I think we can just keep it simple here.
So this as is LGTM
Acked-by: David Hildenbrand david@redhat.com
As noted, we should just move from mremap() to mprotect() or sth like that which has clearer semantics.
If my understanding is correct, we should
mmap 4 PMD_SIZE region with RW madvise and fault in to allocate pmd-mapped thp mprotect first page of each PMD to read-only to split to pte-mapped thp check whether the page is backed by pmd-order folioIs this the correct way?
Yeah, I would just mprotect(PROT_READ) the first page of each PMD. That will trigger a PTE-mapping of the THP reliably.
Zi Yan mentions that there is a reason we are using mremap in the other thread. So likely best to just keep it as is unless we can understand that it definitely can done simpler.
linux-kselftest-mirror@lists.linaro.org
-
Andrew Morton -
David Hildenbrand -
Wei Yang -
Zi Yan