Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio

On Wed, Aug 12, 2026 at 04:20:22PM +0800, Huang, Ying wrote:
> Matthew Brost <[email protected]> writes:
> 
> > On Mon, Aug 10, 2026 at 10:26:27AM +0800, Huang, Ying wrote:
> >> Hi, Matthew,
> >> 
> >> Matthew Brost <[email protected]> writes:
> >> 
> >> > When a CPU faults on a device private PMD and the device driver can only
> >> > allocate order-0 destination folios, __migrate_device_pages() has to
> >> > split the source THP via migrate_vma_split_unmapped_folio(). That path
> >> > is broken in two independent ways when the fault is what triggered the
> >> > migration.
> >> >
> >> > First, the split never succeeds. At the point folio_split_unmapped() is
> >> > called the folio carries two references beyond the ones it is
> >> > entitled to:
> >> >
> >> >   1 - taken by do_huge_pmd_device_private() for the duration of the
> >> >       ->migrate_to_ram() callback
> >> >   2 - taken by migrate_vma_collect_huge_pmd() when the folio was
> >> >       collected
> >> >
> >> > (the mapping reference having been dropped by set_pmd_migration_entry()).
> >> >
> >> > folio_split_unmapped() requires folio_expected_ref_count(folio) ==
> >> > folio_ref_count(folio) - 1, i.e. it tolerates exactly one caller
> >> > reference. With both of the above held the check sees 2 against an
> >> > expected 0 and returns -EAGAIN, so the migration is abandoned and the
> >> > CPU fault makes no progress.
> >> >
> >> > The PTE-based split path does not have this problem:
> >> > migrate_vma_split_folio() is called before any collect reference is
> >> > taken and explicitly skips folio_get() for the fault folio, so the fault
> >> > reference is the single caller reference the split expects.
> >> >
> >> > Fix it by dropping the fault reference across the split and re-taking it
> >> > afterwards. do_huge_pmd_device_private() derives the fault page from the
> >> > PMD entry, so it is always the head page of the folio and always ends up
> >> > in the head folio of an uniform split to order 0; re-taking the
> >> > reference on the folio therefore puts it back exactly where
> >> > do_huge_pmd_device_private() will release it. The folio cannot be freed
> >> > while the reference is dropped because the collect reference is still
> >> > held.
> >> >
> >> > Second, the folio is split globally but the page tables were demoted
> >> > only locally:
> >> >
> >> > 	split_huge_pmd_address(migrate->vma, addr, true);
> >> > 	ret = folio_split_unmapped(folio, 0);
> >> >
> >> > migrate_device_unmap() unmaps via try_to_migrate(folio, 0), deliberately
> >> > without TTU_SPLIT_HUGE_PMD, so every VMA that PMD maps the folio is left
> >> > holding a PMD sized migration entry. A folio that was PMD mapped in more
> >> > than one VMA -- after fork(), for example -- therefore keeps huge
> >> > migration entries in all the other VMAs while only migrate->vma is
> >> > demoted.
> >> >
> >> > folio_split_unmapped() does not notice: the folio is fully unmapped, so
> >> > it only looks at the refcount and happily splits to order 0. The other
> >> > VMAs are then left pointing a huge PMD at an order-0 folio, and
> >> > migrate_vma_finalize() -> remove_migration_ptes() walks into it:
> >> >
> >> >   page dumped because: VM_BUG_ON_FOLIO(folio_test_hugetlb(folio) ||
> >> >                                        !folio_test_pmd_mappable(folio))
> >> >   kernel BUG at mm/migrate.c:368!
> >> >   RIP: 0010:remove_migration_pte+0x56a/0x9b0
> >> >   Call Trace:
> >> >    rmap_walk_anon+0xfc/0x260
> >> >    remove_migration_ptes+0x79/0xb0
> >> >    __migrate_device_finalize+0x113/0x290
> >> >    __drm_pagemap_migrate_to_ram+0x278/0x360 [drm_gpusvm_helper]
> >> >    drm_pagemap_migrate_to_ram+0x5c/0x80 [drm_gpusvm_helper]
> >> >    do_huge_pmd_device_private+0x160/0x280
> >> 
> >> Which is the branch your patchset based on?  I found that
> >> drm_pagemap_migrate_populate_ram_pfn() in mm-everything-2026-08-08-07-08
> >> still don't support fallback to single pages if THP allocation fails as
> >> in the following comments,
> >> 
> >
> > This entire series, on drm-tip (i.e., the 6 patches posted here [1]).
> >
> > [1] https://patchwork.freedesktop.org/series/171651/
> 
> Thanks!
> 
> >> 	/* TODO: Support fallback to single pages if THP allocation fails */
> >> 
> >> 
> >> > Without CONFIG_DEBUG_VM the VM_BUG_ON_FOLIO() is compiled out and
> >> > remove_migration_pmd() installs a huge PMD pointing at an order-0 page
> >> > instead, along with add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR). The
> >> > victim mm then maps 2MB of address space onto a single 4K page, which
> >> > shows up later as bad rss-counter state, leaked page tables and page
> >> > allocator freelist corruption in unrelated processes.
> >> >
> >> > Note this second problem was latent before the refcount fix above: the
> >> > split always failed, and the failed attempt left migrate->vma demoted,
> >> > so the retried fault took the PTE path, where __folio_split() unmaps
> >> > with TTU_SPLIT_HUGE_PMD and demotes every VMA.
> >> >
> >> > Fix it by walking the rmap and demoting every PMD sized migration entry
> >> > mapping the folio before splitting it. Demote with freeze = false: entry
> >> > creation in __split_huge_pmd_locked() is dispatched on
> >> > pmd_is_migration_entry(), not on freeze, so a migration PMD becomes PTE
> >> > sized migration entries either way, and freeze only controls a trailing
> >> > put_page(). With freeze = false there is no refcount change at all,
> >> > which makes the demotion idempotent across N VMAs.
> >> >
> >> > rmap_walk_control.anon_lock is deliberately left unset:
> >> > folio_lock_anon_vma_read() depends on folio_mapped(), and the folio is
> >> > already fully unmapped here. This mirrors remove_migration_ptes().
> >> >
> >> > Finally, refuse the split for a folio that is not anonymous. The rmap
> >> > walk would otherwise reach a file backed VMA, where
> >> > split_huge_pmd_address() zaps the PMD instead of demoting it.
> >> >
> >> > Fixes: 4265d67e405a ("mm/migrate_device: add THP splitting during migration")
> >> > Cc: Andrew Morton <[email protected]>
> >> > Cc: David Hildenbrand <[email protected]>
> >> > Cc: Lorenzo Stoakes <[email protected]>
> >> > Cc: Zi Yan <[email protected]>
> >> > Cc: Baolin Wang <[email protected]>
> >> > Cc: Liam R. Howlett <[email protected]>
> >> > Cc: Nico Pache <[email protected]>
> >> > Cc: Ryan Roberts <[email protected]>
> >> > Cc: Dev Jain <[email protected]>
> >> > Cc: Barry Song <[email protected]>
> >> > Cc: Lance Yang <[email protected]>
> >> > Cc: Usama Arif <[email protected]>
> >> > Cc: Joshua Hahn <[email protected]>
> >> > Cc: Rakie Kim <[email protected]>
> >> > Cc: Byungchul Park <[email protected]>
> >> > Cc: Gregory Price <[email protected]>
> >> > Cc: Ying Huang <[email protected]>
> >> > Cc: Alistair Popple <[email protected]>
> >> > Cc: Balbir Singh <[email protected]>
> >> > Cc: Maarten Lankhorst <[email protected]>
> >> > Cc: Maxime Ripard <[email protected]>
> >> > Cc: Thomas Zimmermann <[email protected]>
> >> > Cc: David Airlie <[email protected]>
> >> > Cc: Simona Vetter <[email protected]>
> >> > Cc: Thomas Hellstrm <[email protected]>
> >> > Cc: Francois Dugast <[email protected]>
> >> > Cc: [email protected]
> >> > Cc: [email protected]
> >> > Cc: [email protected]
> >> > Cc: [email protected]
> >> > Assisted-by: GitHub_Copilot:claude-opus-5
> >> > Signed-off-by: Matthew Brost <[email protected]>
> >> > ---
> >> >  mm/migrate_device.c | 98 ++++++++++++++++++++++++++++++++++++++++-----
> >> >  1 file changed, 89 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> >> > index ae9027421b80..ae17bd516d24 100644
> >> > --- a/mm/migrate_device.c
> >> > +++ b/mm/migrate_device.c
> >> > @@ -899,22 +899,104 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
> >> >  	return 0;
> >> >  }
> >> >  
> >> > +static bool migrate_vma_split_pmd_one(struct folio *folio,
> >> > +				      struct vm_area_struct *vma,
> >> > +				      unsigned long addr, void *arg)
> >> > +{
> >> > +	DEFINE_FOLIO_VMA_WALK(pvmw, folio, vma, addr, PVMW_SYNC | PVMW_MIGRATION);
> >> > +
> >> > +	while (page_vma_mapped_walk(&pvmw)) {
> >> > +		if (pvmw.pte)
> >> > +			continue;
> >> > +
> >> > +		addr = pvmw.address;
> >> > +		page_vma_mapped_walk_done(&pvmw);
> >> > +
> >> > +		/*
> >> > +		 * Demote with freeze = false: the PMD already holds a
> >> > +		 * migration entry, so __split_huge_pmd_locked() creates PTE
> >> > +		 * sized migration entries from it and leaves the refcount
> >> > +		 * alone. There is at most one PMD mapping @folio per VMA, so
> >> > +		 * stop the walk here.
> >> > +		 */
> >> > +		split_huge_pmd_address(vma, addr, false);
> >> > +		break;
> >> > +	}
> >> > +
> >> > +	return true;
> >> > +}
> >> > +
> >> > +/*
> >> > + * Demote every PMD sized migration entry that maps @folio to PTE sized ones.
> >> > + *
> >> > + * migrate_device_unmap() unmaps with try_to_migrate(folio, 0), i.e. without
> >> > + * TTU_SPLIT_HUGE_PMD, so a folio that was PMD mapped in several VMAs -- after
> >> > + * fork(), for instance -- ends up with a PMD sized migration entry in every one
> >> > + * of them. folio_split_unmapped() below does not care, it only looks at the
> >> > + * refcount, so splitting the folio without demoting all of those first would
> >> > + * leave the other VMAs pointing a huge PMD at what is now an order-0 folio.
> >> > + * remove_migration_ptes() trips over that in migrate_vma_finalize().
> >> > + */
> >> > +static void migrate_vma_split_pmd_mappings(struct folio *folio)
> >> > +{
> >> > +	struct rmap_walk_control rwc = {
> >> > +		.rmap_one = migrate_vma_split_pmd_one,
> >> > +	};
> >> > +
> >> > +	/*
> >> > +	 * Do not pass .anon_lock: folio_lock_anon_vma_read() requires
> >> > +	 * folio_mapped(), and @folio is already fully unmapped here.
> >> > +	 */
> >> > +	rmap_walk(folio, &rwc);
> >> > +}
> >> > +
> >> >  static int migrate_vma_split_unmapped_folio(struct migrate_vma *migrate,
> >> > -					    unsigned long idx, unsigned long addr,
> >> > +					    unsigned long idx,
> >> >  					    struct folio *folio)
> >> >  {
> >> >  	unsigned long i;
> >> >  	unsigned long pfn;
> >> >  	unsigned long flags;
> >> > +	bool fault_folio;
> >> >  	int ret = 0;
> >> >  
> >> >  	/*
> >> > -	 * take a reference, since split_huge_pmd_address() with freeze = true
> >> > -	 * drops a reference at the end.
> >> > +	 * migrate_vma_split_pmd_mappings() walks the rmap, and
> >> > +	 * split_huge_pmd_address() zaps rather than demotes a PMD in a VMA that
> >> > +	 * is not anonymous. migrate_vma_collect_huge_pmd() does not check the
> >> > +	 * VMA type, so a file THP can reach here; the rest of the migrate_vma()
> >> > +	 * machinery only supports anonymous memory anyway.
> >> >  	 */
> >> > -	folio_get(folio);
> >> > -	split_huge_pmd_address(migrate->vma, addr, true);
> >> > +	if (!folio_test_anon(folio))
> >> > +		return -EINVAL;
> >> > +
> >> > +	/*
> >> > +	 * A CPU fault on a device private PMD holds an extra reference on the
> >> > +	 * folio, taken by do_huge_pmd_device_private(). folio_split_unmapped()
> >> > +	 * only tolerates a single caller reference, so the split would always
> >> > +	 * fail with -EAGAIN while this fault reference is held.
> >> > +	 *
> >> > +	 * do_huge_pmd_device_private() derives the fault page from the PMD
> >> > +	 * entry, so it is always the head page of @folio, and therefore always
> >> > +	 * ends up in the head folio after an uniform split to order 0. Drop
> >> > +	 * the reference across the split and re-take it on the head folio
> >> > +	 * afterwards, leaving the reference exactly where it is expected to be
> >> > +	 * released.
> >> > +	 *
> >> > +	 * The folio cannot go away while the reference is dropped: the
> >> > +	 * reference taken by migrate_vma_collect_huge_pmd() is still held.
> >> > +	 */
> >> > +	fault_folio = migrate->fault_page &&
> >> > +		page_folio(migrate->fault_page) == folio;
> >> > +
> >> > +	migrate_vma_split_pmd_mappings(folio);
> >> > +
> >> > +	if (fault_folio)
> >> > +		folio_put(folio);
> >> >  	ret = folio_split_unmapped(folio, 0);
> >> > +	if (fault_folio)
> >> > +		folio_get(folio);
> >> > +
> >> 
> >> Is it better to pass "extra_cnt" to folio_split_unmapped()?  This
> >> follows the coding style of the other migrate functions better, like
> >> that in __migrate_device_pages().
> >> 
> >
> > That is an option. To be minimally invasive, I went this route. I also
> > didn't know offhand what would happen if our head page had an extra
> > reference and we then called folio_split_unmapped() with "extra_cnt", or
> > how that would affect the reference counts of the newly split pages
> > (i.e., whether we would need to adjust the reference counts of all split
> > pages after folio_split_unmapped() returns). However, I could quickly
> > reason that dropping the reference and then reacquiring it was
> > functionally correct and safe.
> 
> This makes sense for me.  Thanks!
> 
> I have another question.  If we have to split the large folio when
> migrating from device to ram, should we still migrate all pages of the
> original large folio, or should we migrate only the faulting
> normal-sized page of the original large folio instead?
> 

This is a choice made by the upper layers that call the migrate_vma_*
functions and populate the migrate_vma arguments. In gpusvm/pagemap, we
still migrate the entire 2 MB region of memory as 512 4 KB pages upon
higher order failure, matching what we did prior to having 2 MB device
pages.
 
The reasoning is that migrations are expensive due to the CPU overhead of
migrate_vma_* and because GPU copies are issued, requiring larger transfer
sizes to achieve the full bandwidth of the bus. For example, a 4 KB copy
provides less than 1 GB/s of bandwidth regardless of PCIe speed, whereas
a 2 MB copy can nearly reach the theoretical maximum bandwidth of PCIe.
 
Early in the development of gpusvm/pagemap, I had a knob that forced only
single-page 4 KB faults and migrations, along with a test case that
measured the fault time in user space for a 2 MB buffer. If I recall
correctly, it was about 58× slower than batching 512 4 KB pages together
into a single fault and migration on a low-end BMG part.

So on higher order page allocation failure, the preference is still do
the larger migration.

Matt

> ---
> Best Regards,
> Huang, Ying
> 
> > Matt
> >
> >> >  	if (ret)
> >> >  		return ret;
> >> >  	migrate->src[idx] &= ~MIGRATE_PFN_COMPOUND;
> >> > @@ -935,7 +1017,7 @@ static int migrate_vma_insert_huge_pmd_page(struct migrate_vma *migrate,
> >> >  }
> >> >  
> >> >  static int migrate_vma_split_unmapped_folio(struct migrate_vma *migrate,
> >> > -					    unsigned long idx, unsigned long addr,
> >> > +					    unsigned long idx,
> >> >  					    struct folio *folio)
> >> >  {
> >> >  	return 0;
> >> > @@ -1103,7 +1185,6 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> >> >  	struct mmu_notifier_range range;
> >> >  	unsigned long i, j;
> >> >  	bool notified = false;
> >> > -	unsigned long addr;
> >> >  
> >> >  	for (i = 0; i < npages; ) {
> >> >  		struct page *newpage = migrate_pfn_to_page(dst_pfns[i]);
> >> > @@ -1177,8 +1258,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> >> >  					goto next;
> >> >  				}
> >> >  				nr = 1 << folio_order(folio);
> >> > -				addr = migrate->start + i * PAGE_SIZE;
> >> > -				if (migrate_vma_split_unmapped_folio(migrate, i, addr, folio)) {
> >> > +				if (migrate_vma_split_unmapped_folio(migrate, i, folio)) {
> >> >  					src_pfns[i] &= ~(MIGRATE_PFN_MIGRATE |
> >> >  							 MIGRATE_PFN_COMPOUND);
> >> >  					goto next;
> >> 
> >> ---
> >> Best Regards,
> >> Huang, Ying