On Sun, Feb 01, 2026 at 09:20:35AM -0500, Zi Yan wrote:

On 1 Feb 2026, at 8:04, Gavin Guo wrote:

quoted

On 2/1/26 11:39, Zi Yan wrote:

quoted

On 31 Jan 2026, at 21:09, Wei Yang wrote:

quoted

On Fri, Jan 30, 2026 at 09:44:10PM -0500, Zi Yan wrote:

quoted

On 30 Jan 2026, at 18:00, Wei Yang wrote:

quoted

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.

quoted

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 <redacted>
Fixes: 60fbb14396d5 ("mm/huge_memory: adjust try_to_migrate_one() and split_huge_pmd_locked()")
Cc: Gavin Guo <redacted>
Cc: "David Hildenbrand (Red Hat)" <david@kernel.org>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: <redacted>
---
  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.

quoted

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.

-- 
Wei Yang
Help you, Help me