Commit fcfccd91 authored by Lorenzo Stoakes's avatar Lorenzo Stoakes Committed by Andrew Morton

mm/mmap/vma_merge: further improve prev/next VMA naming

Patch series "further cleanup of vma_merge()", v2.

Following on from Vlastimil Babka's patch series "cleanup vma_merge() and
improve mergeability tests" which was in turn based on Liam's prior
cleanups, this patch series introduces changes discussed in review of
Vlastimil's series and goes further in attempting to make the logic as
clear as possible.

Nearly all of this should have absolutely no functional impact, however it
does add a singular VM_WARN_ON() case.

With many thanks to Vernon for helping kick start the discussion around
simplification - abstract use of vma did indeed turn out not to be
necessary - and to Liam for his excellent suggestions which greatly
simplified things.


This patch (of 4):

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 a
lot more natural and replacing what was 'N' with 'C' which stands for
'concurrent' VMA.

No word quite describes a VMA that has coincident start as the input span,
concurrent, abbreviated to 'curr' (and which can be thought of also as
'current') however fits intuitions well alongside prev and next.

This has no functional impact.

Link: https://lkml.kernel.org/r/cover.1679431180.git.lstoakes@gmail.com
Link: https://lkml.kernel.org/r/6001e08fa7e119470cbb1d2b6275ad8d742ff9a7.1679431180.git.lstoakes@gmail.comSigned-off-by: default avatarLorenzo Stoakes <lstoakes@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Vernon Yang <vernon2gm@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 4c91c07c
...@@ -848,44 +848,44 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, ...@@ -848,44 +848,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 * this area are about to be changed to vm_flags - and the no-change
* case has already been eliminated. * 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 * 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 * vma, PPPP is the previous vma, CCCC is a concurrent vma that starts
* address as AAAA and is of the same or larger span, and XXXX the next * at the same address as **** and is of the same or larger span, and
* vma after AAAA: * NNNN the next vma after ****:
* *
* AAAA AAAA AAAA * **** **** ****
* PPPPPPXXXXXX PPPPPPXXXXXX PPPPPPNNNNNN * PPPPPPNNNNNN PPPPPPNNNNNN PPPPPPCCCCCC
* cannot merge might become might become * cannot merge might become might become
* PPXXXXXXXXXX PPPPPPPPPPNN * PPNNNNNNNNNN PPPPPPPPPPCC
* mmap, brk or case 4 below case 5 below * mmap, brk or case 4 below case 5 below
* mremap move: * mremap move:
* AAAA AAAA * **** ****
* PPPP XXXX PPPPNNNNXXXX * PPPP NNNN PPPPCCCCNNNN
* might become might become * might become might become
* PPPPPPPPPPPP 1 or PPPPPPPPPPPP 6 or * PPPPPPPPPPPP 1 or PPPPPPPPPPPP 6 or
* PPPPPPPPXXXX 2 or PPPPPPPPXXXX 7 or * PPPPPPPPNNNN 2 or PPPPPPPPNNNN 7 or
* PPPPXXXXXXXX 3 PPPPXXXXXXXX 8 * PPPPNNNNNNNN 3 PPPPNNNNNNNN 8
* *
* It is important for case 8 that the vma NNNN overlapping the * It is important for case 8 that the vma CCCC overlapping the
* region AAAA is never going to extended over XXXX. Instead XXXX must * region **** is never going to extended over NNNN. Instead NNNN must
* be extended in region AAAA and NNNN must be removed. This way in * be extended in region **** and CCCC must be removed. This way in
* all cases where vma_merge succeeds, the moment vma_merge drops the * all cases where vma_merge succeeds, the moment vma_merge drops the
* rmap_locks, the properties of the merged vma will be already * rmap_locks, the properties of the merged vma will be already
* correct for the whole merged range. Some of those properties like * 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 * vm_page_prot/vm_flags may be accessed by rmap_walks and they must
* be correct for the whole merged range immediately after the * be correct for the whole merged range immediately after the
* rmap_locks are released. Otherwise if XXXX would be removed and * rmap_locks are released. Otherwise if NNNN would be removed and
* NNNN would be extended over the XXXX range, remove_migration_ptes * CCCC would be extended over the NNNN range, remove_migration_ptes
* or other rmap walkers (if working on addresses beyond the "end" * or other rmap walkers (if working on addresses beyond the "end"
* parameter) may establish ptes with the wrong permissions of NNNN * parameter) may establish ptes with the wrong permissions of CCCC
* instead of the right permissions of XXXX. * instead of the right permissions of NNNN.
* *
* In the code below: * In the code below:
* PPPP is represented by *prev * PPPP is represented by *prev
* NNNN is represented by *mid or not represented at all (NULL) * CCCC is represented by *curr or not represented at all (NULL)
* XXXX is represented by *next or not represented at all (NULL) * NNNN is represented by *next or not represented at all (NULL)
* AAAA is not represented - it will be merged and the vma containing the * **** is not represented - it will be merged and the vma containing the
* area is returned, or the function will return NULL * area is returned, or the function will return NULL
*/ */
struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
...@@ -898,7 +898,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, ...@@ -898,7 +898,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
{ {
pgoff_t pglen = (end - addr) >> PAGE_SHIFT; pgoff_t pglen = (end - addr) >> PAGE_SHIFT;
pgoff_t vma_pgoff; 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; struct vm_area_struct *vma, *adjust, *remove, *remove2;
int err = -1; int err = -1;
bool merge_prev = false; bool merge_prev = false;
...@@ -917,19 +917,19 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, ...@@ -917,19 +917,19 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
if (vm_flags & VM_SPECIAL) if (vm_flags & VM_SPECIAL)
return NULL; return NULL;
mid = find_vma(mm, prev ? prev->vm_end : 0); curr = find_vma(mm, prev ? prev->vm_end : 0);
if (mid && mid->vm_end == end) /* cases 6, 7, 8 */ if (curr && curr->vm_end == end) /* cases 6, 7, 8 */
next = find_vma(mm, mid->vm_end); next = find_vma(mm, curr->vm_end);
else else
next = mid; next = curr;
/* In cases 1 - 4 there's no NNNN vma */ /* In cases 1 - 4 there's no CCCC vma */
if (mid && end <= mid->vm_start) if (curr && end <= curr->vm_start)
mid = NULL; curr = NULL;
/* verify some invariant that must be enforced by the caller */ /* verify some invariant that must be enforced by the caller */
VM_WARN_ON(prev && addr <= prev->vm_start); 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); VM_WARN_ON(addr >= end);
if (prev) { if (prev) {
...@@ -961,21 +961,21 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, ...@@ -961,21 +961,21 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
remove = next; /* case 1 */ remove = next; /* case 1 */
vma_end = next->vm_end; vma_end = next->vm_end;
err = dup_anon_vma(prev, next); err = dup_anon_vma(prev, next);
if (mid) { /* case 6 */ if (curr) { /* case 6 */
remove = mid; remove = curr;
remove2 = next; remove2 = next;
if (!next->anon_vma) if (!next->anon_vma)
err = dup_anon_vma(prev, mid); err = dup_anon_vma(prev, curr);
} }
} else if (merge_prev) { } else if (merge_prev) {
err = 0; /* case 2 */ err = 0; /* case 2 */
if (mid) { if (curr) {
err = dup_anon_vma(prev, mid); err = dup_anon_vma(prev, curr);
if (end == mid->vm_end) { /* case 7 */ if (end == curr->vm_end) { /* case 7 */
remove = mid; remove = curr;
} else { /* case 5 */ } else { /* case 5 */
adjust = mid; adjust = curr;
adj_start = (end - mid->vm_start); adj_start = (end - curr->vm_start);
} }
} }
} else if (merge_next) { } else if (merge_next) {
...@@ -991,10 +991,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, ...@@ -991,10 +991,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
vma_end = next->vm_end; vma_end = next->vm_end;
vma_pgoff = next->vm_pgoff; vma_pgoff = next->vm_pgoff;
err = 0; err = 0;
if (mid) { /* case 8 */ if (curr) { /* case 8 */
vma_pgoff = mid->vm_pgoff; vma_pgoff = curr->vm_pgoff;
remove = mid; remove = curr;
err = dup_anon_vma(next, mid); err = dup_anon_vma(next, curr);
} }
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment