On 6/3/22 16:57, Jakub Matěna wrote:
> Refactor vma_merge() to make it shorter and more understandable.
> Main change is the elimination of code duplicity in the case of
> merge next check. This is done by first doing checks and caching
> the results before executing the merge itself. The variable 'area' is
> divided into 'mid' and 'res' as previously it was used for two purposes,
> as the middle VMA between prev and next and also as the result of the
> merge itself. Exit paths are also unified.
>
> Signed-off-by: Jakub Matěna <matenajakub@xxxxxxxxx>
Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>
> ---
> mm/mmap.c | 87 +++++++++++++++++++++++--------------------------------
> 1 file changed, 37 insertions(+), 50 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 313b57d55a63..91100fdc400a 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1170,8 +1170,10 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
> struct anon_vma_name *anon_name)
> {
> pgoff_t pglen = (end - addr) >> PAGE_SHIFT;
> - struct vm_area_struct *area, *next;
> - int err;
> + struct vm_area_struct *mid, *next, *res;
> + int err = -1;
> + bool merge_prev = false;
> + bool merge_next = false;
>
> /*
> * We later require that vma->vm_flags == vm_flags,
> @@ -1181,75 +1183,60 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
> return NULL;
>
> next = find_vma(mm, prev ? prev->vm_end : 0);
> - area = next;
> - if (area && area->vm_end == end) /* cases 6, 7, 8 */
> + mid = next;
> + if (next && next->vm_end == end) /* cases 6, 7, 8 */
> next = find_vma(mm, next->vm_end);
>
> /* verify some invariant that must be enforced by the caller */
> VM_WARN_ON(prev && addr <= prev->vm_start);
> - VM_WARN_ON(area && end > area->vm_end);
> + VM_WARN_ON(mid && end > mid->vm_end);
> VM_WARN_ON(addr >= end);
>
> - /*
> - * Can it merge with the predecessor?
> - */
> + /* Can we merge the predecessor? */
> if (prev && prev->vm_end == addr &&
> mpol_equal(vma_policy(prev), policy) &&
> can_vma_merge_after(prev, vm_flags,
> anon_vma, file, pgoff,
> vm_userfaultfd_ctx, anon_name)) {
> - /*
> - * OK, it can. Can we now merge in the successor as well?
> - */
> - if (next && end == next->vm_start &&
> - mpol_equal(policy, vma_policy(next)) &&
> - can_vma_merge_before(next, vm_flags,
> - anon_vma, file,
> - pgoff+pglen,
> - vm_userfaultfd_ctx, anon_name) &&
> - is_mergeable_anon_vma(prev->anon_vma,
> - next->anon_vma, NULL)) {
> - /* cases 1, 6 */
> - err = __vma_adjust(prev, prev->vm_start,
> - next->vm_end, prev->vm_pgoff, NULL,
> - prev);
> - } else /* cases 2, 5, 7 */
> - err = __vma_adjust(prev, prev->vm_start,
> - end, prev->vm_pgoff, NULL, prev);
> - if (err)
> - return NULL;
> - khugepaged_enter_vma(prev, vm_flags);
> - return prev;
> + merge_prev = true;
> }
> -
> - /*
> - * Can this new request be merged in front of next?
> - */
> + /* Can we merge the successor? */
> if (next && end == next->vm_start &&
> mpol_equal(policy, vma_policy(next)) &&
> can_vma_merge_before(next, vm_flags,
> anon_vma, file, pgoff+pglen,
> vm_userfaultfd_ctx, anon_name)) {
> + merge_next = true;
> + }
> + /* Can we merge both the predecessor and the successor? */
> + if (merge_prev && merge_next &&
> + is_mergeable_anon_vma(prev->anon_vma,
> + next->anon_vma, NULL)) { /* cases 1, 6 */
> + err = __vma_adjust(prev, prev->vm_start,
> + next->vm_end, prev->vm_pgoff, NULL,
> + prev);
> + res = prev;
> + } else if (merge_prev) { /* cases 2, 5, 7 */
> + err = __vma_adjust(prev, prev->vm_start,
> + end, prev->vm_pgoff, NULL, prev);
> + res = prev;
> + } else if (merge_next) {
> if (prev && addr < prev->vm_end) /* case 4 */
> err = __vma_adjust(prev, prev->vm_start,
> - addr, prev->vm_pgoff, NULL, next);
> - else { /* cases 3, 8 */
> - err = __vma_adjust(area, addr, next->vm_end,
> - next->vm_pgoff - pglen, NULL, next);
> - /*
> - * In case 3 area is already equal to next and
> - * this is a noop, but in case 8 "area" has
> - * been removed and next was expanded over it.
> - */
> - area = next;
> - }
> - if (err)
> - return NULL;
> - khugepaged_enter_vma(area, vm_flags);
> - return area;
> + addr, prev->vm_pgoff, NULL, next);
> + else /* cases 3, 8 */
> + err = __vma_adjust(mid, addr, next->vm_end,
> + next->vm_pgoff - pglen, NULL, next);
> + res = next;
> }
>
> - return NULL;
> + /*
> + * Cannot merge with predecessor or successor or error in __vma_adjust?
> + */
> + if (err)
> + return NULL;
> + khugepaged_enter_vma(res, vm_flags);
> + return res;
> }
>
> /*
