On Mon, 5 Aug 2024 13:13:55 +0100
Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote:
> Pull this operation into its own function and have vma_expand() call
> commit_merge() instead.
>
> This lays the groundwork for a subsequent patch which replaces vma_merge()
> with a simpler function which can share the same code.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> ---
> mm/vma.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index a404cf718f9e..b7e3c64d5d68 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -564,6 +564,49 @@ void validate_mm(struct mm_struct *mm)
> }
> #endif /* CONFIG_DEBUG_VM_MAPLE_TREE */
>
> +/* Actually perform the VMA merge operation. */
> +static int commit_merge(struct vma_merge_struct *vmg,
> + struct vm_area_struct *adjust,
> + struct vm_area_struct *remove,
> + struct vm_area_struct *remove2,
> + long adj_start,
> + bool expanded)
> +{
> + struct vma_prepare vp;
> +
> + init_multi_vma_prep(&vp, vmg->vma, adjust, remove, remove2);
> +
> + if (expanded) {
> + vma_iter_config(vmg->vmi, vmg->start, vmg->end);
> + } else {
> + vma_iter_config(vmg->vmi, adjust->vm_start + adj_start,
> + adjust->vm_end);
> + }
It's hard to follow the logic if you the "expanded" parameter is always
true. I have to look at PATCH 09/10 first to see how it is expected to
be used. Is there no other way?
Note that this is not needed for adjust and adj_start, because they are
merely moved here from vma_expand() and passed down as parameters to
other functions.
Petr T
> +
> + if (vma_iter_prealloc(vmg->vmi, vmg->vma))
> + return -ENOMEM;
> +
> + vma_prepare(&vp);
> + vma_adjust_trans_huge(vmg->vma, vmg->start, vmg->end, adj_start);
> + vma_set_range(vmg->vma, vmg->start, vmg->end, vmg->pgoff);
> +
> + if (expanded)
> + vma_iter_store(vmg->vmi, vmg->vma);
> +
> + if (adj_start) {
> + adjust->vm_start += adj_start;
> + adjust->vm_pgoff += PHYS_PFN(adj_start);
> + if (adj_start < 0) {
> + WARN_ON(expanded);
> + vma_iter_store(vmg->vmi, adjust);
> + }
> + }
> +
> + vma_complete(&vp, vmg->vmi, vmg->vma->vm_mm);
> +
> + return 0;
> +}
> +
> /*
> * vma_merge_new_vma - Attempt to merge a new VMA into address space
> *
> @@ -700,7 +743,6 @@ int vma_expand(struct vma_merge_struct *vmg)
> bool remove_next = false;
> struct vm_area_struct *vma = vmg->vma;
> struct vm_area_struct *next = vmg->next;
> - struct vma_prepare vp;
>
> vma_start_write(vma);
> if (next && (vma != next) && (vmg->end == next->vm_end)) {
> @@ -713,24 +755,15 @@ int vma_expand(struct vma_merge_struct *vmg)
> return ret;
> }
>
> - init_multi_vma_prep(&vp, vma, NULL, remove_next ? next : NULL, NULL);
> /* Not merging but overwriting any part of next is not handled. */
> - VM_WARN_ON(next && !vp.remove &&
> + VM_WARN_ON(next && !remove_next &&
> next != vma && vmg->end > next->vm_start);
> /* Only handles expanding */
> VM_WARN_ON(vma->vm_start < vmg->start || vma->vm_end > vmg->end);
>
> - /* Note: vma iterator must be pointing to 'start' */
> - vma_iter_config(vmg->vmi, vmg->start, vmg->end);
> - if (vma_iter_prealloc(vmg->vmi, vma))
> + if (commit_merge(vmg, NULL, remove_next ? next : NULL, NULL, 0, true))
> goto nomem;
>
> - vma_prepare(&vp);
> - vma_adjust_trans_huge(vma, vmg->start, vmg->end, 0);
> - vma_set_range(vma, vmg->start, vmg->end, vmg->pgoff);
> - vma_iter_store(vmg->vmi, vma);
> -
> - vma_complete(&vp, vmg->vmi, vma->vm_mm);
> return 0;
>
> nomem:
