Re: [PATCH 4/4] mm: abstract VMA extension and merge into vma_merge_extend() helper

[Vlastimil Babka]

On 10/8/23 22:23, Lorenzo Stoakes wrote:
> mremap uses vma_merge() in the case where a VMA needs to be extended. This
> can be significantly simplified and abstracted.
> 
> This makes it far easier to understand what the actual function is doing,
> avoids future mistakes in use of the confusing vma_merge() function and
> importantly allows us to make future changes to how vma_merge() is
> implemented by knowing explicitly which merge cases each invocation uses.
> 
> Note that in the mremap() extend case, we perform this merge only when
> old_len == vma->vm_end - addr. The extension_start, i.e. the start of the
> extended portion of the VMA is equal to addr + old_len, i.e. vma->vm_end.
> 
> With this refactoring, vma_merge() is no longer required anywhere except
> mm/mmap.c, so mark it static.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>

Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>

Nit:
> @@ -2546,6 +2546,24 @@ static struct vm_area_struct *vma_merge_new_vma(struct vma_iterator *vmi,
>  			 vma->vm_userfaultfd_ctx, anon_vma_name(vma));
>  }
>  
> +/*
> + * Expand vma by delta bytes, potentially merging with an immediately adjacent
> + * VMA with identical properties.
> + */
> +struct vm_area_struct *vma_merge_extend(struct vma_iterator *vmi,
> +					struct vm_area_struct *vma,
> +					unsigned long delta)
> +{
> +	pgoff_t pgoff = vma->vm_pgoff +
> +		((vma->vm_end - vma->vm_start) >> PAGE_SHIFT);

could use vma_pages() here

> +
> +	/* vma is specified as prev, so case 1 or 2 will apply. */
> +	return vma_merge(vmi, vma->vm_mm, vma, vma->vm_end, vma->vm_end + delta,
> +			 vma->vm_flags, vma->anon_vma, vma->vm_file, pgoff,
> +			 vma_policy(vma), vma->vm_userfaultfd_ctx,
> +			 anon_vma_name(vma));
> +}
> +
>  /*
>   * do_vmi_align_munmap() - munmap the aligned region from @start to @end.
>   * @vmi: The vma iterator