Skip to contentSkip to conversation

mm/damon/vaddr: attempt per-vma lock during page table walk

PATCH Applied Kefeng Wang 3 participants 5 messages May 12 – May 14

Subsystems (2)

v2 of 3 Applied 1/1 Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 mm/damon/vaddr: attempt per-vma lock during page table walk

Currently, DAMON virtual address operations use mmap_read_lock during page table walks, which can cause unnecessary contention under high concurrency.

Introduce damon_va_walk_page_range() to first attempt acquiring a per-vma lock. If the VMA is found and the range is fully contained within it, the page table walk proceeds with the per-vma lock instead of mmap_read_lock.

This optimization is particularly effective for damon_va_young() and damon_va_mkold(), which are frequently called and typically operate within a single VMA.

---
v2: avoid handling VMAs with the VM_PFNMAP for per-vma path, found by
Sashiko review.
v1: https://lore.kernel.org/linux-mm/[email protected]/

 mm/damon/vaddr.c | 69 ++++++++++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 26 deletions(-)
mm/damon/vaddr.c +43 -26
index 1b0ebe3b6951..d27147603564 100644
@@ -237,6 +237,35 @@ static void damon_va_update(struct damon_ctx *ctx)
 	}
 }
 
+static void damon_va_walk_page_range(struct mm_struct *mm, unsigned long start,
+		unsigned long end, struct mm_walk_ops *ops, void *private)
+{
+	struct vm_area_struct *vma;
+
+	vma = lock_vma_under_rcu(mm, start);
+	if (!vma)
+		goto lock_mmap;
+
+	if (end > vma->vm_end) {
+		vma_end_read(vma);
+		goto lock_mmap;
+	}
+
+	if (!(vma->vm_flags & VM_PFNMAP)) {
+		ops->walk_lock = PGWALK_VMA_RDLOCK_VERIFY;
+		walk_page_range_vma(vma, start, end, ops, private);
+	}
+
+	vma_end_read(vma);
+	return;
+
+lock_mmap:
+	mmap_read_lock(mm);
+	ops->walk_lock = PGWALK_RDLOCK;
+	walk_page_range(mm, start, end, ops, private);
+	mmap_read_unlock(mm);
+}
+
 static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr,
 		unsigned long next, struct mm_walk *walk)
 {
@@ -315,17 +344,14 @@ static int damon_mkold_hugetlb_entry(pte_t *pte, unsigned long hmask,
 #define damon_mkold_hugetlb_entry NULL
 #endif /* CONFIG_HUGETLB_PAGE */
 
-static const struct mm_walk_ops damon_mkold_ops = {
-	.pmd_entry = damon_mkold_pmd_entry,
-	.hugetlb_entry = damon_mkold_hugetlb_entry,
-	.walk_lock = PGWALK_RDLOCK,
-};
-
 static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
 {
-	mmap_read_lock(mm);
-	walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
-	mmap_read_unlock(mm);
+	struct mm_walk_ops damon_mkold_ops = {
+		.pmd_entry = damon_mkold_pmd_entry,
+		.hugetlb_entry = damon_mkold_hugetlb_entry,
+	};
+
+	damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
 }
 
 /*
@@ -445,12 +471,6 @@ static int damon_young_hugetlb_entry(pte_t *pte, unsigned long hmask,
 #define damon_young_hugetlb_entry NULL
 #endif /* CONFIG_HUGETLB_PAGE */
 
-static const struct mm_walk_ops damon_young_ops = {
-	.pmd_entry = damon_young_pmd_entry,
-	.hugetlb_entry = damon_young_hugetlb_entry,
-	.walk_lock = PGWALK_RDLOCK,
-};
-
 static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
 		unsigned long *folio_sz)
 {
@@ -459,9 +479,12 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
 		.young = false,
 	};
 
-	mmap_read_lock(mm);
-	walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
-	mmap_read_unlock(mm);
+	struct mm_walk_ops damon_young_ops = {
+		.pmd_entry = damon_young_pmd_entry,
+		.hugetlb_entry = damon_young_hugetlb_entry,
+	};
+
+	damon_va_walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
 	return arg.young;
 }
 
@@ -750,7 +773,6 @@ static unsigned long damos_va_migrate(struct damon_target *target,
 	struct mm_walk_ops walk_ops = {
 		.pmd_entry = damos_va_migrate_pmd_entry,
 		.pte_entry = NULL,
-		.walk_lock = PGWALK_RDLOCK,
 	};
 
 	use_target_nid = dests->nr_dests == 0;
@@ -768,9 +790,7 @@ static unsigned long damos_va_migrate(struct damon_target *target,
 	if (!mm)
 		goto free_lists;
 
-	mmap_read_lock(mm);
-	walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
-	mmap_read_unlock(mm);
+	damon_va_walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
 	mmput(mm);
 
 	for (int i = 0; i < nr_dests; i++) {
@@ -862,7 +882,6 @@ static unsigned long damos_va_stat(struct damon_target *target,
 	struct mm_struct *mm;
 	struct mm_walk_ops walk_ops = {
 		.pmd_entry = damos_va_stat_pmd_entry,
-		.walk_lock = PGWALK_RDLOCK,
 	};
 
 	priv.scheme = s;
@@ -875,9 +894,7 @@ static unsigned long damos_va_stat(struct damon_target *target,
 	if (!mm)
 		return 0;
 
-	mmap_read_lock(mm);
-	walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
-	mmap_read_unlock(mm);
+	damon_va_walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
 	mmput(mm);
 	return 0;
 }
Signature · 1 line
2.27.0
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] `damos_va_migrate_pmd_entry` and `damos_va_stat_pmd_entry` incorrectly advance iteration by the…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] damos_va_migrate_pmd_entry and damos_va_stat_pmd_entry incorrectly advance iteration by the full folio size, skipping unrelated PTEs for mTHP. --

commit 25c04a646a951de8c86c5c718af6cfc459b7b5e0
Author: Kefeng Wang <[email protected]>

mm/damon/vaddr: attempt per-vma lock during page table walk

Currently, DAMON virtual address operations use mmap_read_lock during page table walks, which can cause unnecessary contention under high concurrency.

Quoted 4 lines
mm/damon/vaddr.c +0 -0
index 1b0ebe3b6951e..d271476035641 100644

[ ... ]

mm/damon/vaddr.c static unsigned long damos_va_migrate(struct damon_target *target, · line 776

Quoted 6 lines
mm/damon/vaddr.c +0 -1
@@ -750,7 +773,6 @@ static unsigned long damos_va_migrate(struct damon_target *target,
 	struct mm_walk_ops walk_ops = {
 		.pmd_entry = damos_va_migrate_pmd_entry,
 		.pte_entry = NULL,
-		.walk_lock = PGWALK_RDLOCK,
 	};

This isn't a regression from this patch, but there appears to be a pre-existing issue in damos_va_migrate_pmd_entry() and damos_va_stat_pmd_entry() when handling large folios.

In the PTE iteration loops, when a large folio is found, the code sets nr = folio_nr_pages(folio) and updates pte += nr and addr += nr * PAGE_SIZE.

Could this skip unrelated PTEs if the iteration started on a tail page of this folio (e.g., halfway into an mTHP)?

Advancing by the full folio size could overshoot the end of the folio and skip unrelated PTEs mapped immediately after it within the same PMD.

Would it be better to subtract the offset within the folio to safely skip only the remaining pages of the current folio?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer
#2 lore raw
Quoted 18 lines · On Wed, 13 May 2026 22:52:03 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] damos_va_migrate_pmd_entry and damos_va_stat_pmd_entry incorrectly advance iteration by the full folio size, skipping unrelated PTEs for mTHP. --

commit 25c04a646a951de8c86c5c718af6cfc459b7b5e0
Author: Kefeng Wang <[email protected]>

mm/damon/vaddr: attempt per-vma lock during page table walk

Currently, DAMON virtual address operations use mmap_read_lock during page table walks, which can cause unnecessary contention under high concurrency.

Quoted 4 lines
mm/damon/vaddr.c +0 -0
index 1b0ebe3b6951e..d271476035641 100644

[ ... ]

Quoted 8 lines

mm/damon/vaddr.c static unsigned long damos_va_migrate(struct damon_target *target, · line 776

Quoted 6 lines
mm/damon/vaddr.c +0 -1
@@ -750,7 +773,6 @@ static unsigned long damos_va_migrate(struct damon_target *target,
 	struct mm_walk_ops walk_ops = {
 		.pmd_entry = damos_va_migrate_pmd_entry,
 		.pte_entry = NULL,
-		.walk_lock = PGWALK_RDLOCK,
 	};

This isn't a regression from this patch,

Yes, no blocker for this patch.

Quoted 7 lines

but there appears to be a pre-existing
issue in damos_va_migrate_pmd_entry() and damos_va_stat_pmd_entry() when handling large folios.

In the PTE iteration loops, when a large folio is found, the code sets nr = folio_nr_pages(folio) and updates pte += nr and addr += nr * PAGE_SIZE.

Quoted 8 lines

Could this skip unrelated PTEs if the iteration started on a tail page of this folio (e.g., halfway into an mTHP)?

Advancing by the full folio size could overshoot the end of the folio and skip unrelated PTEs mapped immediately after it within the same PMD.

Would it be better to subtract the offset within the folio to safely skip only the remaining pages of the current folio?

The user impact should be minor, but makes sense. I will work on this.

Thanks,
SJ

Quoted 3 lines
Signature · 2 lines
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer Reviewed-by on v2 1/1
#3 lore raw
In reply to Kefeng Wang
Quoted 4 lines · On Tue, 12 May 2026 23:15:23 +0800 Kefeng Wang <[email protected]>…

Currently, DAMON virtual address operations use mmap_read_lock during page table walks, which can cause unnecessary contention under high concurrency.

Quoted 8 lines

Introduce damon_va_walk_page_range() to first attempt acquiring a per-vma lock. If the VMA is found and the range is fully contained within it, the page table walk proceeds with the per-vma lock instead of mmap_read_lock.

This optimization is particularly effective for damon_va_young() and damon_va_mkold(), which are frequently called and typically operate within a single VMA.

Maybe this is not only making DAMON faster, but avoid DAMON blocking others.

And because we don't have performance measurements, how about making this sentence less assertive? E.g., "This optimization is particularly expected to be effective for ..."

I guess Andrew could make the adjustment when he picks this into mm.git. If not I will repost this with the trivial change if you don't mind.

Quoted 2 lines
Quoted 3 lines

---
v2: avoid handling VMAs with the VM_PFNMAP for per-vma path, found by
Sashiko review.

This patch helped Sashiko finding [1] yet another DAMON bug (impact is minor imho). The bug is orthogonal to this patch so there is no blocker for this patch.

Quoted 1 line

v1: https://lore.kernel.org/linux-mm/[email protected]/

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

Thanks,
SJ

[...]

#4 lore raw
Quoted 11 lines · SeongJae Park
Quoted 9 lines · On Tue, 12 May 2026 23:15:23 +0800 Kefeng Wang <[email protected]>…

Currently, DAMON virtual address operations use mmap_read_lock during page table walks, which can cause unnecessary contention under high concurrency.

Introduce damon_va_walk_page_range() to first attempt acquiring a per-vma lock. If the VMA is found and the range is fully contained within it, the page table walk proceeds with the per-vma lock instead of mmap_read_lock.

Quoted 6 lines
Quoted 3 lines

This optimization is particularly effective for damon_va_young() and damon_va_mkold(), which are frequently called and typically operate within a single VMA.

Maybe this is not only making DAMON faster, but avoid DAMON blocking others.

Yes, this is exactly what per-vma is for.

Quoted 3 lines

And because we don't have performance measurements, how about making this sentence less assertive? E.g., "This optimization is particularly expected to be effective for ..."

Fine to me.

Quoted 5 lines

I guess Andrew could make the adjustment when he picks this into mm.git. If not I will repost this with the trivial change if you don't mind.

Quoted 1 line

Sure

Quoted 4 lines
Quoted 1 line

Thanks

Quoted 18 lines
Quoted 3 lines

---
v2: avoid handling VMAs with the VM_PFNMAP for per-vma path, found by
Sashiko review.

This patch helped Sashiko finding [1] yet another DAMON bug (impact is minor imho). The bug is orthogonal to this patch so there is no blocker for this patch.

Quoted 1 line

v1: https://lore.kernel.org/linux-mm/[email protected]/

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

Thanks,
SJ

[...]