Commit c372473a authored by Lorenzo Stoakes's avatar Lorenzo Stoakes Committed by Andrew Morton
Browse files

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: default avatarLorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: default avatarVlastimil Babka <vbabka@suse.cz>
Cc: Jann Horn <jannh@google.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 0e5ffe9b

include/linux/huge_mm.h

+2 −2
Original line number Diff line number Diff line
@@ -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)

mm/huge_memory.c

+6 −13
Original line number Diff line number Diff line
@@ -3019,7 +3019,7 @@ 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)

			   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)

mm/vma.c

+47 −52
Original line number Diff line number Diff line
@@ -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) {

		adjust = vmg->next;

		pgoff = adjust->vm_pgoff - PHYS_PFN(adjust->vm_start - vmg->end);

	} else {

		return;

	}



	vma_set_range(adjust, vmg->end, adjust->vm_end, pgoff);

}



/*

		 * In this case alone, the VMA we manipulate is vmg->middle, but

		 * we ultimately return vmg->next.

 * Actually perform the VMA merge operation.

 *

 * Returns 0 on success, or an error value on failure.

 */

		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);

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);

tools/testing/vma/vma_internal.h

+2 −2
Original line number Diff line number Diff line
@@ -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)