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

[Lorenzo Stoakes]

On Mon, Oct 09, 2023 at 06:30:02PM +0200, Vlastimil Babka wrote:
> 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>

Thanks!

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

Will update in v2.

>
> > +
> > +	/* 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