[PATCH] selftests/mm: directly add pagesize instead of increase until page size

[PATCH] selftests/mm: directly add pagesize instead of increase until page size

[Wei Yang]

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)
-- 
2.34.1

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[Andrew Morton]

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?

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[Wei Yang]

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.

-- 
Wei Yang
Help you, Help me

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[Zi Yan]

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

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[Wei Yang]

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

-- 
Wei Yang
Help you, Help me

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[David Hildenbrand]

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.

-- 
Cheers

David / dhildenb

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[Wei Yang]

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? 

-- 
Wei Yang
Help you, Help me

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[David Hildenbrand]

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 folio
> 
> Is 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.

-- 
Cheers

David / dhildenb

Re: [PATCH] selftests/mm: directly add pagesize instead of increase until page size

[David Hildenbrand]

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 folio
>>
>> Is 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.

-- 
Cheers

David / dhildenb