mm: completely abstract unnecessary adj_start calculation

The adj_start calculation has been a constant source of confusion in the
VMA merge code.

There are two cases to consider, one where we adjust the start of the
vmg->middle VMA (i.e.  the vmg->__adjust_middle_start merge flag is set),
in which case adj_start is calculated as:

(1) adj_start = vmg->end - vmg->middle->vm_start

And the case where we adjust the start of the vmg->next VMA (i.e.  the
vmg->__adjust_next_start merge flag is set), in which case adj_start is
calculated as:

(2) adj_start = -(vmg->middle->vm_end - vmg->end)

We apply (1) thusly:

vmg->middle->vm_start =
	vmg->middle->vm_start + vmg->end - vmg->middle->vm_start

Which simplifies to:

vmg->middle->vm_start = vmg->end

Similarly, we apply (2) as:

vmg->next->vm_start =
	vmg->next->vm_start + -(vmg->middle->vm_end - vmg->end)

Noting that for these VMAs to be mergeable vmg->middle->vm_end ==
vmg->next->vm_start and so this simplifies to:

vmg->next->vm_start =
	vmg->next->vm_start + -(vmg->next->vm_start - vmg->end)

Which simplifies to:

vmg->next->vm_start = vmg->end

Therefore in each case, we simply need to adjust the start of the VMA to
vmg->end (!) and can do away with this adj_start calculation.  The only
caveat is that we must ensure we update the vm_pgoff field correctly.

We therefore abstract this entire calculation to a new function
vmg_adjust_set_range() which performs this calculation and sets the
adjusted VMA's new range using the general vma_set_range() function.

We also must update vma_adjust_trans_huge() which expects the
now-abstracted adj_start parameter.  It turns out this is wholly
unnecessary.

In vma_adjust_trans_huge() the relevant code is:

	if (adjust_next > 0) {
		struct vm_area_struct *next = find_vma(vma->vm_mm, vma->vm_end);
		unsigned long nstart = next->vm_start;
		nstart += adjust_next;
		split_huge_pmd_if_needed(next, nstart);
	}

The only case where this is relevant is when vmg->__adjust_middle_start is
specified (in which case adj_next would have been positive), i.e.  the one
in which the vma specified is vmg->prev and this the sought 'next' VMA
would be vmg->middle.

We can therefore eliminate the find_vma() invocation altogether and simply
provide the vmg->middle VMA in this instance, or NULL otherwise.

Again we have an adj_next offset calculation:

next->vm_start + vmg->end - vmg->middle->vm_start

Where next == vmg->middle this simplifies to vmg->end as previously
demonstrated.

Therefore nstart is equal to vmg->end, which is already passed to
vma_adjust_trans_huge() via the 'end' parameter and so this code (rather
delightfully) simplifies to:

	if (next)
		split_huge_pmd_if_needed(next, end);

With these changes in place, it becomes silly for commit_merge() to return
vmg->target, as it is always the same and threaded through vmg, so we
finally change commit_merge() to return an error value once again.

This patch has no change in functional behaviour.

Link: https://lkml.kernel.org/r/7bce2cd4b5afb56211822835d145471280c3dccc.1738326519.git.lorenzo.stoakes@oracle.com
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Jann Horn <jannh@google.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 93e509b6c00eb353e3f2fef1257c8ef3a41f1814..e1bea54820fff03111f5ec21716b256af655e614 100644 (file)
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -404,7 +404,7 @@ int madvise_collapse(struct vm_area_struct *vma,
                     struct vm_area_struct **prev,
                     unsigned long start, unsigned long end);
 void vma_adjust_trans_huge(struct vm_area_struct *vma, unsigned long start,
-                          unsigned long end, long adjust_next);
+                          unsigned long end, struct vm_area_struct *next);
 spinlock_t *__pmd_trans_huge_lock(pmd_t *pmd, struct vm_area_struct *vma);
 spinlock_t *__pud_trans_huge_lock(pud_t *pud, struct vm_area_struct *vma);
 
@@ -571,7 +571,7 @@ static inline int madvise_collapse(struct vm_area_struct *vma,
 static inline void vma_adjust_trans_huge(struct vm_area_struct *vma,
                                         unsigned long start,
                                         unsigned long end,
-                                        long adjust_next)
+                                        struct vm_area_struct *next)
 {
 }
 static inline int is_swap_pmd(pmd_t pmd)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e33da765c428ebe289155885c5ed9416ea87d15b..e7ac4f0dc21dd7f859c9b9b8998264e035428a69 100644 (file)
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3017,9 +3017,9 @@ static inline void split_huge_pmd_if_needed(struct vm_area_struct *vma, unsigned
 }
 
 void vma_adjust_trans_huge(struct vm_area_struct *vma,
-                            unsigned long start,
-                            unsigned long end,
-                            long adjust_next)
+                          unsigned long start,
+                          unsigned long end,
+                          struct vm_area_struct *next)
 {
        /* Check if we need to split start first. */
        split_huge_pmd_if_needed(vma, start);
@@ -3027,16 +3027,9 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma,
        /* Check if we need to split end next. */
        split_huge_pmd_if_needed(vma, end);
 
-       /*
-        * If we're also updating the next vma vm_start,
-        * check if we need to split it.
-        */
-       if (adjust_next > 0) {
-               struct vm_area_struct *next = find_vma(vma->vm_mm, vma->vm_end);
-               unsigned long nstart = next->vm_start;
-               nstart += adjust_next;
-               split_huge_pmd_if_needed(next, nstart);
-       }
+       /* If we're incrementing next->vm_start, we might need to split it. */
+       if (next)
+               split_huge_pmd_if_needed(next, end);
 }
 
 static void unmap_folio(struct folio *folio)

diff --git a/mm/vma.c b/mm/vma.c
index b10625e8fc998e4a558e1809dfc8fb7333d7cf7e..603e538a093ff38f9436dcdbc0210f3b5c0bfc66 100644 (file)
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -513,7 +513,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
        init_vma_prep(&vp, vma);
        vp.insert = new;
        vma_prepare(&vp);
-       vma_adjust_trans_huge(vma, vma->vm_start, addr, 0);
+       vma_adjust_trans_huge(vma, vma->vm_start, addr, NULL);
 
        if (new_below) {
                vma->vm_start = addr;
@@ -644,46 +644,44 @@ void validate_mm(struct mm_struct *mm)
 #endif /* CONFIG_DEBUG_VM_MAPLE_TREE */
 
 /*
- * Actually perform the VMA merge operation.
- *
- * On success, returns the merged VMA. Otherwise returns NULL.
+ * Based on the vmg flag indicating whether we need to adjust the vm_start field
+ * for the middle or next VMA, we calculate what the range of the newly adjusted
+ * VMA ought to be, and set the VMA's range accordingly.
  */
-static struct vm_area_struct *commit_merge(struct vma_merge_struct *vmg)
+static void vmg_adjust_set_range(struct vma_merge_struct *vmg)
 {
-       struct vm_area_struct *vma;
-       struct vma_prepare vp;
        struct vm_area_struct *adjust;
-       long adj_start;
+       pgoff_t pgoff;
 
-       /*
-        * If modifying an existing VMA and we don't remove vmg->middle, then we
-        * shrink the adjacent VMA.
-        */
        if (vmg->__adjust_middle_start) {
-               vma = vmg->target;
                adjust = vmg->middle;
-               /* The POSITIVE value by which we offset vmg->middle->vm_start. */
-               adj_start = vmg->end - vmg->middle->vm_start;
-
-                /* Note: vma iterator must be pointing to 'start'. */
-               vma_iter_config(vmg->vmi, vmg->start, vmg->end);
+               pgoff = adjust->vm_pgoff + PHYS_PFN(vmg->end - adjust->vm_start);
        } else if (vmg->__adjust_next_start) {
-               /*
-                * In this case alone, the VMA we manipulate is vmg->middle, but
-                * we ultimately return vmg->next.
-                */
-               vma = vmg->middle;
                adjust = vmg->next;
-               /* The NEGATIVE value by which we offset vmg->next->vm_start. */
-               adj_start = -(vmg->middle->vm_end - vmg->end);
+               pgoff = adjust->vm_pgoff - PHYS_PFN(adjust->vm_start - vmg->end);
+       } else {
+               return;
+       }
+
+       vma_set_range(adjust, vmg->end, adjust->vm_end, pgoff);
+}
+
+/*
+ * Actually perform the VMA merge operation.
+ *
+ * Returns 0 on success, or an error value on failure.
+ */
+static int commit_merge(struct vma_merge_struct *vmg)
+{
+       struct vm_area_struct *vma;
+       struct vma_prepare vp;
 
-               vma_iter_config(vmg->vmi, vmg->next->vm_start + adj_start,
-                               vmg->next->vm_end);
+       if (vmg->__adjust_next_start) {
+               /* We manipulate middle and adjust next, which is the target. */
+               vma = vmg->middle;
+               vma_iter_config(vmg->vmi, vmg->end, vmg->next->vm_end);
        } else {
                vma = vmg->target;
-               adjust = NULL;
-               adj_start = 0;
-
                 /* Note: vma iterator must be pointing to 'start'. */
                vma_iter_config(vmg->vmi, vmg->start, vmg->end);
        }
@@ -691,22 +689,22 @@ static struct vm_area_struct *commit_merge(struct vma_merge_struct *vmg)
        init_multi_vma_prep(&vp, vma, vmg);
 
        if (vma_iter_prealloc(vmg->vmi, vma))
-               return NULL;
+               return -ENOMEM;
 
        vma_prepare(&vp);
-       vma_adjust_trans_huge(vma, vmg->start, vmg->end, adj_start);
+       /*
+        * THP pages may need to do additional splits if we increase
+        * middle->vm_start.
+        */
+       vma_adjust_trans_huge(vma, vmg->start, vmg->end,
+                             vmg->__adjust_middle_start ? vmg->middle : NULL);
        vma_set_range(vma, vmg->start, vmg->end, vmg->pgoff);
-
-       if (adj_start) {
-               adjust->vm_start += adj_start;
-               adjust->vm_pgoff += PHYS_PFN(adj_start);
-       }
-
+       vmg_adjust_set_range(vmg);
        vma_iter_store(vmg->vmi, vmg->target);
 
        vma_complete(&vp, vmg->vmi, vma->vm_mm);
 
-       return vmg->target;
+       return 0;
 }
 
 /* We can only remove VMAs when merging if they do not have a close hook. */
@@ -749,7 +747,7 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
 {
        struct vm_area_struct *middle = vmg->middle;
        struct vm_area_struct *prev = vmg->prev;
-       struct vm_area_struct *next, *res;
+       struct vm_area_struct *next;
        struct vm_area_struct *anon_dup = NULL;
        unsigned long start = vmg->start;
        unsigned long end = vmg->end;
@@ -900,12 +898,7 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
                        vmg->end = next->vm_end;
                        vmg->pgoff = next->vm_pgoff - pglen;
                } else {
-                       /*
-                        * We shrink middle and expand next.
-                        *
-                        * IMPORTANT: This is the ONLY case where the final
-                        * merged VMA is NOT vmg->target, but rather vmg->next.
-                        */
+                       /* We shrink middle and expand next. */
                        vmg->__adjust_next_start = true;
                        vmg->start = middle->vm_start;
                        vmg->end = start;
@@ -918,8 +911,10 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
        if (err)
                goto abort;
 
-       res = commit_merge(vmg);
-       if (!res) {
+       err = commit_merge(vmg);
+       if (err) {
+               VM_WARN_ON(err != -ENOMEM);
+
                if (anon_dup)
                        unlink_anon_vmas(anon_dup);
 
@@ -927,9 +922,9 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
                return NULL;
        }
 
-       khugepaged_enter_vma(res, vmg->flags);
+       khugepaged_enter_vma(vmg->target, vmg->flags);
        vmg->state = VMA_MERGE_SUCCESS;
-       return res;
+       return vmg->target;
 
 abort:
        vma_iter_set(vmg->vmi, start);
@@ -1092,7 +1087,7 @@ int vma_expand(struct vma_merge_struct *vmg)
        if (remove_next)
                vmg->__remove_next = true;
 
-       if (!commit_merge(vmg))
+       if (commit_merge(vmg))
                goto nomem;
 
        return 0;
@@ -1132,7 +1127,7 @@ int vma_shrink(struct vma_iterator *vmi, struct vm_area_struct *vma,
 
        init_vma_prep(&vp, vma);
        vma_prepare(&vp);
-       vma_adjust_trans_huge(vma, start, end, 0);
+       vma_adjust_trans_huge(vma, start, end, NULL);
 
        vma_iter_clear(vmi);
        vma_set_range(vma, start, end, pgoff);

diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index 1eae23039854af6881010f6abd5d854f402c2572..bb273927af0ff2daaf25543e9f312ef8a1cb9eb4 100644 (file)
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -796,12 +796,12 @@ static inline void vma_start_write(struct vm_area_struct *vma)
 static inline void vma_adjust_trans_huge(struct vm_area_struct *vma,
                                         unsigned long start,
                                         unsigned long end,
-                                        long adjust_next)
+                                        struct vm_area_struct *next)
 {
        (void)vma;
        (void)start;
        (void)end;
-       (void)adjust_next;
+       (void)next;
 }
 
 static inline void vma_iter_free(struct vma_iterator *vmi)