Re: [PATCH v3 1/4] mm/mmap/vma_merge: further improve prev/next VMA naming

[Liam R. Howlett]

* Lorenzo Stoakes <lstoakes@xxxxxxxxx> [230322 03:13]:
> Previously the ASCII diagram above vma_merge() and the accompanying
> variable naming was rather confusing, however recent efforts by Liam
> Howlett and Vlastimil Babka have significantly improved matters.
> 
> This patch goes a little further - replacing 'X' with 'N', which feels more
> natural as this represents the _next_ VMA and replacing what was 'N' with
> 'C' which represents the current VMA.
> 
> No word quite describes a VMA that has coincident start as the input span,
> however 'concurrent' (or more simply 'current') abbreviated to 'curr' fits
> intuitions well alongside prev and next.
> 
> This has no functional impact.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>

Reviewed-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>

> ---
>  mm/mmap.c | 86 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 042d22e63528..c9834364ac98 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -861,44 +861,44 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags,
>   * this area are about to be changed to vm_flags - and the no-change
>   * case has already been eliminated.
>   *
> - * The following mprotect cases have to be considered, where AAAA is
> + * The following mprotect cases have to be considered, where **** is
>   * the area passed down from mprotect_fixup, never extending beyond one
> - * vma, PPPP is the previous vma, NNNN is a vma that starts at the same
> - * address as AAAA and is of the same or larger span, and XXXX the next
> - * vma after AAAA:
> + * vma, PPPP is the previous vma, CCCC is a concurrent vma that starts
> + * at the same address as **** and is of the same or larger span, and
> + * NNNN the next vma after ****:
>   *
> - *     AAAA             AAAA                   AAAA
> - *    PPPPPPXXXXXX    PPPPPPXXXXXX       PPPPPPNNNNNN
> + *     ****             ****                   ****
> + *    PPPPPPNNNNNN    PPPPPPNNNNNN       PPPPPPCCCCCC
>   *    cannot merge    might become       might become
> - *                    PPXXXXXXXXXX       PPPPPPPPPPNN
> + *                    PPNNNNNNNNNN       PPPPPPPPPPCC
>   *    mmap, brk or    case 4 below       case 5 below
>   *    mremap move:
> - *                        AAAA               AAAA
> - *                    PPPP    XXXX       PPPPNNNNXXXX
> + *                        ****               ****
> + *                    PPPP    NNNN       PPPPCCCCNNNN
>   *                    might become       might become
>   *                    PPPPPPPPPPPP 1 or  PPPPPPPPPPPP 6 or
> - *                    PPPPPPPPXXXX 2 or  PPPPPPPPXXXX 7 or
> - *                    PPPPXXXXXXXX 3     PPPPXXXXXXXX 8
> + *                    PPPPPPPPNNNN 2 or  PPPPPPPPNNNN 7 or
> + *                    PPPPNNNNNNNN 3     PPPPNNNNNNNN 8
>   *
> - * It is important for case 8 that the vma NNNN overlapping the
> - * region AAAA is never going to extended over XXXX. Instead XXXX must
> - * be extended in region AAAA and NNNN must be removed. This way in
> + * It is important for case 8 that the vma CCCC overlapping the
> + * region **** is never going to extended over NNNN. Instead NNNN must
> + * be extended in region **** and CCCC must be removed. This way in
>   * all cases where vma_merge succeeds, the moment vma_merge drops the
>   * rmap_locks, the properties of the merged vma will be already
>   * correct for the whole merged range. Some of those properties like
>   * vm_page_prot/vm_flags may be accessed by rmap_walks and they must
>   * be correct for the whole merged range immediately after the
> - * rmap_locks are released. Otherwise if XXXX would be removed and
> - * NNNN would be extended over the XXXX range, remove_migration_ptes
> + * rmap_locks are released. Otherwise if NNNN would be removed and
> + * CCCC would be extended over the NNNN range, remove_migration_ptes
>   * or other rmap walkers (if working on addresses beyond the "end"
> - * parameter) may establish ptes with the wrong permissions of NNNN
> - * instead of the right permissions of XXXX.
> + * parameter) may establish ptes with the wrong permissions of CCCC
> + * instead of the right permissions of NNNN.
>   *
>   * In the code below:
>   * PPPP is represented by *prev
> - * NNNN is represented by *mid or not represented at all (NULL)
> - * XXXX is represented by *next or not represented at all (NULL)
> - * AAAA is not represented - it will be merged and the vma containing the
> + * CCCC is represented by *curr or not represented at all (NULL)
> + * NNNN is represented by *next or not represented at all (NULL)
> + * **** is not represented - it will be merged and the vma containing the
>   *      area is returned, or the function will return NULL
>   */
>  struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
> @@ -911,7 +911,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  {
>  	pgoff_t pglen = (end - addr) >> PAGE_SHIFT;
>  	pgoff_t vma_pgoff;
> -	struct vm_area_struct *mid, *next, *res = NULL;
> +	struct vm_area_struct *curr, *next, *res = NULL;
>  	struct vm_area_struct *vma, *adjust, *remove, *remove2;
>  	int err = -1;
>  	bool merge_prev = false;
> @@ -930,19 +930,19 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  	if (vm_flags & VM_SPECIAL)
>  		return NULL;
>  
> -	mid = find_vma(mm, prev ? prev->vm_end : 0);
> -	if (mid && mid->vm_end == end)			/* cases 6, 7, 8 */
> -		next = find_vma(mm, mid->vm_end);
> +	curr = find_vma(mm, prev ? prev->vm_end : 0);
> +	if (curr && curr->vm_end == end)		/* cases 6, 7, 8 */
> +		next = find_vma(mm, curr->vm_end);
>  	else
> -		next = mid;
> +		next = curr;
>  
> -	/* In cases 1 - 4 there's no NNNN vma */
> -	if (mid && end <= mid->vm_start)
> -		mid = NULL;
> +	/* In cases 1 - 4 there's no CCCC vma */
> +	if (curr && end <= curr->vm_start)
> +		curr = NULL;
>  
>  	/* verify some invariant that must be enforced by the caller */
>  	VM_WARN_ON(prev && addr <= prev->vm_start);
> -	VM_WARN_ON(mid && end > mid->vm_end);
> +	VM_WARN_ON(curr && end > curr->vm_end);
>  	VM_WARN_ON(addr >= end);
>  
>  	if (prev) {
> @@ -974,21 +974,21 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  		remove = next;				/* case 1 */
>  		vma_end = next->vm_end;
>  		err = dup_anon_vma(prev, next);
> -		if (mid) {				/* case 6 */
> -			remove = mid;
> +		if (curr) {				/* case 6 */
> +			remove = curr;
>  			remove2 = next;
>  			if (!next->anon_vma)
> -				err = dup_anon_vma(prev, mid);
> +				err = dup_anon_vma(prev, curr);
>  		}
>  	} else if (merge_prev) {
>  		err = 0;				/* case 2 */
> -		if (mid) {
> -			err = dup_anon_vma(prev, mid);
> -			if (end == mid->vm_end) {	/* case 7 */
> -				remove = mid;
> +		if (curr) {
> +			err = dup_anon_vma(prev, curr);
> +			if (end == curr->vm_end) {	/* case 7 */
> +				remove = curr;
>  			} else {			/* case 5 */
> -				adjust = mid;
> -				adj_start = (end - mid->vm_start);
> +				adjust = curr;
> +				adj_start = (end - curr->vm_start);
>  			}
>  		}
>  	} else if (merge_next) {
> @@ -1004,10 +1004,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  			vma_end = next->vm_end;
>  			vma_pgoff = next->vm_pgoff;
>  			err = 0;
> -			if (mid) {			/* case 8 */
> -				vma_pgoff = mid->vm_pgoff;
> -				remove = mid;
> -				err = dup_anon_vma(next, mid);
> +			if (curr) {			/* case 8 */
> +				vma_pgoff = curr->vm_pgoff;
> +				remove = curr;
> +				err = dup_anon_vma(next, curr);
>  			}
>  		}
>  	}
> -- 
> 2.39.2
>