Skip to contentSkip to conversation

mm: handle device-private PMDs in walk callbacks

PATCH Superseded Usama Arif 3 participants 8 messages Jul 8

Subsystems (5)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Usama Arif

author reviewer
#0 lore raw

v2 cover mm: handle device-private PMDs in walk callbacks

I have followed Joshua's suggestion in [1] for v2. The patches look different but acheive the same goal, so I have kept the Reviewed-by tags from v1. Joshua, Zi and Balbir please let me know if this wasn't ok!

Since commit 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations") a PMD may hold a device-private swap entry whenever an HMM-based GPU driver migrates an anonymous THP folio to device memory via migrate_vma_pages().

pmd_trans_huge_lock() succeeds for such PMDs (pmd_is_huge() returns true for any non-present, non-none huge PMD), so several MM walk callbacks that used to assume present THP or migration entry are now reachable with a device-private PMD. The results range from a VM_BUG_ON() firing on debug kernels, to an oops on a bogus vmemmap dereference, to silently isolating an unrelated live folio from LRU in the aliasing case.

[1] https://lore.kernel.org/all/[email protected]/

v1 -> v2: https://lore.kernel.org/all/[email protected]/
- Patch 1 now gates queue_folios_pmd() on !pmd_present() instead of checking only pmd_is_device_private_entry(). This matches the PTE path, keeps migration entries counted in qp->nr_failed, and skips other non-present PMDs such as device-private entries before calling pmd_folio(). (Joshua)
- Patches 2 and 3 now fold device-private PMD handling into the existing !pmd_present() VM_BUG_ON() condition instead of using a separate early pmd_is_device_private_entry() check. (Joshua)
- cc stable (Zi)

Usama Arif (3):
mm/mempolicy: skip non-present PMDs when queueing folios mm/madvise: skip device-private PMDs in cold and pageout walks mm/huge_memory: skip device-private PMDs in madvise_free_huge_pmd

 mm/huge_memory.c | 3 ++-
 mm/madvise.c     | 3 ++-
 mm/mempolicy.c   | 8 +++++---
 3 files changed, 9 insertions(+), 5 deletions(-)
Signature · 1 line
2.53.0-Meta
author reviewer
#1 lore raw

PATCH v2 1/3 mm/mempolicy: skip non-present PMDs when queueing folios

queue_folios_pmd() is called under pmd_trans_huge_lock(), whose pmd_is_huge() check returns true for any non-present, non-none PMD softleaf. Passing such a PMD to pmd_folio() treats the softleaf encoding as a hardware PFN and can return a bogus folio pointer.

Mirror queue_folios_pte_range(): handle non-present entries before looking up a folio. Keep migration entries counted as failures, but skip other non-present PMDs such as device-private entries.

Potential trigger: an HMM-based GPU driver migrates an anonymous THP folio to device memory via migrate_vma_pages(), leaving a device-private PMD. Userspace then calls mbind(), migrate_pages() or set_mempolicy_home_node() on that range.

Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")
---
 mm/mempolicy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
mm/mempolicy.c +5 -3
index 914f81863db5..4785b55c02da 100644
@@ -654,12 +654,14 @@ static void queue_folios_pmd(pmd_t *pmd, struct mm_walk *walk)
 {
 	struct folio *folio;
 	struct queue_pages *qp = walk->private;
+	pmd_t pmdval = *pmd;
 
-	if (unlikely(pmd_is_migration_entry(*pmd))) {
-		qp->nr_failed++;
+	if (unlikely(!pmd_present(pmdval))) {
+		if (pmd_is_migration_entry(pmdval))
+			qp->nr_failed++;
 		return;
 	}
-	folio = pmd_folio(*pmd);
+	folio = pmd_folio(pmdval);
 	if (is_huge_zero_folio(folio)) {
 		walk->action = ACTION_CONTINUE;
 		return;
Signature · 1 line
2.53.0-Meta
maintainer Acked-by on v2 1/3
#2 lore raw

Re: [PATCH v2 1/3] mm/mempolicy: skip non-present PMDs when queueing folios

Quoted 14 lines · Usama Arif

queue_folios_pmd() is called under pmd_trans_huge_lock(), whose pmd_is_huge() check returns true for any non-present, non-none PMD softleaf. Passing such a PMD to pmd_folio() treats the softleaf encoding as a hardware PFN and can return a bogus folio pointer.

Mirror queue_folios_pte_range(): handle non-present entries before looking up a folio. Keep migration entries counted as failures, but skip other non-present PMDs such as device-private entries.

Potential trigger: an HMM-based GPU driver migrates an anonymous THP folio to device memory via migrate_vma_pages(), leaving a device-private PMD. Userspace then calls mbind(), migrate_pages() or set_mempolicy_home_node() on that range.

Quoted 7 lines
Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")

---

Signature · 3 lines
Cheers,

David
reviewer Reviewed-by on v2 1/3
#3 lore raw

Re: [PATCH v2 1/3] mm/mempolicy: skip non-present PMDs when queueing folios

In reply to Usama Arif
Quoted 29 lines · Usama Arif

queue_folios_pmd() is called under pmd_trans_huge_lock(), whose pmd_is_huge() check returns true for any non-present, non-none PMD softleaf. Passing such a PMD to pmd_folio() treats the softleaf encoding as a hardware PFN and can return a bogus folio pointer.

Mirror queue_folios_pte_range(): handle non-present entries before looking up a folio. Keep migration entries counted as failures, but skip other non-present PMDs such as device-private entries.

Potential trigger: an HMM-based GPU driver migrates an anonymous THP folio to device memory via migrate_vma_pages(), leaving a device-private PMD. Userspace then calls mbind(), migrate_pages() or set_mempolicy_home_node() on that range.

Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")
---
 mm/mempolicy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
mm/mempolicy.c +0 -0
index 914f81863db5..4785b55c02da 100644

mm/mempolicy.c static void queue_folios_pmd(pmd_t *pmd, struct mm_walk *walk) · line 657

Quoted 4 lines
mm/mempolicy.c +1 -0
@@ -654,12 +654,14 @@ static void queue_folios_pmd(pmd_t *pmd, struct mm_walk *walk)
 {
 	struct folio *folio;
 	struct queue_pages *qp = walk->private;
+	pmd_t pmdval = *pmd;

Use pmdp_get() instead?

Quoted 15 lines
mm/mempolicy.c +4 -3
-	if (unlikely(pmd_is_migration_entry(*pmd))) {
-		qp->nr_failed++;
+	if (unlikely(!pmd_present(pmdval))) {
+		if (pmd_is_migration_entry(pmdval))
+			qp->nr_failed++;
 		return;
 	}
-	folio = pmd_folio(*pmd);
+	folio = pmd_folio(pmdval);
 	if (is_huge_zero_folio(folio)) {
 		walk->action = ACTION_CONTINUE;
 		return;
Signature · 1 line
2.53.0-Meta

Otherwise, LGTM.

Best Regards,
Yan, Zi

author reviewer
#4 lore raw

PATCH v2 2/3 mm/madvise: skip device-private PMDs in cold and pageout walks

In reply to Usama Arif

madvise_cold_or_pageout_pte_range() takes pmd_trans_huge_lock(), whose pmd_is_huge() check returns true for a device-private PMD. The subsequent !pmd_present() branch has a VM_BUG_ON() asserting migration is the only allowed non-present case; a device-private PMD trips it.

Allow device-private PMDs in that non-present assertion and continue to huge_unlock before calling pmd_folio(). This keeps the assertion for unexpected PMD softleafs while skipping device-private PMDs like other non-present PMDs in this path.

Potential trigger: an HMM-based GPU driver races with madvise(MADV_COLD)/MADV_PAGEOUT: pmd_trans_huge(*pmd) reads true, then migrate_vma_pages() flips the PMD to a device-private entry before the PMD lock is acquired.

---
 mm/madvise.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/madvise.c +2 -1
index 9292f60b19aa..1065b5a84ea7 100644
@@ -389,7 +389,8 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 
 		if (unlikely(!pmd_present(orig_pmd))) {
 			VM_BUG_ON(thp_migration_supported() &&
-					!pmd_is_migration_entry(orig_pmd));
+					!pmd_is_migration_entry(orig_pmd) &&
+					!pmd_is_device_private_entry(orig_pmd));
 			goto huge_unlock;
 		}
 
Signature · 1 line
2.53.0-Meta
#5 lore raw

Re: [PATCH v2 2/3] mm/madvise: skip device-private PMDs in cold and pageout walks

Quoted 32 lines · Usama Arif

madvise_cold_or_pageout_pte_range() takes pmd_trans_huge_lock(), whose pmd_is_huge() check returns true for a device-private PMD. The subsequent !pmd_present() branch has a VM_BUG_ON() asserting migration is the only allowed non-present case; a device-private PMD trips it.

Allow device-private PMDs in that non-present assertion and continue to huge_unlock before calling pmd_folio(). This keeps the assertion for unexpected PMD softleafs while skipping device-private PMDs like other non-present PMDs in this path.

Potential trigger: an HMM-based GPU driver races with madvise(MADV_COLD)/MADV_PAGEOUT: pmd_trans_huge(*pmd) reads true, then migrate_vma_pages() flips the PMD to a device-private entry before the PMD lock is acquired.

---
 mm/madvise.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/madvise.c +0 -0
index 9292f60b19aa..1065b5a84ea7 100644

mm/madvise.c static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, · line 393

Quoted 6 lines
mm/madvise.c +2 -1
@@ -389,7 +389,8 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 
 		if (unlikely(!pmd_present(orig_pmd))) {
 			VM_BUG_ON(thp_migration_supported() &&
-					!pmd_is_migration_entry(orig_pmd));
+					!pmd_is_migration_entry(orig_pmd) &&
+					!pmd_is_device_private_entry(orig_pmd));

thp_migration_supported() is just effectively CONFIG_TRANSPARENT_HUGEPAGE && CONFIG_ARCH_HAS_PMD_SOFTLEAVES.

pmd_is_migration_entry() -> softleaf_is_migration(softleaf_from_pmd(pmd)) == false without CONFIG_ARCH_HAS_PMD_SOFTLEAVES. ... and CONFIG_ARCH_HAS_PMD_SOFTLEAVES is only selected if TRANSPARENT_HUGEPAGE.

While at it, can you drop the thp_migration_supported() check and turn the VM_BUG_ON into a VM_WARN_ON_ONCE() ?

Signature · 3 lines
Cheers,

David
author reviewer
#6 lore raw

PATCH v2 3/3 mm/huge_memory: skip device-private PMDs in madvise_free_huge_pmd

In reply to Usama Arif

madvise_free_pte_range() checks pmd_trans_huge(*pmd) unlocked, then madvise_free_huge_pmd() takes pmd_trans_huge_lock(). pmd_is_huge() returns true for a device-private PMD, so orig_pmd can be device-private and enter the !pmd_present() branch.

Allow device-private PMDs in that non-present assertion and continue to out before calling pmd_folio(). This keeps the assertion for unexpected PMD softleafs while skipping device-private PMDs like other non-present PMDs in this path.

Potential trigger: an HMM-based GPU driver races with madvise(MADV_FREE):
migrate_vma_pages() flips the PMD to a device-private entry between the caller's pmd_trans_huge() check and the callee's pmd_trans_huge_lock().

Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")
---
 mm/huge_memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/huge_memory.c +2 -1
index c0892cc533a9..ddbdc83b4cae 100644
@@ -2298,7 +2298,8 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 
 	if (unlikely(!pmd_present(orig_pmd))) {
 		VM_BUG_ON(thp_migration_supported() &&
-				  !pmd_is_migration_entry(orig_pmd));
+				  !pmd_is_migration_entry(orig_pmd) &&
+				  !pmd_is_device_private_entry(orig_pmd));
 		goto out;
 	}
 
Signature · 1 line
2.53.0-Meta
#7 lore raw

Re: [PATCH v2 3/3] mm/huge_memory: skip device-private PMDs in madvise_free_huge_pmd

Quoted 29 lines · Usama Arif

madvise_free_pte_range() checks pmd_trans_huge(*pmd) unlocked, then madvise_free_huge_pmd() takes pmd_trans_huge_lock(). pmd_is_huge() returns true for a device-private PMD, so orig_pmd can be device-private and enter the !pmd_present() branch.

Allow device-private PMDs in that non-present assertion and continue to out before calling pmd_folio(). This keeps the assertion for unexpected PMD softleafs while skipping device-private PMDs like other non-present PMDs in this path.

Potential trigger: an HMM-based GPU driver races with madvise(MADV_FREE):
migrate_vma_pages() flips the PMD to a device-private entry between the caller's pmd_trans_huge() check and the callee's pmd_trans_huge_lock().

Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")
---
 mm/huge_memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/huge_memory.c +0 -0
index c0892cc533a9..ddbdc83b4cae 100644

mm/huge_memory.c bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, · line 2302

Quoted 6 lines
mm/huge_memory.c +2 -1
@@ -2298,7 +2298,8 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 
 	if (unlikely(!pmd_present(orig_pmd))) {
 		VM_BUG_ON(thp_migration_supported() &&
-				  !pmd_is_migration_entry(orig_pmd));
+				  !pmd_is_migration_entry(orig_pmd) &&
+				  !pmd_is_device_private_entry(orig_pmd));

Same comment as to #2.

Signature · 3 lines
Cheers,

David