Re: [RFC PATCH v2 04/11] mm: khugepaged: add shmem mTHP collapse support

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 6/11/26 10:47 AM, Lance Yang wrote:
> 
> On Thu, Jun 11, 2026 at 08:42:28AM +0800, Baolin Wang wrote:
>>
>>
>> On 6/10/26 8:44 PM, Lance Yang wrote:
>>>
>>> On Wed, Jun 10, 2026 at 06:29:12PM +0800, Baolin Wang wrote:
>>> [...]
>>>> @@ -1512,8 +1517,12 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
>>>> 			enum scan_result ret;
>>>>
>>>> 			collapse_address = address + offset * PAGE_SIZE;
>>>> -			ret = collapse_huge_page(mm, collapse_address, referenced,
>>>> -						 unmapped, cc, order);
>>>> +			if (file)
>>>> +				ret = collapse_file(mm, collapse_address, file,
>>>> +						start + offset, cc, order);
>>>> +			else
>>>> +				ret = collapse_huge_page(mm, collapse_address,
>>>> +						referenced, unmapped, cc, order);
>>>>
>>>> 			switch (ret) {
>>>> 			/* Cases where we continue to next collapse candidate */
>>>> @@ -1521,6 +1530,7 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
>>>> 				collapsed += nr_ptes;
>>>> 				fallthrough;
>>>> 			case SCAN_PTE_MAPPED_HUGEPAGE:
>>>
>>> Looks like SCAN_PTE_MAPPED_HUGEPAGE from collapse_file() get lost for
>>> the PMD-order case.
>>>
>>> Previously, collapse_file() returned it straight back to
>>> collapse_single_pmd(), so we would run try_collapse_pte_mapped_thp().
>>>
>>> Now it hits mthp_collapse() fitst, and that case just goes to
>>> next_offset ...
>>
>> This is the expected behavior. SCAN_PTE_MAPPED_HUGEPAGE is only returned
>> when the MADV_COLLAPSE collapse succeeds. In this case, if
>> collapse_file() still returns SCAN_PTE_MAPPED_HUGEPAGE, then
>> mthp_collapse() will ultimately return SCAN_FAIL (because collapsed ==
>> 0), even though the MADV_COLLAPSE collapse has already succeeded.
>>
>> Additionally, the SCAN_PTE_MAPPED_HUGEPAGE logic is already handled in
>> collapse_scan_file(), see:
>>
>> result = mthp_collapse(mm, file, start, addr, 0, 0, cc, enabled_orders);
>> if (result == SCAN_SUCCEED && !cc->is_khugepaged) {
>>          /* If MADV_COLLAPSE, adjust result to call
>> collapse_pte_mapped_thp(). */
>>          result = SCAN_PTE_MAPPED_HUGEPAGE;
>> }
> 
> Right, I agree for that case. The one I meant is a bit different.
> 
> I was looking at this earlier return from collapse_file():
> 
> 		if (is_pmd_order(folio_order(folio))) {
> 			result = SCAN_PTE_MAPPED_HUGEPAGE;
> 			goto out_unlock;
> 		}
> 
> Old code let it bubble back up:
> 
> collapse_single_pmd()
>    -> collapse_scan_file()
>      -> collapse_file()
>        -> SCAN_PTE_MAPPED_HUGEPAGE
>      -> SCAN_PTE_MAPPED_HUGEPAGE
>    -> try_collapse_pte_mapped_thp()
> 
> Not limited to !is_khugepaged. collapse_single_pmd() only checked the
> result, then passed !cc->is_khugepaged as install_pmd.
> 
> Now the same return goes through mthp_collapse() first:
> 
> collapse_single_pmd()
>    -> collapse_scan_file()
>      -> mthp_collapse()
>        -> collapse_file()
>          -> SCAN_PTE_MAPPED_HUGEPAGE
>        -> goto next_offset
> 
> So collapse_single_pmd() never sees SCAN_PTE_MAPPED_HUGEPAGE for this
> case anymore. Hopefully Im not missing something here :)

Yes, you are right. I missed this case. Will fix. Thanks.