[PATCH] mm/damon/vaddr: drop redundant mmap_read_lock from damon_va_three_regions

[PATCH] mm/damon/vaddr: drop redundant mmap_read_lock from damon_va_three_regions

[Cunlong Li]

__damon_va_three_regions() already walks VMAs under rcu_read_lock(), so
the outer mmap_read_lock() is no longer needed.

---
Signed-off-by: Cunlong Li <shenxiaogll@gmail.com>
---
 mm/damon/vaddr.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index b069dbc7e3d2..27e34596c43b 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -77,7 +77,7 @@ static int __damon_va_three_regions(struct mm_struct *mm,
 	struct damon_addr_range first_gap = {0}, second_gap = {0};
 	VMA_ITERATOR(vmi, mm, 0);
 	struct vm_area_struct *vma, *prev = NULL;
-	unsigned long start;
+	unsigned long start = 0, last_vma_end = 0;
 
 	/*
 	 * Find the two biggest gaps so that first_gap > second_gap > others.
@@ -104,6 +104,7 @@ static int __damon_va_three_regions(struct mm_struct *mm,
 		}
 next:
 		prev = vma;
+		last_vma_end = vma->vm_end;
 	}
 	rcu_read_unlock();
 
@@ -120,7 +121,7 @@ static int __damon_va_three_regions(struct mm_struct *mm,
 	regions[1].start = ALIGN(first_gap.end, DAMON_MIN_REGION_SZ);
 	regions[1].end = ALIGN(second_gap.start, DAMON_MIN_REGION_SZ);
 	regions[2].start = ALIGN(second_gap.end, DAMON_MIN_REGION_SZ);
-	regions[2].end = ALIGN(prev->vm_end, DAMON_MIN_REGION_SZ);
+	regions[2].end = ALIGN(last_vma_end, DAMON_MIN_REGION_SZ);
 
 	return 0;
 }
@@ -140,9 +141,7 @@ static int damon_va_three_regions(struct damon_target *t,
 	if (!mm)
 		return -EINVAL;
 
-	mmap_read_lock(mm);
 	rc = __damon_va_three_regions(mm, regions);
-	mmap_read_unlock(mm);
 
 	mmput(mm);
 	return rc;

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260607-rcu-24b324e0f076

Best regards,
-- 
Cunlong Li <shenxiaogll@gmail.com>

Re: [PATCH] mm/damon/vaddr: drop redundant mmap_read_lock from damon_va_three_regions

[SeongJae Park]

Hello Cunlong,

On Sun, 07 Jun 2026 21:48:52 +0800 Cunlong Li <shenxiaogll@gmail.com> wrote:

> __damon_va_three_regions() already walks VMAs under rcu_read_lock(), so
> the outer mmap_read_lock() is no longer needed.

The function aims to read a snapshot of the virtual address space.  So I think
mmap_read_lock() is still required.  Please let me know if I'm missing
something.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/vaddr: drop redundant mmap_read_lock from damon_va_three_regions

[Cunlong Li]

On Sun, Jun 07, 2026 at 10:36:44AM -0700, SeongJae Park wrote:
> Hello Cunlong,
> 
> On Sun, 07 Jun 2026 21:48:52 +0800 Cunlong Li <shenxiaogll@gmail.com> wrote:
> 
> > __damon_va_three_regions() already walks VMAs under rcu_read_lock(), so
> > the outer mmap_read_lock() is no longer needed.
> 
> The function aims to read a snapshot of the virtual address space.  So I think
> mmap_read_lock() is still required.  Please let me know if I'm missing
> something.
> 
> 
> Thanks,
> SJ
> 
> [...]

Yes — I understand this path is meant to obtain a snapshot of the VMA layout.

My thinking was that an exact snapshot may not be required here, and
the RCU-protected traversal inside __damon_va_three_regions() already
makes the walk safe, so the outer mmap_read_lock() might be redundant.

But, if we want a consistent VMA layout here, mmap_read_lock() is the
right mechanism and rcu_read_lock() alone is not enough.  I'll drop
the patch :)

Thanks,
Cunlong