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

[Vlastimil Babka]

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@xxxxxxxxxx>
> 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@xxxxxxxxxx>

Acked-by: Vlastimil Babka <vbabka@xxxxxxx>

Can we get a testcase that hits this path? :)

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