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

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Jan 30, 2026 at 09:44:10PM -0500, Zi Yan wrote:
>On 30 Jan 2026, at 18:00, Wei Yang wrote:
>
>> Commit 60fbb14396d5 ("mm/huge_memory: adjust try_to_migrate_one() and
>> split_huge_pmd_locked()") return false unconditionally after
>> split_huge_pmd_locked() which may fail early during try_to_migrate() for
>> shared thp. This will lead to unexpected folio split failure.
>>
>> One way to reproduce:
>>
>>     Create an anonymous thp range and fork 512 children, so we have a
>>     thp shared mapped in 513 processes. Then trigger folio split with
>>     /sys/kernel/debug/split_huge_pages debugfs to split the thp folio to
>>     order 0.
>>
>> 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 reasoning looks good to me.
>
>>
>> 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.
>>
>> This patch fixes this by removing the unconditional false return after
>> split_huge_pmd_locked(). Later, we may introduce a true fail early if
>> split_huge_pmd_locked() does fail.
>>
>> 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: <stable@vger.kernel.org>
>> ---
>>  mm/rmap.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 618df3385c8b..eed971568d65 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -2448,7 +2448,6 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
>>  			if (flags & TTU_SPLIT_HUGE_PMD) {
>>  				split_huge_pmd_locked(vma, pvmw.address,
>>  						      pvmw.pmd, true);
>> -				ret = false;
>>  				page_vma_mapped_walk_done(&pvmw);
>>  				break;
>>  			}
>
>How about the patch below? It matches the pattern of set_pmd_migration_entry() below.
>Basically, continue if the operation is successful, break otherwise.
>
>diff --git a/mm/rmap.c b/mm/rmap.c
>index 618df3385c8b..83cc9d98533e 100644
>--- a/mm/rmap.c
>+++ b/mm/rmap.c
>@@ -2448,9 +2448,7 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
> 			if (flags & TTU_SPLIT_HUGE_PMD) {
> 				split_huge_pmd_locked(vma, pvmw.address,
> 						      pvmw.pmd, true);
>-				ret = false;
>-				page_vma_mapped_walk_done(&pvmw);
>-				break;
>+				continue;
> 			}

Per my understanding if @freeze is trur, split_huge_pmd_locked() may "fail" as
the comment says:

		 * Without "freeze", we'll simply split the PMD, propagating the
		 * PageAnonExclusive() flag for each PTE by setting it for
		 * each subpage -- no need to (temporarily) clear.
		 *
		 * With "freeze" we want to replace mapped pages by
		 * migration entries right away. This is only possible if we
		 * managed to clear PageAnonExclusive() -- see
		 * set_pmd_migration_entry().
		 *
		 * In case we cannot clear PageAnonExclusive(), split the PMD
		 * only and let try_to_migrate_one() fail later.

While currently we don't return the status of split_huge_pmd_locked() to
indicate whether it does replaced PMD with migration entries successfully. So
we are not sure this operation succeed.

Another difference from set_pmd_migration_entry() is split_huge_pmd_locked()
would change the page table from PMD mapped to PTE mapped.
page_vma_mapped_walk() can handle it now for (pvmw->pmd && !pvmw->pte), but I
am not sure this is what we expected. For example, in try_to_unmap_one(), we
use page_vma_mapped_walk_restart() after pmd splitted.

So I prefer just remove the "ret = false" for a fix. Not sure this is
reasonable to you.

I am thinking two things after this fix:

  * add one similar test in selftests
  * let split_huge_pmd_locked() return value to indicate freeze is degrade to
    !freeze, and fail early on try_to_migrate() like the thp migration branch

Look forward your opinion on whether it worth to do it.

> #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> 			pmdval = pmdp_get(pvmw.pmd);
>
>
>
>--
>Best Regards,
>Yan, Zi

-- 
Wei Yang
Help you, Help me