[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230322153109.wtz42xxmnchcigaa@revolver>
Date: Wed, 22 Mar 2023 11:31:09 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Lorenzo Stoakes <lstoakes@...il.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
David Hildenbrand <david@...hat.com>,
Matthew Wilcox <willy@...radead.org>,
Vlastimil Babka <vbabka@...e.cz>,
maple-tree@...ts.infradead.org, Vernon Yang <vernon2gm@...il.com>
Subject: Re: [PATCH v3 1/4] mm/mmap/vma_merge: further improve prev/next VMA
naming
* Lorenzo Stoakes <lstoakes@...il.com> [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@...il.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> ---
> 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
>
Powered by blists - more mailing lists