On 8/5/24 14:13, Lorenzo Stoakes 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>
<snip>
> +struct vm_area_struct *vma_merge_new_vma(struct vma_merge_struct *vmg)
> {
> - struct vma_merge_struct vmg = {
> - .vmi = vmi,
> - .prev = prev,
> - .vma = vma,
This line not being addded in the wrapper felt like a mistake. I see in the
other subthread that it's not so I'll just support your decision to tackle
it so the code is less surprising.
> - .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);
> + }
Admit this is another surprise. The old code didn't do it anywhere AFAICS so
I don't see why it's now done. Maybe it's necessary for futher changes.
Could you add a comment or explain it in the changelog, please?
> - return vma_merge(&vmg);
> + return vma_merge(vmg);
> }
>
> /*
