Re: [Patch v2] mm/huge_memory: fix early failure try_to_migrate() when split huge pmd for shared thp

[Zi Yan <ziy@nvidia.com>]

On 4 Feb 2026, at 14:36, David Hildenbrand (arm) wrote:

> Sorry for the late reply. I saw that I was CCed in v1 but I am only now catching up with mails ... slowly but steadily.
>
>> Without the above commit, we can successfully split to order 0.
>> With the above commit, the folio is still a large folio.
>>
>> The reason is the above commit return false after split pmd
>> unconditionally in the first process and break try_to_migrate().
>>
>> The tricky thing in above reproduce method is current debugfs interface
>> leverage function split_huge_pages_pid(), which will iterate the whole
>> pmd range and do folio split on each base page address. This means it
>> will try 512 times, and each time split one pmd from pmd mapped to pte
>> mapped thp. If there are less than 512 shared mapped process,
>> the folio is still split successfully at last. But in real world, we
>> usually try it for once.
>
> Ah, that explains magic number 513.
>
>>
>> This patch fixes this by restart page_vma_mapped_walk() after
>> split_huge_pmd_locked(). Because split_huge_pmd_locked() may fall back to
>> (freeze = false) if folio_try_share_anon_rmap_pmd() fails and the PMD is
>> just split instead of split to migration entry.
>
> Right, but folio_try_share_anon_rmap_pmd() should never fail on the folios that have already been shared? (above you write that it is shared with 512 children)
>
> The only case where  folio_try_share_anon_rmap_pmd() could fail would be if the folio would not be shared, and there would only be a single PMD then, so there is nothing you can do -> abort.
>
> Returning "false" from try_to_migrate_one() is the real issue, as it makes rmap_walk_anon() to just stop -> abort the walk.
>
>
> So I suspect v1 was actually sufficient, or what am I missing where the restart would actually be required?

The explanation is not for the shared case mentioned above. It is for unshared
folio. If an unshared folio’s PAE cannot be cleared, try_to_migrate_one() return
true, indicating a success. Yeah, since it is an unshared folio, the return
value of try_to_migrate_one() does not matter. This fix makes try_to_migrate_one()
return false.

>
>
> (maybe we should get rid of the usage of booleans here at some point, an enum like abort/continue would have been much clearer)
>
>> Restart
>> page_vma_mapped_walk() and let try_to_migrate_one() try on each PTE
>> again and fail try_to_migrate() early if it fails.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Fixes: 60fbb14396d5 ("mm/huge_memory: adjust try_to_migrate_one() and split_huge_pmd_locked()")
>> Cc: Gavin Guo <gavinguo@igalia.com>
>> Cc: "David Hildenbrand (Red Hat)" <david@kernel.org>
>> Cc: Zi Yan <ziy@nvidia.com>
>> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Cc: Lance Yang <lance.yang@linux.dev>
>> Cc: <stable@vger.kernel.org>
>>
>> ---
>> v2:
>>    * restart page_vma_mapped_walk() after split_huge_pmd_locked()
>> ---
>>   mm/rmap.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 618df3385c8b..5b853ec8901d 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -2446,11 +2446,16 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
>>   			__maybe_unused pmd_t pmdval;
>>    			if (flags & TTU_SPLIT_HUGE_PMD) {
>> +				/*
>> +				 * After split_huge_pmd_locked(), restart the
>> +				 * walk to detect PageAnonExclusive handling
>> +				 * failure in __split_huge_pmd_locked().
>> +				 */
>>   				split_huge_pmd_locked(vma, pvmw.address,
>>   						      pvmw.pmd, true);
>> -				ret = false;
>> -				page_vma_mapped_walk_done(&pvmw);
>> -				break;
>> +				flags &= ~TTU_SPLIT_HUGE_PMD;
>> +				page_vma_mapped_walk_restart(&pvmw);
>> +				continue;
>>   			}
>
> The change looks more consistent to what we have in try_to_unmap().
>
> But the explanation above is not quite right I think. And consequently the comment above as well.
>
> PAE being set implies "single PMD" -> unshared.

The commit message might be improved with some additional context. The comment
above pairs with the comment in __split_huge_pmd_locked()
“In case we cannot clear PageAnonExclusive(), split the PMD
only and let try_to_migrate_one() fail later”. What is problem with it?

Best Regards,
Yan, Zi