[PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Wei Yang]

SCAN_PMD_NONE means current pmd is empty, but we can still continue
collapse next pmd range.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/khugepaged.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 7e8cb181d5bd..4c2217076ad3 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
 			goto handle_result;
 		/* Whitelisted set of results where continuing OK */
 		case SCAN_PMD_NULL:
+		case SCAN_PMD_NONE:
 		case SCAN_PTE_NON_PRESENT:
 		case SCAN_PTE_UFFD_WP:
 		case SCAN_LACK_REFERENCED_PAGE:
-- 
2.34.1

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Lance Yang]

On 2025/11/12 10:00, Wei Yang wrote:
> SCAN_PMD_NONE means current pmd is empty, but we can still continue
> collapse next pmd range.

Right, bailing out of the whole MADV_COLLAPSE request just because
we encounter one empty PMD is too strict ...

It makes sense to treat SCAN_PMD_NONE like the other whitelisted cases
(e.g., SCAN_PMD_NULL).
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---

LGTM.

Reviewed-by: Lance Yang <lance.yang@linux.dev>

>   mm/khugepaged.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 7e8cb181d5bd..4c2217076ad3 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>   			goto handle_result;
>   		/* Whitelisted set of results where continuing OK */
>   		case SCAN_PMD_NULL:
> +		case SCAN_PMD_NONE:
>   		case SCAN_PTE_NON_PRESENT:
>   		case SCAN_PTE_UFFD_WP:
>   		case SCAN_LACK_REFERENCED_PAGE:

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Wei Yang]

On Wed, Nov 12, 2025 at 10:37:06AM +0800, Lance Yang wrote:
>
>
>On 2025/11/12 10:00, Wei Yang wrote:
>> SCAN_PMD_NONE means current pmd is empty, but we can still continue
>> collapse next pmd range.
>
>Right, bailing out of the whole MADV_COLLAPSE request just because
>we encounter one empty PMD is too strict ...
>
>It makes sense to treat SCAN_PMD_NONE like the other whitelisted cases
>(e.g., SCAN_PMD_NULL).
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>
>LGTM.
>
>Reviewed-by: Lance Yang <lance.yang@linux.dev>
>

Thanks for the review.

I'm currently evaluating the use of SCAN_NULL versus SCAN_NONE. It seems their
usage in the current code is intermixed, and I'm questioning if there's a
strong, practical reason to maintain a distinction between these two states.
Should they be unified?


-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Dev Jain]

On 12/11/25 9:10 am, Wei Yang wrote:
> On Wed, Nov 12, 2025 at 10:37:06AM +0800, Lance Yang wrote:
>>
>> On 2025/11/12 10:00, Wei Yang wrote:
>>> SCAN_PMD_NONE means current pmd is empty, but we can still continue
>>> collapse next pmd range.
>> Right, bailing out of the whole MADV_COLLAPSE request just because
>> we encounter one empty PMD is too strict ...
>>
>> It makes sense to treat SCAN_PMD_NONE like the other whitelisted cases
>> (e.g., SCAN_PMD_NULL).
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> ---
>> LGTM.
>>
>> Reviewed-by: Lance Yang <lance.yang@linux.dev>
>>
> Thanks for the review.
>
> I'm currently evaluating the use of SCAN_NULL versus SCAN_NONE. It seems their
> usage in the current code is intermixed, and I'm questioning if there's a
> strong, practical reason to maintain a distinction between these two states.
> Should they be unified?

There was some discussion about that in this thread:

https://lore.kernel.org/all/v5ivpub6z2n2uyemlnxgbilzs52ep4lrary7lm7o6axxoneb75@yfacfl5rkzeh/

There are some cleanups mentioned in the thread which you can try if you want :)

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Wei Yang]

On Wed, Nov 12, 2025 at 09:28:34AM +0530, Dev Jain wrote:
>
>On 12/11/25 9:10 am, Wei Yang wrote:
>> On Wed, Nov 12, 2025 at 10:37:06AM +0800, Lance Yang wrote:
>> > 
>> > On 2025/11/12 10:00, Wei Yang wrote:
>> > > SCAN_PMD_NONE means current pmd is empty, but we can still continue
>> > > collapse next pmd range.
>> > Right, bailing out of the whole MADV_COLLAPSE request just because
>> > we encounter one empty PMD is too strict ...
>> > 
>> > It makes sense to treat SCAN_PMD_NONE like the other whitelisted cases
>> > (e.g., SCAN_PMD_NULL).
>> > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> > > ---
>> > LGTM.
>> > 
>> > Reviewed-by: Lance Yang <lance.yang@linux.dev>
>> > 
>> Thanks for the review.
>> 
>> I'm currently evaluating the use of SCAN_NULL versus SCAN_NONE. It seems their
>> usage in the current code is intermixed, and I'm questioning if there's a
>> strong, practical reason to maintain a distinction between these two states.
>> Should they be unified?
>
>There was some discussion about that in this thread:
>
>https://lore.kernel.org/all/v5ivpub6z2n2uyemlnxgbilzs52ep4lrary7lm7o6axxoneb75@yfacfl5rkzeh/
>
>There are some cleanups mentioned in the thread which you can try if you want :)
>

Thanks, will take a look.

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Dev Jain]

On 12/11/25 7:30 am, Wei Yang wrote:
> SCAN_PMD_NONE means current pmd is empty, but we can still continue
> collapse next pmd range.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>   

LGTM

Reviewed-by: Dev Jain <dev.jain@arm.com>

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[David Hildenbrand (Red Hat)]

On 12.11.25 03:00, Wei Yang wrote:
> SCAN_PMD_NONE means current pmd is empty, but we can still continue
> collapse next pmd range.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>   mm/khugepaged.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 7e8cb181d5bd..4c2217076ad3 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>   			goto handle_result;
>   		/* Whitelisted set of results where continuing OK */
>   		case SCAN_PMD_NULL:
> +		case SCAN_PMD_NONE:
>   		case SCAN_PTE_NON_PRESENT:
>   		case SCAN_PTE_UFFD_WP:
>   		case SCAN_LACK_REFERENCED_PAGE:

Looks ok to me.

I am really confused about PMD_NONE vs. PMD_NULL.

Is my understanding correct, hat with your patch we can just merge both 
into a single NO_PMD_TABLE or sth. like that?

-- 
Cheers

David

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[David Hildenbrand (Red Hat)]

On 12.11.25 10:51, David Hildenbrand (Red Hat) wrote:
> On 12.11.25 03:00, Wei Yang wrote:
>> SCAN_PMD_NONE means current pmd is empty, but we can still continue
>> collapse next pmd range.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>    mm/khugepaged.c | 1 +
>>    1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 7e8cb181d5bd..4c2217076ad3 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>>    			goto handle_result;
>>    		/* Whitelisted set of results where continuing OK */
>>    		case SCAN_PMD_NULL:
>> +		case SCAN_PMD_NONE:
>>    		case SCAN_PTE_NON_PRESENT:
>>    		case SCAN_PTE_UFFD_WP:
>>    		case SCAN_LACK_REFERENCED_PAGE:
> 
> Looks ok to me.
> 
> I am really confused about PMD_NONE vs. PMD_NULL.
> 
> Is my understanding correct, hat with your patch we can just merge both
> into a single NO_PMD_TABLE or sth. like that?

NO_PTE_TABLE, actually

-- 
Cheers

David

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Wei Yang]

On Wed, Nov 12, 2025 at 10:51:43AM +0100, David Hildenbrand (Red Hat) wrote:
>On 12.11.25 10:51, David Hildenbrand (Red Hat) wrote:
>> On 12.11.25 03:00, Wei Yang wrote:
>> > SCAN_PMD_NONE means current pmd is empty, but we can still continue
>> > collapse next pmd range.
>> > 
>> > Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> > ---
>> >    mm/khugepaged.c | 1 +
>> >    1 file changed, 1 insertion(+)
>> > 
>> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> > index 7e8cb181d5bd..4c2217076ad3 100644
>> > --- a/mm/khugepaged.c
>> > +++ b/mm/khugepaged.c
>> > @@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>> >    			goto handle_result;
>> >    		/* Whitelisted set of results where continuing OK */
>> >    		case SCAN_PMD_NULL:
>> > +		case SCAN_PMD_NONE:
>> >    		case SCAN_PTE_NON_PRESENT:
>> >    		case SCAN_PTE_UFFD_WP:
>> >    		case SCAN_LACK_REFERENCED_PAGE:
>> 
>> Looks ok to me.
>> 
>> I am really confused about PMD_NONE vs. PMD_NULL.
>> 
>> Is my understanding correct, hat with your patch we can just merge both
>> into a single NO_PMD_TABLE or sth. like that?
>
>NO_PTE_TABLE, actually
>

I am thinking about this too.

If no other objections, I will prepare a v2 with this.

BTW, you prefer merge them directly, or merge on top of this change?

>-- 
>Cheers
>
>David

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[David Hildenbrand (Red Hat)]

On 12.11.25 12:27, Wei Yang wrote:
> On Wed, Nov 12, 2025 at 10:51:43AM +0100, David Hildenbrand (Red Hat) wrote:
>> On 12.11.25 10:51, David Hildenbrand (Red Hat) wrote:
>>> On 12.11.25 03:00, Wei Yang wrote:
>>>> SCAN_PMD_NONE means current pmd is empty, but we can still continue
>>>> collapse next pmd range.
>>>>
>>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>>> ---
>>>>     mm/khugepaged.c | 1 +
>>>>     1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index 7e8cb181d5bd..4c2217076ad3 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>>>>     			goto handle_result;
>>>>     		/* Whitelisted set of results where continuing OK */
>>>>     		case SCAN_PMD_NULL:
>>>> +		case SCAN_PMD_NONE:
>>>>     		case SCAN_PTE_NON_PRESENT:
>>>>     		case SCAN_PTE_UFFD_WP:
>>>>     		case SCAN_LACK_REFERENCED_PAGE:
>>>
>>> Looks ok to me.
>>>
>>> I am really confused about PMD_NONE vs. PMD_NULL.
>>>
>>> Is my understanding correct, hat with your patch we can just merge both
>>> into a single NO_PMD_TABLE or sth. like that?
>>
>> NO_PTE_TABLE, actually
>>
> 
> I am thinking about this too.
> 
> If no other objections, I will prepare a v2 with this.
> 
> BTW, you prefer merge them directly, or merge on top of this change?

I'd probably do it in a single commit, stating that both scenarios 
should be handled completely the same.

But no strong opinion against doing it in two commits from my side.

-- 
Cheers

David

Re: [PATCH] mm/khugepaged: continue to collapse on SCAN_PMD_NONE

[Baolin Wang]

On 2025/11/12 10:00, Wei Yang wrote:
> SCAN_PMD_NONE means current pmd is empty, but we can still continue
> collapse next pmd range.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---

Make sense to me. And I agree with others' opinions to unify 
SCAN_PMD_NULL and SCAN_PMD_NONE.

Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

>   mm/khugepaged.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 7e8cb181d5bd..4c2217076ad3 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2835,6 +2835,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>   			goto handle_result;
>   		/* Whitelisted set of results where continuing OK */
>   		case SCAN_PMD_NULL:
> +		case SCAN_PMD_NONE:
>   		case SCAN_PTE_NON_PRESENT:
>   		case SCAN_PTE_UFFD_WP:
>   		case SCAN_LACK_REFERENCED_PAGE: