[v2,3/5] mm: eliminate adj_start parameter from commit_merge()

Commit Message

Lorenzo Stoakes Jan. 29, 2025, 6:16 p.m. UTC

Introduce internal vmg->__adjust_middle_start and vmg->__adjust_next_start
merge flags, enabling us to indicate to commit_merge() that we are
performing a merge which either spans only part of vmg->middle, or part of
vmg->next respectively.

In the former instance, we change the start of vmg->middle to match the
attributes of vmg->prev, without spanning all of vmg->middle.

This implies that vmg->prev->vm_end and vmg->middle->vm_start are both
increased to form the new merged VMA (vmg->prev) and the new subsequent VMA
(vmg->middle).

In the latter case, we change the end of vmg->middle to match the
attributes of vmg->next, without spanning all of vmg->next.

This implies that vmg->middle->vm_end and vmg->next->vm_start are both
decreased to form the new merged VMA (vmg->next) and the new prior VMA
(vmg->middle).

Since we now have a stable set of prev, middle, next VMAs threaded through
vmg and with these flags set know what is happening, we can perform
the calculation in commit_merge() instead.

This allows us to drop the confusing adj_start parameter and instead pass
semantic information to commit_merge().

In the latter case the -(middle->vm_end - start) calculation becomes
-(middle->vm-end - vmg->end), however this is correct as vmg->end is set to
the start parameter.

This is because in this case (rather confusingly), we manipulate
vmg->middle, but ultimately return vmg->next, whose range will be correctly
specified. At this point vmg->start, end is the new range for the prior VMA
rather than the merged one.

This patch has no change in functional behaviour.

Signed-off-by: Lorenzo Stoakes <[e-mail]>
Reviewed-by: Vlastimil Babka <[e-mail]>
---
 mm/debug.c              |  8 +++++--
 mm/vma.c                | 50 ++++++++++++++++++++++++-----------------
 mm/vma.h                | 10 +++++++++
 tools/testing/vma/vma.c |  2 ++
 4 files changed, 48 insertions(+), 22 deletions(-)

--
2.48.1

Patch

diff --git a/mm/debug.c b/mm/debug.c
index 92936908ed0f..adddfd69dbf7 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -267,7 +267,9 @@  void dump_vmg(const struct vma_merge_struct *vmg, const char *reason)
 		"uffd_ctx %px\n"
 		"anon_name %px\n"
 		"state %x\n"
-		"just_expand %b __remove_middle %b __remove_next %b\n",
+		"just_expand %b\n"
+		"__adjust_middle_start %b __adjust_next_start %b\n"
+		"__remove_middle %b __remove_next %b\n",
 		vmg, vmg->mm, vmg->pgoff,
 		vmg->vmi, vmg->vmi ? vma_iter_addr(vmg->vmi) : 0,
 		vmg->vmi ? vma_iter_end(vmg->vmi) : 0,
@@ -281,7 +283,9 @@  void dump_vmg(const struct vma_merge_struct *vmg, const char *reason)
 #endif
 		vmg->anon_name,
 		(int)vmg->state,
-		vmg->just_expand, vmg->__remove_middle, vmg->__remove_next);
+		vmg->just_expand,
+		vmg->__adjust_middle_start, vmg->__adjust_next_start,
+		vmg->__remove_middle, vmg->__remove_next);

 	if (vmg->mm) {
 		pr_warn("vmg %px mm:\n", vmg);
diff --git a/mm/vma.c b/mm/vma.c
index 206e3523cf38..3fe264200b79 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -632,29 +632,44 @@  void validate_mm(struct mm_struct *mm)
  *
  * On success, returns the merged VMA. Otherwise returns NULL.
  */
-static struct vm_area_struct *commit_merge(struct vma_merge_struct *vmg,
-		long adj_start)
+static struct vm_area_struct *commit_merge(struct vma_merge_struct *vmg)
 {
-	struct vma_prepare vp;
 	struct vm_area_struct *remove = NULL;
 	struct vm_area_struct *remove2 = NULL;
+	struct vma_prepare vp;
 	struct vm_area_struct *adjust = NULL;
+	long adj_start;
+	bool merge_target;
+
 	/*
-	 * In all cases but that of merge right, shrink next, we write
-	 * vmg->target to the maple tree and return this as the merged VMA.
+	 * If modifying an existing VMA and we don't remove vmg->middle, then we
+	 * shrink the adjacent VMA.
 	 */
-	bool merge_target = adj_start >= 0;
+	if (vmg->__adjust_middle_start) {
+		adjust = vmg->middle;
+		/* The POSITIVE value by which we offset vmg->middle->vm_start. */
+		adj_start = vmg->end - vmg->middle->vm_start;
+		merge_target = true;
+	} else if (vmg->__adjust_next_start) {
+		adjust = vmg->next;
+		/* The NEGATIVE value by which we offset vmg->next->vm_start. */
+		adj_start = -(vmg->middle->vm_end - vmg->end);
+		/*
+		 * In all cases but this - merge right, shrink next - we write
+		 * vmg->target to the maple tree and return this as the merged VMA.
+		 */
+		merge_target = false;
+	} else {
+		adjust = NULL;
+		adj_start = 0;
+		merge_target = true;
+	}

 	if (vmg->__remove_middle)
 		remove = vmg->middle;
 	if (vmg->__remove_next)
 		remove2 = vmg->next;

-	if (adj_start > 0)
-		adjust = vmg->middle;
-	else if (adj_start < 0)
-		adjust = vmg->next;
-
 	init_multi_vma_prep(&vp, vmg->target, adjust, remove, remove2);

 	VM_WARN_ON(vp.anon_vma && adjust && adjust->anon_vma &&
@@ -738,7 +753,6 @@  static __must_check struct vm_area_struct *vma_merge_existing_range(
 	bool left_side = middle && start == middle->vm_start;
 	bool right_side = middle && end == middle->vm_end;
 	int err = 0;
-	long adj_start = 0;
 	bool merge_left, merge_right, merge_both;

 	mmap_assert_write_locked(vmg->mm);
@@ -858,11 +872,8 @@  static __must_check struct vm_area_struct *vma_merge_existing_range(
 		vmg->start = prev->vm_start;
 		vmg->pgoff = prev->vm_pgoff;

-		/*
-		 * We both expand prev and shrink middle.
-		 */
 		if (!vmg->__remove_middle)
-			adj_start = vmg->end - middle->vm_start;
+			vmg->__adjust_middle_start = true;

 		err = dup_anon_vma(prev, middle, &anon_dup);
 	} else { /* merge_right */
@@ -891,12 +902,11 @@  static __must_check struct vm_area_struct *vma_merge_existing_range(
 			 * IMPORTANT: This is the ONLY case where the final
 			 * merged VMA is NOT vmg->target, but rather vmg->next.
 			 */
+			vmg->__adjust_next_start = true;
 			vmg->target = middle;
 			vmg->start = middle->vm_start;
 			vmg->end = start;
 			vmg->pgoff = middle->vm_pgoff;
-
-			adj_start = -(middle->vm_end - start);
 		}

 		err = dup_anon_vma(next, middle, &anon_dup);
@@ -905,7 +915,7 @@  static __must_check struct vm_area_struct *vma_merge_existing_range(
 	if (err)
 		goto abort;

-	res = commit_merge(vmg, adj_start);
+	res = commit_merge(vmg);
 	if (!res) {
 		if (anon_dup)
 			unlink_anon_vmas(anon_dup);
@@ -1079,7 +1089,7 @@  int vma_expand(struct vma_merge_struct *vmg)
 	if (remove_next)
 		vmg->__remove_next = true;

-	if (!commit_merge(vmg, 0))
+	if (!commit_merge(vmg))
 		goto nomem;

 	return 0;
diff --git a/mm/vma.h b/mm/vma.h
index 7935681a2db8..e18487797fa4 100644
--- a/mm/vma.h
+++ b/mm/vma.h
@@ -120,6 +120,16 @@  struct vma_merge_struct {

 	/* Internal flags set during merge process: */

+	/*
+	 * Internal flag indicating the merge increases vmg->middle->vm_start
+	 * (and thereby, vmg->prev->vm_end).
+	 */
+	bool __adjust_middle_start :1;
+	/*
+	 * Internal flag indicating the merge decreases vmg->next->vm_start
+	 * (and thereby, vmg->middle->vm_end).
+	 */
+	bool __adjust_next_start :1;
 	/*
 	 * Internal flag used during the merge operation to indicate we will
 	 * remove vmg->middle.
diff --git a/tools/testing/vma/vma.c b/tools/testing/vma/vma.c
index 7728498b2f7e..c7ffa71841ca 100644
--- a/tools/testing/vma/vma.c
+++ b/tools/testing/vma/vma.c
@@ -159,6 +159,8 @@  static void vmg_set_range(struct vma_merge_struct *vmg, unsigned long start,
 	vmg->just_expand = false;
 	vmg->__remove_middle = false;
 	vmg->__remove_next = false;
+	vmg->__adjust_middle_start = false;
+	vmg->__adjust_next_start = false;
 }

 /*