Re: [PATCH v3 4/7] mm/mremap: initial refactor of move_vma()

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

On Mon, Mar 31, 2025 at 11:13:41AM +0200, Vlastimil Babka wrote:
> On 3/30/25 18:52, Lorenzo Stoakes wrote:
> > On Mon, Mar 10, 2025 at 08:50:37PM +0000, Lorenzo Stoakes wrote:
> >> Update move_vma() to use the threaded VRM object, de-duplicate code and
> >> separate into smaller functions to aid readability and debug-ability.
> >>
> >> This in turn allows further simplification of expand_vma() as we can
> >> simply thread VRM through the function.
> >
> > [snip]
> >
> > Andrew - I enclose a fix-patch for the issue kindly reported in [0] by Yi
> > Lai. Since you've not sent the PR to Linus yet maybe you could squash this
> > in? Otherwise obviously one for 6.15-rc1.
> >
> > I've tested against the repro and confirm it fixes it, also the fix is
> > 'obvious' as is the cause. I have replied to [0] with an explanation there
> > also inline.
> >
> > Apologies for missing this before!
> >
> > Thanks, Lorenzo
> >
> > [0]: https://lore.kernel.org/linux-mm/Z+lcvEIHMLiKVR1i@ly-workstation/
> >
> > ----8<----
> > From 3709f42feb30e2cfe2f39527d4cd8c74a9e8b724 Mon Sep 17 00:00:00 2001
> > From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Date: Sun, 30 Mar 2025 17:20:48 +0100
> > Subject: [PATCH] mm/mremap: do not set vrm->vma NULL immediately prior to
> >  checking it
> >
> > This seems rather unwise. If we cannot merge, extend, then we need to
> > recall the original VMA to see if we need to uncharge.
> >
> > If we do need to, do so.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>
> Can we get a testcase that hits this path? :)

Ack, will add!

>
> >
> > ---
> >  mm/mremap.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index 0865387531ed..7db9da609c84 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -1561,11 +1561,12 @@ static unsigned long expand_vma_in_place(struct vma_remap_struct *vrm)
> >  	 * adjacent to the expanded vma and otherwise
> >  	 * compatible.
> >  	 */
> > -	vma = vrm->vma = vma_merge_extend(&vmi, vma, vrm->delta);
> > +	vma = vma_merge_extend(&vmi, vma, vrm->delta);
> >  	if (!vma) {
> >  		vrm_uncharge(vrm);
> >  		return -ENOMEM;
> >  	}
> > +	vrm->vma = vma;
> >
> >  	vrm_stat_account(vrm, vrm->delta);
> >
> > --
> > 2.49.0
>