| Message ID | 20230309111258.24079-8-vbabka@suse.cz (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | cleanup vma_merge() and improve mergeability tests | expand |
On Thu, Mar 09, 2023 at 12:12:55PM +0100, Vlastimil Babka wrote: > The variable 'adj_next' holds the value by which we adjust vm_start of a > vma in variable 'adjust', that's either 'next' or 'mid', so the current > name is inaccurate. Rename it to 'adj_start'. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/mmap.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 3396c9b13f1c..c51d69592e4e 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -903,7 +903,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > bool vma_expanded = false; > struct vma_prepare vp; > unsigned long vma_end = end; > - long adj_next = 0; > + long adj_start = 0; > unsigned long vma_start = addr; > > validate_mm(mm); > @@ -971,7 +971,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > remove = mid; > } else { /* case 5 */ > adjust = mid; > - adj_next = (end - mid->vm_start); > + adj_start = (end - mid->vm_start); > } > } > } else if (merge_next) { > @@ -979,7 +979,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (prev && addr < prev->vm_end) { /* case 4 */ > vma_end = addr; > adjust = next; > - adj_next = -(prev->vm_end - addr); > + adj_start = -(prev->vm_end - addr); > err = dup_anon_vma(next, prev); > } else { > vma = next; /* case 3 */ > @@ -1002,7 +1002,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (vma_iter_prealloc(vmi)) > return NULL; > > - vma_adjust_trans_huge(vma, vma_start, vma_end, adj_next); > + vma_adjust_trans_huge(vma, vma_start, vma_end, adj_start); > init_multi_vma_prep(&vp, vma, adjust, remove, remove2); > VM_WARN_ON(vp.anon_vma && adjust && adjust->anon_vma && > vp.anon_vma != adjust->anon_vma); > @@ -1018,10 +1018,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (vma_expanded) > vma_iter_store(vmi, vma); > > - if (adj_next) { > - adjust->vm_start += adj_next; > - adjust->vm_pgoff += adj_next >> PAGE_SHIFT; > - if (adj_next < 0) { > + if (adj_start) { > + adjust->vm_start += adj_start; > + adjust->vm_pgoff += adj_start >> PAGE_SHIFT; > + if (adj_start < 0) { > WARN_ON(vma_expanded); > vma_iter_store(vmi, next); > } > -- > 2.39.2 > It looks like Suren's dc72d59c416d ("mm/mmap: move vma_prepare before vma_adjust_trans_huge") trivially conflicts with this change.
On Thu, Mar 09, 2023 at 12:12:55PM +0100, Vlastimil Babka wrote: > The variable 'adj_next' holds the value by which we adjust vm_start of a > vma in variable 'adjust', that's either 'next' or 'mid', so the current > name is inaccurate. Rename it to 'adj_start'. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/mmap.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 3396c9b13f1c..c51d69592e4e 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -903,7 +903,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > bool vma_expanded = false; > struct vma_prepare vp; > unsigned long vma_end = end; > - long adj_next = 0; > + long adj_start = 0; > unsigned long vma_start = addr; > > validate_mm(mm); > @@ -971,7 +971,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > remove = mid; > } else { /* case 5 */ > adjust = mid; > - adj_next = (end - mid->vm_start); > + adj_start = (end - mid->vm_start); > } > } > } else if (merge_next) { > @@ -979,7 +979,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (prev && addr < prev->vm_end) { /* case 4 */ > vma_end = addr; > adjust = next; > - adj_next = -(prev->vm_end - addr); > + adj_start = -(prev->vm_end - addr); > err = dup_anon_vma(next, prev); > } else { > vma = next; /* case 3 */ > @@ -1002,7 +1002,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (vma_iter_prealloc(vmi)) > return NULL; > > - vma_adjust_trans_huge(vma, vma_start, vma_end, adj_next); > + vma_adjust_trans_huge(vma, vma_start, vma_end, adj_start); > init_multi_vma_prep(&vp, vma, adjust, remove, remove2); > VM_WARN_ON(vp.anon_vma && adjust && adjust->anon_vma && > vp.anon_vma != adjust->anon_vma); > @@ -1018,10 +1018,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (vma_expanded) > vma_iter_store(vmi, vma); > > - if (adj_next) { > - adjust->vm_start += adj_next; > - adjust->vm_pgoff += adj_next >> PAGE_SHIFT; > - if (adj_next < 0) { > + if (adj_start) { > + adjust->vm_start += adj_start; > + adjust->vm_pgoff += adj_start >> PAGE_SHIFT; > + if (adj_start < 0) { > WARN_ON(vma_expanded); > vma_iter_store(vmi, next); > } > -- > 2.39.2 > This feels like a very silly oversight in the first instance but I imagine one that is a result of 'code evolution' :) excellent improvement in clarity, so:- Reviewed-By: Lorenzo Stoakes <lstoakes@gmail.com>
diff --git a/mm/mmap.c b/mm/mmap.c index 3396c9b13f1c..c51d69592e4e 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -903,7 +903,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, bool vma_expanded = false; struct vma_prepare vp; unsigned long vma_end = end; - long adj_next = 0; + long adj_start = 0; unsigned long vma_start = addr; validate_mm(mm); @@ -971,7 +971,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, remove = mid; } else { /* case 5 */ adjust = mid; - adj_next = (end - mid->vm_start); + adj_start = (end - mid->vm_start); } } } else if (merge_next) { @@ -979,7 +979,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, if (prev && addr < prev->vm_end) { /* case 4 */ vma_end = addr; adjust = next; - adj_next = -(prev->vm_end - addr); + adj_start = -(prev->vm_end - addr); err = dup_anon_vma(next, prev); } else { vma = next; /* case 3 */ @@ -1002,7 +1002,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, if (vma_iter_prealloc(vmi)) return NULL; - vma_adjust_trans_huge(vma, vma_start, vma_end, adj_next); + vma_adjust_trans_huge(vma, vma_start, vma_end, adj_start); init_multi_vma_prep(&vp, vma, adjust, remove, remove2); VM_WARN_ON(vp.anon_vma && adjust && adjust->anon_vma && vp.anon_vma != adjust->anon_vma); @@ -1018,10 +1018,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, if (vma_expanded) vma_iter_store(vmi, vma); - if (adj_next) { - adjust->vm_start += adj_next; - adjust->vm_pgoff += adj_next >> PAGE_SHIFT; - if (adj_next < 0) { + if (adj_start) { + adjust->vm_start += adj_start; + adjust->vm_pgoff += adj_start >> PAGE_SHIFT; + if (adj_start < 0) { WARN_ON(vma_expanded); vma_iter_store(vmi, next); }
The variable 'adj_next' holds the value by which we adjust vm_start of a vma in variable 'adjust', that's either 'next' or 'mid', so the current name is inaccurate. Rename it to 'adj_start'. Signed-off-by: Vlastimil Babka <vbabka@suse.cz> --- mm/mmap.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)