On Mon, 5 Aug 2024 13:13:52 +0100
Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote:
> Abstract this function to so we can write tests which use the newly
> abstracted interface and maintain a stable interface for tests before/after
> refactoring.
>
> We introduce a temporary wrapper vma_merge_new_vma_wrapper() to minimise
> the code changes, in a subsequent commit we will entirely refactor this
> function.
>
> We also introduce a temporary implementation of vma_merge_modified() for
> the same reason - maintaining a common interface to the tests, this will be
> removed when vma_merge_modified() is correctly implemented in a subsequent
> commit.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> ---
> mm/mmap.c | 6 +++---
> mm/vma.c | 33 ++++++++++++---------------------
> mm/vma.h | 33 ++++++++++++++++++++++++++++++---
> tools/testing/vma/vma.c | 12 ++++++++----
> 4 files changed, 53 insertions(+), 31 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 04145347c245..f6593a81f73d 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1494,9 +1494,9 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> * vma again as we may succeed this time.
> */
> if (unlikely(vm_flags != vma->vm_flags && prev)) {
> - merge = vma_merge_new_vma(&vmi, prev, vma,
> - vma->vm_start, vma->vm_end,
> - vma->vm_pgoff);
> + merge = vma_merge_new_vma_wrapper(&vmi, prev, vma,
> + vma->vm_start, vma->vm_end,
> + vma->vm_pgoff);
> if (merge) {
> /*
> * ->mmap() can change vma->vm_file and fput
> diff --git a/mm/vma.c b/mm/vma.c
> index 3d6ce04f1b9c..55615392e8d2 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -1106,6 +1106,11 @@ static struct vm_area_struct *vma_merge(struct vma_merge_struct *vmg)
> return NULL;
> }
>
> +struct vm_area_struct *vma_merge_modified(struct vma_merge_struct *vmg)
> +{
> + return vma_merge(vmg);
> +}
> +
> /*
> * We are about to modify one or multiple of a VMA's flags, policy, userfaultfd
> * context and anonymous VMA name within the range [start, end).
> @@ -1260,27 +1265,14 @@ struct vm_area_struct
> * Attempt to merge a newly mapped VMA with those adjacent to it. The caller
> * must ensure that [start, end) does not overlap any existing VMA.
> */
> -struct vm_area_struct
> -*vma_merge_new_vma(struct vma_iterator *vmi, struct vm_area_struct *prev,
> - struct vm_area_struct *vma, unsigned long start,
> - unsigned long end, pgoff_t pgoff)
> +struct vm_area_struct *vma_merge_new_vma(struct vma_merge_struct *vmg)
> {
> - struct vma_merge_struct vmg = {
> - .vmi = vmi,
> - .prev = prev,
> - .vma = vma,
> - .start = start,
> - .end = end,
> - .flags = vma->vm_flags,
> - .file = vma->vm_file,
> - .anon_vma = vma->anon_vma,
> - .pgoff = pgoff,
> - .policy = vma_policy(vma),
> - .uffd_ctx = vma->vm_userfaultfd_ctx,
> - .anon_name = anon_vma_name(vma),
> - };
> + if (!vmg->prev) {
> + vmg->prev = vma_prev(vmg->vmi);
> + vma_iter_set(vmg->vmi, vmg->start);
> + }
>
> - return vma_merge(&vmg);
> + return vma_merge(vmg);
> }
>
> /*
> @@ -1295,7 +1287,6 @@ struct vm_area_struct *vma_merge_extend(struct vma_iterator *vmi,
> struct vma_merge_struct vmg = {
> .vmi = vmi,
> .prev = vma,
> - .vma = vma,
Yes, this member is not used later by vma_merge(), so it need not be
initialized. What about not adding this line in PATCH 02/10 of this
series? AFAICS vmg->vma was never used by vma_merge(). The net result
is the same, but it would make it easier to understand that this patch
in the series does not change the use of vmg->vma by vma_merge_extend().
Petr T
