Re: [PATCH v3 4/5] mm: remove unnecessary reset state logic on merge new VMA

[Vlastimil Babka <vbabka@suse.cz>]

On 10/25/24 14:26, Lorenzo Stoakes wrote:
> The only place where this was used was in mmap_region(), which we have now
> adjusted to not require this to be performed (we reset ourselves in
> effect).
> 
> It also created a dangerous assumption that VMG state could be safely
> reused after a merge, at which point it may have been mutated in unexpected
> ways, leading to subtle bugs.
> 
> Note that it was discovered by Wei Yang that there was also an error in
> this code - we are comparing vmg->vma with prev after setting it to
> NULL.
> 
> This however had no impact, as we previously reset VMA iterator state
> before attempting merge again, but it was useless effort.
> 
> In any case, this patch removes all of the logic so also eliminates this
> wasted effort.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Yeah glad to be rid of this.

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/vma.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/mm/vma.c b/mm/vma.c
> index b91c947babd6..7c690be67910 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -963,7 +963,6 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
>  	struct vm_area_struct *next = vmg->next;
>  	unsigned long start = vmg->start;
>  	unsigned long end = vmg->end;
> -	pgoff_t pgoff = vmg->pgoff;
>  	pgoff_t pglen = PHYS_PFN(end - start);
>  	bool can_merge_left, can_merge_right;
>  	bool just_expand = vmg->merge_flags & VMG_FLAG_JUST_EXPAND;
> @@ -1020,16 +1019,6 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
>  		return vmg->vma;
>  	}
>  
> -	/* If expansion failed, reset state. Allows us to retry merge later. */
> -	if (!just_expand) {
> -		vmg->vma = NULL;
> -		vmg->start = start;
> -		vmg->end = end;
> -		vmg->pgoff = pgoff;
> -		if (vmg->vma == prev)
> -			vma_iter_set(vmg->vmi, start);
> -	}
> -
>  	return NULL;
>  }
>