Re: [PATCH v2 06/10] mm: avoid using vma_merge() for new VMAs

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, Aug 29, 2024 at 08:46:28PM GMT, Mark Brown wrote:
> On Fri, Aug 23, 2024 at 09:07:01PM +0100, Lorenzo Stoakes wrote:
> > Abstract vma_merge_new_vma() to use vma_merge_struct and rename the
> > resultant function vma_merge_new_range() to be clear what the purpose of
> > this function is - a new VMA is desired in the specified range, and we wish
> > to see if it is possible to 'merge' surrounding VMAs into this range rather
> > than having to allocate a new VMA.
>
> This patch, which is in -next today with the fixup Lorenzo posted as
> commit 8c9d0f8b1e9a42586, seems to be causing problems with the mremap
> expand merge selftest.  The test has been failing for a few days.  It
> unfortunately doesn't log anything about why it's upset:
>
> # # ok 15 5MB mremap - Source 1MB-aligned, Dest 1MB-aligned with 40MB Preamble
> # # not ok 16 mremap expand merge
> # # ok 18 mremap mremap move within range

[snip]

Thanks, I figured out the problem, it's not arm-specific, I was running
self-tests but eyeballing-failure resulted in me missing this.

This is a product of vma_merge_extend() invoking vma_merge_new_range() without
having determined the next VMA correctly, after moving from vma_merge() (which
looked this up for us) to vma_merge_new_range() (which does not).

This is after having adjusted the assumptions between v1 and v2 of the series in
each merge function, and I simply missed this mremap()-specific case.

Andrew - I enclose a fix-patch to get a fix out for this asap, but I am due a
respin relatively soon and will also include that in this.

----8<----
From 3678f8a53f98de52f11946d4d32e6fb239d11c2f Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Thu, 29 Aug 2024 22:18:02 +0100
Subject: [PATCH] mm: correctly determine vmg.next in vma_merge_extend()

vma_merge_next_range() requires that the caller specify prev AND next.

Failure to specify results in missed merges. Fix this by explicitly looking
up next.

This function is explicitly used by mremap() in extend cases.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reported-by: Mark Brown <broonie@kernel.org>
---
 mm/vma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/vma.c b/mm/vma.c
index 7cddeea907f8..bd35abc70ed8 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -1489,6 +1489,10 @@ struct vm_area_struct *vma_merge_extend(struct vma_iterator *vmi,
 {
 	VMG_VMA_STATE(vmg, vmi, vma, vma, vma->vm_end, vma->vm_end + delta);

+	vmg.next = vma_next(vmi);
+	if (vma_prev(vmi))
+		vma_iter_next_range(vmi);
+
 	/* We use the VMA to populate VMG fields only. */
 	vmg.vma = NULL;
 	return vma_merge_new_range(&vmg);
--
2.46.0