* Re: [PATCH v3 2/6] mm/migrate_device: Do not write past the end of the src_pfns array [not found] ` <20260805231041.3791771-3-matthew.brost@intel.com> @ 2026-08-05 23:29 ` Balbir Singh 0 siblings, 0 replies; 10+ messages in thread From: Balbir Singh @ 2026-08-05 23:29 UTC (permalink / raw) To: Matthew Brost, intel-xe, dri-devel, linux-mm, linux-kernel Cc: Sashiko, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang, Alistair Popple, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellström, Francois Dugast, stable On 8/6/26 9:10 AM, Matthew Brost wrote: > migrate_device_range() and migrate_device_pfns() zero the tail entries > of a large folio without checking them against @npages: > > for (j = 1; j < nr; j++) > src_pfns[i+j] = 0; > > @nr comes from the folio, not from the array, so a folio that extends > past the end of the range being migrated writes beyond src_pfns[]. > Callers size that array for @npages entries, so this corrupts whatever > follows it. > > Bound the loop by @npages. The subsequent "i += j - 1" still terminates > the outer loop correctly: on a bounded exit j is @npages - i, leaving i > at @npages after the increment. > > Reported-by: Sashiko <sashiko-bot@kernel.org> > Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone device pages") > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: David Hildenbrand <david@kernel.org> > Cc: Lorenzo Stoakes <ljs@kernel.org> > Cc: Zi Yan <ziy@nvidia.com> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Liam R. Howlett <liam@infradead.org> > Cc: Nico Pache <nico.pache@linux.dev> > Cc: Ryan Roberts <ryan.roberts@arm.com> > Cc: Dev Jain <dev.jain@arm.com> > Cc: Barry Song <baohua@kernel.org> > Cc: Lance Yang <lance.yang@linux.dev> > Cc: Usama Arif <usama.arif@linux.dev> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > Cc: Rakie Kim <rakie.kim@sk.com> > Cc: Byungchul Park <byungchul@sk.com> > Cc: Gregory Price <gourry@gourry.net> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > Cc: Alistair Popple <apopple@nvidia.com> > Cc: Balbir Singh <balbirs@nvidia.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Francois Dugast <francois.dugast@intel.com> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Cc: stable@vger.kernel.org > Assisted-by: GitHub_Copilot:claude-opus-5 > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > mm/migrate_device.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/migrate_device.c b/mm/migrate_device.c > index 162d29b2807a..ae9027421b80 100644 > --- a/mm/migrate_device.c > +++ b/mm/migrate_device.c > @@ -1415,7 +1415,7 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start, > nr = folio_nr_pages(folio); > if (nr > 1) { > src_pfns[i] |= MIGRATE_PFN_COMPOUND; > - for (j = 1; j < nr; j++) > + for (j = 1; j < nr && (i + j) < npages; j++) > src_pfns[i+j] = 0; I have a similar patch lined up in my clean ups (that I am yet to send out), but the patch was more along the lines of nr = folio_nr_pages(folio); + if (nr > npages - i) { + migrate_device_folio_unlock(folio); + src_pfns[i] = 0; + continue; + } + This prevents partial selection, migrate_device_unmap() does take npages as an argument. We could change the increment of i here to skip past the entire folio. I am OK with this change as well > i += j - 1; > pfn += j - 1; > @@ -1449,7 +1449,7 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages) > nr = folio_nr_pages(folio); > if (nr > 1) { > src_pfns[i] |= MIGRATE_PFN_COMPOUND; > - for (j = 1; j < nr; j++) > + for (j = 1; j < nr && (i + j) < npages; j++) > src_pfns[i+j] = 0; > i += j - 1; > } Reviewed-by: Balbir Singh <balbirs@nvidia.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20260805231041.3791771-4-matthew.brost@intel.com>]
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio [not found] ` <20260805231041.3791771-4-matthew.brost@intel.com> @ 2026-08-06 8:10 ` Balbir Singh 2026-08-10 2:26 ` Huang, Ying 1 sibling, 0 replies; 10+ messages in thread From: Balbir Singh @ 2026-08-06 8:10 UTC (permalink / raw) To: Matthew Brost, intel-xe, dri-devel, linux-mm, linux-kernel Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang, Alistair Popple, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellström, Francois Dugast, stable On 8/6/26 9:10 AM, Matthew Brost wrote: > 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 > > 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. > The refcount fix to enable migration of PMD's in fault context. IOW, if we need PMD migration in the context of a CPU fault, then the proposed patch is the right fix, otherwise we split and migrate and that has no crashes/impact? > 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 <akpm@linux-foundation.org> > Cc: David Hildenbrand <david@kernel.org> > Cc: Lorenzo Stoakes <ljs@kernel.org> > Cc: Zi Yan <ziy@nvidia.com> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Liam R. Howlett <liam@infradead.org> > Cc: Nico Pache <nico.pache@linux.dev> > Cc: Ryan Roberts <ryan.roberts@arm.com> > Cc: Dev Jain <dev.jain@arm.com> > Cc: Barry Song <baohua@kernel.org> > Cc: Lance Yang <lance.yang@linux.dev> > Cc: Usama Arif <usama.arif@linux.dev> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > Cc: Rakie Kim <rakie.kim@sk.com> > Cc: Byungchul Park <byungchul@sk.com> > Cc: Gregory Price <gourry@gourry.net> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > Cc: Alistair Popple <apopple@nvidia.com> > Cc: Balbir Singh <balbirs@nvidia.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Francois Dugast <francois.dugast@intel.com> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Cc: stable@vger.kernel.org > Assisted-by: GitHub_Copilot:claude-opus-5 > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > 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); > + > 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; Reviewed-by: Balbir Singh <balbirs@nvidia.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio [not found] ` <20260805231041.3791771-4-matthew.brost@intel.com> 2026-08-06 8:10 ` [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio Balbir Singh @ 2026-08-10 2:26 ` Huang, Ying 2026-08-10 19:43 ` Matthew Brost 1 sibling, 1 reply; 10+ messages in thread From: Huang, Ying @ 2026-08-10 2:26 UTC (permalink / raw) To: Matthew Brost Cc: intel-xe, dri-devel, linux-mm, linux-kernel, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellström, Francois Dugast, stable Hi, Matthew, Matthew Brost <matthew.brost@intel.com> 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, /* 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 <akpm@linux-foundation.org> > Cc: David Hildenbrand <david@kernel.org> > Cc: Lorenzo Stoakes <ljs@kernel.org> > Cc: Zi Yan <ziy@nvidia.com> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Liam R. Howlett <liam@infradead.org> > Cc: Nico Pache <nico.pache@linux.dev> > Cc: Ryan Roberts <ryan.roberts@arm.com> > Cc: Dev Jain <dev.jain@arm.com> > Cc: Barry Song <baohua@kernel.org> > Cc: Lance Yang <lance.yang@linux.dev> > Cc: Usama Arif <usama.arif@linux.dev> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > Cc: Rakie Kim <rakie.kim@sk.com> > Cc: Byungchul Park <byungchul@sk.com> > Cc: Gregory Price <gourry@gourry.net> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > Cc: Alistair Popple <apopple@nvidia.com> > Cc: Balbir Singh <balbirs@nvidia.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Francois Dugast <francois.dugast@intel.com> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Cc: stable@vger.kernel.org > Assisted-by: GitHub_Copilot:claude-opus-5 > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > 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(). > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio 2026-08-10 2:26 ` Huang, Ying @ 2026-08-10 19:43 ` Matthew Brost 2026-08-12 8:20 ` Huang, Ying 0 siblings, 1 reply; 10+ messages in thread From: Matthew Brost @ 2026-08-10 19:43 UTC (permalink / raw) To: Huang, Ying Cc: intel-xe, dri-devel, linux-mm, linux-kernel, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellström, Francois Dugast, stable On Mon, Aug 10, 2026 at 10:26:27AM +0800, Huang, Ying wrote: > Hi, Matthew, > > Matthew Brost <matthew.brost@intel.com> 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/ > /* 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 <akpm@linux-foundation.org> > > Cc: David Hildenbrand <david@kernel.org> > > Cc: Lorenzo Stoakes <ljs@kernel.org> > > Cc: Zi Yan <ziy@nvidia.com> > > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > > Cc: Liam R. Howlett <liam@infradead.org> > > Cc: Nico Pache <nico.pache@linux.dev> > > Cc: Ryan Roberts <ryan.roberts@arm.com> > > Cc: Dev Jain <dev.jain@arm.com> > > Cc: Barry Song <baohua@kernel.org> > > Cc: Lance Yang <lance.yang@linux.dev> > > Cc: Usama Arif <usama.arif@linux.dev> > > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > > Cc: Rakie Kim <rakie.kim@sk.com> > > Cc: Byungchul Park <byungchul@sk.com> > > Cc: Gregory Price <gourry@gourry.net> > > Cc: Ying Huang <ying.huang@linux.alibaba.com> > > Cc: Alistair Popple <apopple@nvidia.com> > > Cc: Balbir Singh <balbirs@nvidia.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Maxime Ripard <mripard@kernel.org> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: David Airlie <airlied@gmail.com> > > Cc: Simona Vetter <simona@ffwll.ch> > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > > Cc: Francois Dugast <francois.dugast@intel.com> > > Cc: dri-devel@lists.freedesktop.org > > Cc: linux-mm@kvack.org > > Cc: linux-kernel@vger.kernel.org > > Cc: stable@vger.kernel.org > > Assisted-by: GitHub_Copilot:claude-opus-5 > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > --- > > 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. 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio 2026-08-10 19:43 ` Matthew Brost @ 2026-08-12 8:20 ` Huang, Ying 2026-08-12 23:33 ` Matthew Brost 0 siblings, 1 reply; 10+ messages in thread From: Huang, Ying @ 2026-08-12 8:20 UTC (permalink / raw) To: Matthew Brost Cc: intel-xe, dri-devel, linux-mm, linux-kernel, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellstrm, Francois Dugast, stable Matthew Brost <matthew.brost@intel.com> writes: > On Mon, Aug 10, 2026 at 10:26:27AM +0800, Huang, Ying wrote: >> Hi, Matthew, >> >> Matthew Brost <matthew.brost@intel.com> 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 <akpm@linux-foundation.org> >> > Cc: David Hildenbrand <david@kernel.org> >> > Cc: Lorenzo Stoakes <ljs@kernel.org> >> > Cc: Zi Yan <ziy@nvidia.com> >> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> >> > Cc: Liam R. Howlett <liam@infradead.org> >> > Cc: Nico Pache <nico.pache@linux.dev> >> > Cc: Ryan Roberts <ryan.roberts@arm.com> >> > Cc: Dev Jain <dev.jain@arm.com> >> > Cc: Barry Song <baohua@kernel.org> >> > Cc: Lance Yang <lance.yang@linux.dev> >> > Cc: Usama Arif <usama.arif@linux.dev> >> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> >> > Cc: Rakie Kim <rakie.kim@sk.com> >> > Cc: Byungchul Park <byungchul@sk.com> >> > Cc: Gregory Price <gourry@gourry.net> >> > Cc: Ying Huang <ying.huang@linux.alibaba.com> >> > Cc: Alistair Popple <apopple@nvidia.com> >> > Cc: Balbir Singh <balbirs@nvidia.com> >> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> > Cc: Maxime Ripard <mripard@kernel.org> >> > Cc: Thomas Zimmermann <tzimmermann@suse.de> >> > Cc: David Airlie <airlied@gmail.com> >> > Cc: Simona Vetter <simona@ffwll.ch> >> > Cc: Thomas Hellstrm <thomas.hellstrom@linux.intel.com> >> > Cc: Francois Dugast <francois.dugast@intel.com> >> > Cc: dri-devel@lists.freedesktop.org >> > Cc: linux-mm@kvack.org >> > Cc: linux-kernel@vger.kernel.org >> > Cc: stable@vger.kernel.org >> > Assisted-by: GitHub_Copilot:claude-opus-5 >> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> >> > --- >> > 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? --- 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio 2026-08-12 8:20 ` Huang, Ying @ 2026-08-12 23:33 ` Matthew Brost 2026-08-13 1:54 ` Huang, Ying 0 siblings, 1 reply; 10+ messages in thread From: Matthew Brost @ 2026-08-12 23:33 UTC (permalink / raw) To: Huang, Ying Cc: intel-xe, dri-devel, linux-mm, linux-kernel, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellstrm, Francois Dugast, stable On Wed, Aug 12, 2026 at 04:20:22PM +0800, Huang, Ying wrote: > Matthew Brost <matthew.brost@intel.com> writes: > > > On Mon, Aug 10, 2026 at 10:26:27AM +0800, Huang, Ying wrote: > >> Hi, Matthew, > >> > >> Matthew Brost <matthew.brost@intel.com> 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 <akpm@linux-foundation.org> > >> > Cc: David Hildenbrand <david@kernel.org> > >> > Cc: Lorenzo Stoakes <ljs@kernel.org> > >> > Cc: Zi Yan <ziy@nvidia.com> > >> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > >> > Cc: Liam R. Howlett <liam@infradead.org> > >> > Cc: Nico Pache <nico.pache@linux.dev> > >> > Cc: Ryan Roberts <ryan.roberts@arm.com> > >> > Cc: Dev Jain <dev.jain@arm.com> > >> > Cc: Barry Song <baohua@kernel.org> > >> > Cc: Lance Yang <lance.yang@linux.dev> > >> > Cc: Usama Arif <usama.arif@linux.dev> > >> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > >> > Cc: Rakie Kim <rakie.kim@sk.com> > >> > Cc: Byungchul Park <byungchul@sk.com> > >> > Cc: Gregory Price <gourry@gourry.net> > >> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > >> > Cc: Alistair Popple <apopple@nvidia.com> > >> > Cc: Balbir Singh <balbirs@nvidia.com> > >> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> > Cc: Maxime Ripard <mripard@kernel.org> > >> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > >> > Cc: David Airlie <airlied@gmail.com> > >> > Cc: Simona Vetter <simona@ffwll.ch> > >> > Cc: Thomas Hellstrm <thomas.hellstrom@linux.intel.com> > >> > Cc: Francois Dugast <francois.dugast@intel.com> > >> > Cc: dri-devel@lists.freedesktop.org > >> > Cc: linux-mm@kvack.org > >> > Cc: linux-kernel@vger.kernel.org > >> > Cc: stable@vger.kernel.org > >> > Assisted-by: GitHub_Copilot:claude-opus-5 > >> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > >> > --- > >> > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio 2026-08-12 23:33 ` Matthew Brost @ 2026-08-13 1:54 ` Huang, Ying 2026-08-13 8:33 ` Matthew Brost 0 siblings, 1 reply; 10+ messages in thread From: Huang, Ying @ 2026-08-13 1:54 UTC (permalink / raw) To: Matthew Brost Cc: intel-xe, dri-devel, linux-mm, linux-kernel, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellstrm, Francois Dugast, stable Matthew Brost <matthew.brost@intel.com> writes: > On Wed, Aug 12, 2026 at 04:20:22PM +0800, Huang, Ying wrote: >> Matthew Brost <matthew.brost@intel.com> writes: >> >> > On Mon, Aug 10, 2026 at 10:26:27AM +0800, Huang, Ying wrote: >> >> Hi, Matthew, >> >> >> >> Matthew Brost <matthew.brost@intel.com> 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 <akpm@linux-foundation.org> >> >> > Cc: David Hildenbrand <david@kernel.org> >> >> > Cc: Lorenzo Stoakes <ljs@kernel.org> >> >> > Cc: Zi Yan <ziy@nvidia.com> >> >> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> >> >> > Cc: Liam R. Howlett <liam@infradead.org> >> >> > Cc: Nico Pache <nico.pache@linux.dev> >> >> > Cc: Ryan Roberts <ryan.roberts@arm.com> >> >> > Cc: Dev Jain <dev.jain@arm.com> >> >> > Cc: Barry Song <baohua@kernel.org> >> >> > Cc: Lance Yang <lance.yang@linux.dev> >> >> > Cc: Usama Arif <usama.arif@linux.dev> >> >> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> >> >> > Cc: Rakie Kim <rakie.kim@sk.com> >> >> > Cc: Byungchul Park <byungchul@sk.com> >> >> > Cc: Gregory Price <gourry@gourry.net> >> >> > Cc: Ying Huang <ying.huang@linux.alibaba.com> >> >> > Cc: Alistair Popple <apopple@nvidia.com> >> >> > Cc: Balbir Singh <balbirs@nvidia.com> >> >> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> >> > Cc: Maxime Ripard <mripard@kernel.org> >> >> > Cc: Thomas Zimmermann <tzimmermann@suse.de> >> >> > Cc: David Airlie <airlied@gmail.com> >> >> > Cc: Simona Vetter <simona@ffwll.ch> >> >> > Cc: Thomas Hellstrm <thomas.hellstrom@linux.intel.com> >> >> > Cc: Francois Dugast <francois.dugast@intel.com> >> >> > Cc: dri-devel@lists.freedesktop.org >> >> > Cc: linux-mm@kvack.org >> >> > Cc: linux-kernel@vger.kernel.org >> >> > Cc: stable@vger.kernel.org >> >> > Assisted-by: GitHub_Copilot:claude-opus-5 >> >> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> >> >> > --- >> >> > 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. Got it! Thanks for detailed explanation! I think that another added complexity is that we need to split the 2MB page table of the GPU side. --- Best Regards, Huang, Ying > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio 2026-08-13 1:54 ` Huang, Ying @ 2026-08-13 8:33 ` Matthew Brost 0 siblings, 0 replies; 10+ messages in thread From: Matthew Brost @ 2026-08-13 8:33 UTC (permalink / raw) To: Huang, Ying Cc: intel-xe, dri-devel, linux-mm, linux-kernel, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellstrm, Francois Dugast, stable On Thu, Aug 13, 2026 at 09:54:39AM +0800, Huang, Ying wrote: > Matthew Brost <matthew.brost@intel.com> writes: > > > On Wed, Aug 12, 2026 at 04:20:22PM +0800, Huang, Ying wrote: > >> Matthew Brost <matthew.brost@intel.com> writes: > >> > >> > On Mon, Aug 10, 2026 at 10:26:27AM +0800, Huang, Ying wrote: > >> >> Hi, Matthew, > >> >> > >> >> Matthew Brost <matthew.brost@intel.com> 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 <akpm@linux-foundation.org> > >> >> > Cc: David Hildenbrand <david@kernel.org> > >> >> > Cc: Lorenzo Stoakes <ljs@kernel.org> > >> >> > Cc: Zi Yan <ziy@nvidia.com> > >> >> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > >> >> > Cc: Liam R. Howlett <liam@infradead.org> > >> >> > Cc: Nico Pache <nico.pache@linux.dev> > >> >> > Cc: Ryan Roberts <ryan.roberts@arm.com> > >> >> > Cc: Dev Jain <dev.jain@arm.com> > >> >> > Cc: Barry Song <baohua@kernel.org> > >> >> > Cc: Lance Yang <lance.yang@linux.dev> > >> >> > Cc: Usama Arif <usama.arif@linux.dev> > >> >> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > >> >> > Cc: Rakie Kim <rakie.kim@sk.com> > >> >> > Cc: Byungchul Park <byungchul@sk.com> > >> >> > Cc: Gregory Price <gourry@gourry.net> > >> >> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > >> >> > Cc: Alistair Popple <apopple@nvidia.com> > >> >> > Cc: Balbir Singh <balbirs@nvidia.com> > >> >> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> >> > Cc: Maxime Ripard <mripard@kernel.org> > >> >> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > >> >> > Cc: David Airlie <airlied@gmail.com> > >> >> > Cc: Simona Vetter <simona@ffwll.ch> > >> >> > Cc: Thomas Hellstrm <thomas.hellstrom@linux.intel.com> > >> >> > Cc: Francois Dugast <francois.dugast@intel.com> > >> >> > Cc: dri-devel@lists.freedesktop.org > >> >> > Cc: linux-mm@kvack.org > >> >> > Cc: linux-kernel@vger.kernel.org > >> >> > Cc: stable@vger.kernel.org > >> >> > Assisted-by: GitHub_Copilot:claude-opus-5 > >> >> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > >> >> > --- > >> >> > 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. > > Got it! Thanks for detailed explanation! > > I think that another added complexity is that we need to split the 2MB > page table of the GPU side. > That part is somewhat of an independent issue. When memory is moved, we receive an MMU notifier that invalidates the GPU pages. Whenever we rebind, the GPU pages are recreated based on the new memory placement, and GPU pages of the appropriate size are populated. This part is more or less driver- and device-specific. Matt > --- > Best Regards, > Huang, Ying > > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20260805231041.3791771-5-matthew.brost@intel.com>]
* Re: [PATCH v3 4/6] drm/pagemap: dma-unmap pages before handling migration errors [not found] ` <20260805231041.3791771-5-matthew.brost@intel.com> @ 2026-08-16 15:04 ` Ghimiray, Himal Prasad 0 siblings, 0 replies; 10+ messages in thread From: Ghimiray, Himal Prasad @ 2026-08-16 15:04 UTC (permalink / raw) To: Matthew Brost, intel-xe, dri-devel, linux-mm, linux-kernel Cc: Sashiko, Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellström, Francois Dugast, stable On 06-08-2026 04:40, Matthew Brost wrote: > drm_pagemap_migrate_unmap_pages() relies on the pages array to determine > which pages require DMA unmapping. However, > drm_pagemap_migration_unlock_put_pages() clears the array as part of its > cleanup, leaving drm_pagemap_migrate_unmap_pages() with no valid page > information if it is called afterward. > > Call drm_pagemap_migrate_unmap_pages() before > drm_pagemap_migration_unlock_put_pages() so the pages array remains > valid during DMA unmapping. > > Reported-by: Sashiko <sashiko-bot@kernel.org> > Fixes: f86ad0ed620c ("drm/gpusvm, drm/pagemap: Move migration functionality to drm_pagemap") > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: David Hildenbrand <david@kernel.org> > Cc: Lorenzo Stoakes <ljs@kernel.org> > Cc: Zi Yan <ziy@nvidia.com> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Liam R. Howlett <liam@infradead.org> > Cc: Nico Pache <nico.pache@linux.dev> > Cc: Ryan Roberts <ryan.roberts@arm.com> > Cc: Dev Jain <dev.jain@arm.com> > Cc: Barry Song <baohua@kernel.org> > Cc: Lance Yang <lance.yang@linux.dev> > Cc: Usama Arif <usama.arif@linux.dev> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > Cc: Rakie Kim <rakie.kim@sk.com> > Cc: Byungchul Park <byungchul@sk.com> > Cc: Gregory Price <gourry@gourry.net> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > Cc: Alistair Popple <apopple@nvidia.com> > Cc: Balbir Singh <balbirs@nvidia.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Francois Dugast <francois.dugast@intel.com> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Cc: stable@vger.kernel.org > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/drm_pagemap.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index 892b325fa99b..aef1fcea663d 100644 > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c > @@ -1175,12 +1175,12 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation) > goto err_finalize; > > err_finalize: > + drm_pagemap_migrate_unmap_pages(devmem_allocation->dev, pagemap_addr, dst, npages, > + DMA_FROM_DEVICE, &state); > if (err) > drm_pagemap_migration_unlock_put_pages(npages, dst); > migrate_device_pages(src, dst, npages); > migrate_device_finalize(src, dst, npages); > - drm_pagemap_migrate_unmap_pages(devmem_allocation->dev, pagemap_addr, dst, npages, > - DMA_FROM_DEVICE, &state); > > err_free: > kvfree(buf); > @@ -1305,14 +1305,14 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas, > goto err_finalize; > > err_finalize: > - if (err) > - drm_pagemap_migration_unlock_put_pages(npages, migrate.dst); > - migrate_vma_pages(&migrate); > - migrate_vma_finalize(&migrate); > if (dev) > drm_pagemap_migrate_unmap_pages(dev, pagemap_addr, migrate.dst, > npages, DMA_FROM_DEVICE, > &state); > + if (err) > + drm_pagemap_migration_unlock_put_pages(npages, migrate.dst); > + migrate_vma_pages(&migrate); > + migrate_vma_finalize(&migrate); LGTM Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > err_free: > kvfree(buf); > err_out: ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20260805231041.3791771-6-matthew.brost@intel.com>]
* Re: [PATCH v3 5/6] drm/pagemap: Fix folio allocation fallback and use-after-put [not found] ` <20260805231041.3791771-6-matthew.brost@intel.com> @ 2026-08-16 15:36 ` Ghimiray, Himal Prasad 0 siblings, 0 replies; 10+ messages in thread From: Ghimiray, Himal Prasad @ 2026-08-16 15:36 UTC (permalink / raw) To: Matthew Brost, intel-xe, dri-devel, linux-mm, linux-kernel Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif, Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang, Alistair Popple, Balbir Singh, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Thomas Hellström, Francois Dugast, stable On 06-08-2026 04:40, Matthew Brost wrote: > drm_pagemap_migrate_populate_ram_pfn() had two issues when populating > RAM PFNs with higher-order folios: > > 1. The higher-order vma_alloc_folio()/folio_alloc() calls did not pass > __GFP_NOWARN, so a THP allocation failure under memory pressure > would spam the kernel log, and there was no fallback path despite a > TODO comment stating one was needed. Add __GFP_NOWARN to the > higher-order allocation and, on failure, fall back to order-0 > allocations for the entire range originally covered by the failed > higher-order allocation, leaving MIGRATE_PFN_COMPOUND unset for > those PFNs. > > 2. In the free_pages error path, order was computed via > folio_order(page_folio(page)) *after* put_page(page) had already > dropped the reference, resulting in a use-after-free/put when that > was the last reference on the page. Compute order before releasing > the page. > > Introducing the fallback in 1. also requires the source page array > handed to ->copy_to_ram() to be built differently. Both callers only > populated the entry at the head of each source folio, relying on the > copy callback to derive the rest of the folio from the order recorded > in the matching drm_pagemap_addr. Once the destination has been demoted > to order-0 folios the drm_pagemap_addr entries are per-page, so a source > page is needed for every one of them; leaving them NULL makes the copy > callback stop after the first page and the remainder of the range is > never copied. > > The source folio is only split later, by migrate_vma_pages() / > migrate_device_pages(), so its order cannot be used to detect the > demotion - test the destination for MIGRATE_PFN_COMPOUND instead. Factor > the array population out into drm_pagemap_migrate_populate_src_pages() > and use it from both drm_pagemap_evict_to_ram() and > __drm_pagemap_migrate_to_ram(). > > Fixes: ddeda6136038 ("drm/pagemap: Allocate folios when possible") > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: David Hildenbrand <david@kernel.org> > Cc: Lorenzo Stoakes <ljs@kernel.org> > Cc: Zi Yan <ziy@nvidia.com> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Liam R. Howlett <liam@infradead.org> > Cc: Nico Pache <nico.pache@linux.dev> > Cc: Ryan Roberts <ryan.roberts@arm.com> > Cc: Dev Jain <dev.jain@arm.com> > Cc: Barry Song <baohua@kernel.org> > Cc: Lance Yang <lance.yang@linux.dev> > Cc: Usama Arif <usama.arif@linux.dev> > Cc: Joshua Hahn <joshua.hahnjy@gmail.com> > Cc: Rakie Kim <rakie.kim@sk.com> > Cc: Byungchul Park <byungchul@sk.com> > Cc: Gregory Price <gourry@gourry.net> > Cc: Ying Huang <ying.huang@linux.alibaba.com> > Cc: Alistair Popple <apopple@nvidia.com> > Cc: Balbir Singh <balbirs@nvidia.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Francois Dugast <francois.dugast@intel.com> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Cc: stable@vger.kernel.org > Assisted-by: GitHub_Copilot:claude-opus-5 > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > --- > v2: Add THP-mid-PMD invariant (Sashiko) > --- > drivers/gpu/drm/drm_pagemap.c | 128 +++++++++++++++++++++++++++------- > 1 file changed, 103 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index aef1fcea663d..51c6f12e4256 100644 > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c > @@ -383,6 +383,58 @@ drm_pagemap_migrate_map_system_pages(struct device *dev, > return 0; > } > > +/** > + * drm_pagemap_migrate_populate_src_pages() - Populate the source page array > + * @pages: Array of source pages to populate > + * @src_mpfn: Source array of migrate PFNs > + * @dst_mpfn: Destination array of migrate PFNs > + * @npages: Number of pages in the arrays > + * > + * Populate @pages with the device pages the copy callback is to read from. > + * > + * Entries are normally only populated at the head of each source folio, with > + * the copy callback deriving the rest of the folio from the order recorded in > + * the corresponding drm_pagemap_addr. That does not work where > + * drm_pagemap_migrate_populate_ram_pfn() had to demote a higher-order source > + * folio to order-0 destination folios: the drm_pagemap_addr entries are then > + * per-page, and the copy callback needs a source page for each of them. > + * Populate every entry for those ranges. > + * > + * Note that the source folio itself is only split later, by > + * migrate_vma_pages() / migrate_device_pages(), so its order cannot be used to > + * detect the demotion - the destination has to be inspected instead. > + */ > +static void drm_pagemap_migrate_populate_src_pages(struct page **pages, > + unsigned long *src_mpfn, > + unsigned long *dst_mpfn, > + unsigned long npages) > +{ > + unsigned long i; > + > + for (i = 0; i < npages;) { > + struct page *page = migrate_pfn_to_page(src_mpfn[i]); > + unsigned int order = 0; > + unsigned long j, nr; > + > + if (!page) { > + i++; > + continue; > + } > + > + order = folio_order(page_folio(page)); > + nr = NR_PAGES(order); > + > + if (order && !(dst_mpfn[i] & MIGRATE_PFN_COMPOUND)) { > + for (j = 0; j < nr && i + j < npages; j++) > + pages[i + j] = folio_page(page_folio(page), j); > + } else { > + pages[i] = page; > + } > + > + i += nr; > + } > +} > + > /** > * drm_pagemap_migrate_unmap_pages() - Unmap pages previously mapped for GPU SVM migration > * @dev: The device for which the pages were mapped > @@ -875,6 +927,7 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct vm_area_struct *vas, > struct page *page = NULL, *src_page; > struct folio *folio; > unsigned int order = 0; > + gfp_t gfp = GFP_HIGHUSER; > > if (!(src_mpfn[i] & MIGRATE_PFN_MIGRATE)) > goto next; > @@ -891,11 +944,51 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct vm_area_struct *vas, > > order = folio_order(page_folio(src_page)); > > - /* TODO: Support fallback to single pages if THP allocation fails */ > + /* > + * A large source folio is always collected whole, at its head > + * page, PMD aligned and flagged MIGRATE_PFN_COMPOUND: anything > + * else is split before it reaches us, either by > + * migrate_vma_collect_pmd() or, for the eviction path, by > + * migrate_device_pfns(). Both the order-0 fallback below and > + * drm_pagemap_migrate_populate_src_pages() rely on that, as > + * they index the folio from @i. > + */ > + WARN_ON_ONCE(order && > + (src_page != folio_page(page_folio(src_page), 0) || > + !(src_mpfn[i] & MIGRATE_PFN_COMPOUND))); > + > + if (order) > + gfp |= __GFP_NOWARN; > + > if (vas) > - folio = vma_alloc_folio(GFP_HIGHUSER, order, vas, addr); > + folio = vma_alloc_folio(gfp, order, vas, addr); > else > - folio = folio_alloc(GFP_HIGHUSER, order); > + folio = folio_alloc(gfp, order); > + > + if (!folio && order) { > + /* > + * Higher-order allocation failed, fall back to > + * order-0 allocations for the entire range covered > + * by the original higher-order allocation, without > + * setting MIGRATE_PFN_COMPOUND, until we move past > + * that range. > + */ > + unsigned long nr = NR_PAGES(order); > + unsigned long j; > + > + gfp &= ~__GFP_NOWARN; > + for (j = 0; j < nr && i < npages; j++, i++, addr += PAGE_SIZE) { > + folio = vas ? > + vma_alloc_folio(gfp, 0, vas, addr) : > + folio_alloc(gfp, 0); > + if (!folio) > + goto free_pages; > + > + page = folio_page(folio, 0); > + mpfn[i] = migrate_pfn(page_to_pfn(page)); > + } > + continue; > + } > > if (!folio) > goto free_pages; > @@ -940,11 +1033,11 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct vm_area_struct *vas, > if (!page) > goto next_put; > > + order = folio_order(page_folio(page)); > + > put_page(page); > mpfn[i] = 0; > > - order = folio_order(page_folio(page)); > - > next_put: > i += NR_PAGES(order); > } > @@ -1120,7 +1213,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation) > unsigned long *src, *dst; > struct drm_pagemap_addr *pagemap_addr; > void *buf; > - int i, err = 0; > + int err = 0; > unsigned int retry_count = 2; > > npages = devmem_allocation->size >> PAGE_SHIFT; > @@ -1160,15 +1253,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation) > if (err) > goto err_finalize; > > - for (i = 0; i < npages;) { > - unsigned int order = 0; > - > - pages[i] = migrate_pfn_to_page(src[i]); > - if (pages[i]) > - order = folio_order(page_folio(pages[i])); > - > - i += NR_PAGES(order); > - } > + drm_pagemap_migrate_populate_src_pages(pages, src, dst, npages); > > err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL); > if (err) > @@ -1235,7 +1320,7 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas, > struct drm_pagemap_addr *pagemap_addr; > unsigned long start, end; > void *buf; > - int i, err = 0; > + int err = 0; > > zdd = drm_pagemap_page_zone_device_data(page); > if (time_before64(get_jiffies_64(), zdd->devmem_allocation->timeslice_expiration)) > @@ -1290,15 +1375,8 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas, > if (err) > goto err_finalize; > > - for (i = 0; i < npages;) { > - unsigned int order = 0; > - > - pages[i] = migrate_pfn_to_page(migrate.src[i]); > - if (pages[i]) > - order = folio_order(page_folio(pages[i])); > - > - i += NR_PAGES(order); > - } > + drm_pagemap_migrate_populate_src_pages(pages, migrate.src, migrate.dst, > + LGTM Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> npages); > > err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL); > if (err) ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-16 15:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260805231041.3791771-1-matthew.brost@intel.com>
[not found] ` <20260805231041.3791771-3-matthew.brost@intel.com>
2026-08-05 23:29 ` [PATCH v3 2/6] mm/migrate_device: Do not write past the end of the src_pfns array Balbir Singh
[not found] ` <20260805231041.3791771-4-matthew.brost@intel.com>
2026-08-06 8:10 ` [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio Balbir Singh
2026-08-10 2:26 ` Huang, Ying
2026-08-10 19:43 ` Matthew Brost
2026-08-12 8:20 ` Huang, Ying
2026-08-12 23:33 ` Matthew Brost
2026-08-13 1:54 ` Huang, Ying
2026-08-13 8:33 ` Matthew Brost
[not found] ` <20260805231041.3791771-5-matthew.brost@intel.com>
2026-08-16 15:04 ` [PATCH v3 4/6] drm/pagemap: dma-unmap pages before handling migration errors Ghimiray, Himal Prasad
[not found] ` <20260805231041.3791771-6-matthew.brost@intel.com>
2026-08-16 15:36 ` [PATCH v3 5/6] drm/pagemap: Fix folio allocation fallback and use-after-put Ghimiray, Himal Prasad
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox