Re: [PATCH] mm/vma: use vmg->target to specify target VMA for new VMA merge

[Vlastimil Babka <vbabka@suse.cz>]

On 6/13/25 20:48, Lorenzo Stoakes wrote:
> In commit 3a75ccba047b ("mm: simplify vma merge structure and expand
> comments") we introduced the vmg->target field to make the merging of
> existing VMAs simpler - clarifying precisely which VMA would eventually
> become the merged VMA once the merge operation was complete.
> 
> New VMA merging did not get quite the same treatment, retaining the rather
> confusing convention of storing the target VMA in vmg->middle.
> 
> This patch corrects this state of affairs, utilising vmg->target for this
> purpose for both vma_merge_new_range() and also for vma_expand().
> 
> We retain the WARN_ON for vmg->middle being specified in
> vma_merge_new_range() as doing so would make no sense, but add an
> additional debug assert for setting vmg->target.
> 
> This patch additionally updates VMA userland testing to account for this
> change.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

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

Nit below:

> @@ -1086,27 +1087,29 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
>   * @vmg: Describes a VMA expansion operation.
>   *
>   * Expand @vma to vmg->start and vmg->end.  Can expand off the start and end.
> - * Will expand over vmg->next if it's different from vmg->middle and vmg->end ==
> - * vmg->next->vm_end.  Checking if the vmg->middle can expand and merge with
> + * Will expand over vmg->next if it's different from vmg->target and vmg->end ==
> + * vmg->next->vm_end.  Checking if the vmg->target can expand and merge with
>   * vmg->next needs to be handled by the caller.
>   *
>   * Returns: 0 on success.
>   *
>   * ASSUMPTIONS:
> - * - The caller must hold a WRITE lock on vmg->middle->mm->mmap_lock.
> - * - The caller must have set @vmg->middle and @vmg->next.
> + * - The caller must hold a WRITE lock on vmg->target->mm->mmap_lock.

The assert uses vmg->mm so maybe the comment should do the same? (IIRC mm
was added only later to vmg?)

> + * - The caller must have set @vmg->target and @vmg->next.
>   */
>  int vma_expand(struct vma_merge_struct *vmg)
>  {
>  	struct vm_area_struct *anon_dup = NULL;
>  	bool remove_next = false;
> -	struct vm_area_struct *middle = vmg->middle;
> +	struct vm_area_struct *target = vmg->target;
>  	struct vm_area_struct *next = vmg->next;
> 
> +	VM_WARN_ON_VMG(!target, vmg);
> +
>  	mmap_assert_write_locked(vmg->mm);
> 
> -	vma_start_write(middle);
> -	if (next && (middle != next) && (vmg->end == next->vm_end)) {
> +	vma_start_write(target);
> +	if (next && (target != next) && (vmg->end == next->vm_end)) {
>  		int ret;
> 
>  		remove_next = true;