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

[Zi Yan <ziy@nvidia.com>]

On 3 Feb 2026, at 8:04, Wei Yang wrote:

> On Mon, Feb 02, 2026 at 07:07:12PM -0500, Zi Yan wrote:
>> On 2 Feb 2026, at 19:00, Wei Yang wrote:
>>
>>> On Sun, Feb 01, 2026 at 09:20:35AM -0500, Zi Yan wrote:
>>>> On 1 Feb 2026, at 8:04, Gavin Guo wrote:
>>>>
>>>>> On 2/1/26 11:39, Zi Yan wrote:
>>>>>> On 31 Jan 2026, at 21:09, Wei Yang wrote:
>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> This is the right reasoning. This means to properly handle it, split_huge_pmd_locked()
>>>>>> needs to return whether it inserts migration entries or not when freeze is true.
>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> This is not the right fix, neither was mine above. Because before commit 60fbb14396d5,
>>>>>> the code handles PAE properly. If PAE is cleared, PMD is split into PTEs and each
>>>>>> PTE becomes a migration entry, page_vma_mapped_walk(&pvmw) returns false,
>>>>>> and try_to_migrate_one() returns true. If PAE is not cleared, PMD is split into PTEs
>>>>>> and each PTE is not a migration entry, inside while (page_vma_mapped_walk(&pvmw)),
>>>>>> PAE will be attempted to get cleared again and it will fail again, leading to
>>>>>> try_to_migrate_one() returns false. After commit 60fbb14396d5, no matter PAE is
>>>>>> cleared or not, try_to_migrate_one() always returns false. It causes folio split
>>>>>> failures for shared PMD THPs.
>>>>>>
>>>>>> Now with your fix (and mine above), no matter PAE is cleared or not, try_to_migrate_one()
>>>>>> always returns true. It just flips the code to a different issue. So the proper fix
>>>>>> is to let split_huge_pmd_locked() returns whether it inserts migration entries or not
>>>>>> and do the same pattern as THP migration code path.
>>>>>
>>>>> How about aligning with the try_to_unmap_one()? The behavior would be the same before applying the commit 60fbb14396d5:
>>>>>
>>>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>>>> index 7b9879ef442d..0c96f0883013 100644
>>>>> --- a/mm/rmap.c
>>>>> +++ b/mm/rmap.c
>>>>> @@ -2333,9 +2333,9 @@ 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;
>>>>> +                               flags &= ~TTU_SPLIT_HUGE_PMD;
>>>>> +                               page_vma_mapped_walk_restart(&pvmw);
>>>>> +                               continue;
>>>>>                         }
>>>>>  #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
>>>>>                         pmdval = pmdp_get(pvmw.pmd);
>>>>
>>>> Yes, it works and definitely needs a comment like "After split_huge_pmd_locked(), restart
>>>> the walk to detect PageAnonExclusive handling failure in __split_huge_pmd_locked()".
>>>> The change is good for backporting, but an additional patch to fix it properly by adding
>>>> a return value to split_huge_pmd_locked() is also necessary.
>>>>
>>>
>>> If my understanding is correct, this approach is good for backporting.
>>>
>>> And yes, we could further improve it by return a value to indicate whether
>>> split_huge_pmd_locked() do split to migration entry.
>>>
>>> Thanks both for your thoughtful inputs.
>>
>> Are you going to send two patches in a series, one is the above fix with a comment
>> and the other changes split_huge_pmd_locked() to return a value?
>>
>
> Hmm... as the above fix is supposed to be cc stable and backported, I think
> separate them is the correct process. And for the return value of

Right. Andrew prefer this way. You can send the fix first and send the refactor
patch after the fix is picked up by Andrew.

> split_huge_pmd_locked(), I will take another look at all the call places. Are
> you ok with this?
>
> Well, do you think we need to wait for David's comment? If not, I will prepare
> the v2 fix with the above change.

You answered my question. There should be no issue with existing code.

Thanks.


Best Regards,
Yan, Zi