| Message ID | 4d717269303d8a6fe1d837968c252eeb6ff1d7e5.1679137163.git.lstoakes@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | further cleanup of vma_merge() | expand |
On Sat, Mar 18, 2023 at 11:13:19AM +0000, Lorenzo Stoakes wrote: > We are only interested in next if end == next->vm_start (in which case we > check to see if we can set merge_next), so perform this check alongside > checking whether curr should be set. > > This groups all of the simple range checks together and establishes the > invariant that, if prev, curr or next are non-NULL then their positions are > as expected. > > Additionally, use the abstract 'vma' object to look up the possible curr or > next VMA in order to avoid any confusion as to what these variables > represent - now curr and next are assigned once and only once. Hi Lorenzo, Due to the "vma" variable is used as an intermediate member, I feels a bit confusing, so cleanup this patch as below. If this cleanup patch is issue, please let me know, and then ignore it directly, thanks. ---- From 7dac3ed8c1b747c2edf2a6c867c4e6342c7447c3 Mon Sep 17 00:00:00 2001 From: Vernon Yang <vernon2gm@gmail.com> Date: Mon, 20 Mar 2023 21:38:09 +0800 Subject: [PATCH] mm/mmap/vma_merge: set next to NULL if not applicable-cleanup Make code logic simpler and more readable. Signed-off-by: Vernon Yang <vernon2gm@gmail.com> --- mm/mmap.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index 45bd43225013..78cb96774602 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -921,7 +921,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, * If there is a previous VMA specified, find the next, otherwise find * the first. */ - vma = find_vma(mm, prev ? prev->vm_end : 0); + curr = find_vma(mm, prev ? prev->vm_end : 0); /* * Does the input range span an existing VMA? If so, we designate this @@ -929,21 +929,19 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, * * Cases 5 - 8. */ - if (vma && end > vma->vm_start) { - curr = vma; - + if (curr && end > curr->vm_start) { /* * If the addr - end range spans this VMA entirely, then we * check to see if another VMA follows it. * - * If it is _immediately_ adjacent (checked below), then we + * If it is immediately adjacent (checked below), then we * designate it 'next' (cases 6 - 8). */ if (curr->vm_end == end) - vma = find_vma(mm, curr->vm_end); + next = find_vma(mm, curr->vm_end); else /* Case 5. */ - vma = NULL; + next = NULL; } else { /* * The addr - end range either spans the end of prev or spans no @@ -952,19 +950,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, * * Cases 1 - 4. */ + next = curr; curr = NULL; } - /* - * We only actually examine the next VMA if it is immediately adjacent - * to end which sits either at the end of a hole (cases 1 - 3), PPPP - * (case 4) or CCCC (cases 6 - 8). - */ - if (vma && end == vma->vm_start) - next = vma; - else - next = NULL; - /* * By default, we return prev. Cases 3, 4, 8 will instead return next * and cases 3, 8 will also update vma to point at next. -- 2.34.1 > > This has no functional impact. > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> > --- > mm/mmap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 49 insertions(+), 12 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index c9834364ac98..66893fc72e03 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -930,15 +930,53 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (vm_flags & VM_SPECIAL) > return NULL; > > - 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 = curr; > + /* > + * If there is a previous VMA specified, find the next, otherwise find > + * the first. > + */ > + vma = find_vma(mm, prev ? prev->vm_end : 0); > + > + /* > + * Does the input range span an existing VMA? If so, we designate this > + * VMA 'curr'. The caller will have ensured that curr->vm_start == addr. > + * > + * Cases 5 - 8. > + */ > + if (vma && end > vma->vm_start) { > + curr = vma; > > - /* In cases 1 - 4 there's no CCCC vma */ > - if (curr && end <= curr->vm_start) > + /* > + * If the addr - end range spans this VMA entirely, then we > + * check to see if another VMA follows it. > + * > + * If it is _immediately_ adjacent (checked below), then we > + * designate it 'next' (cases 6 - 8). > + */ > + if (curr->vm_end == end) > + vma = find_vma(mm, curr->vm_end); > + else > + /* Case 5. */ > + vma = NULL; > + } else { > + /* > + * The addr - end range either spans the end of prev or spans no > + * VMA at all - in either case we dispense with 'curr' and > + * maintain only 'prev' and (possibly) 'next'. > + * > + * Cases 1 - 4. > + */ > curr = NULL; > + } > + > + /* > + * We only actually examine the next VMA if it is immediately adjacent > + * to end which sits either at the end of a hole (cases 1 - 3), PPPP > + * (case 4) or CCCC (cases 6 - 8). > + */ > + if (vma && end == vma->vm_start) > + next = vma; > + else > + next = NULL; > > /* verify some invariant that must be enforced by the caller */ > VM_WARN_ON(prev && addr <= prev->vm_start); > @@ -959,11 +997,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > } > } > /* 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)) { > + if (next && 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; > } > > > -- > 2.39.2 >
On Mon, Mar 20, 2023 at 10:25:11PM +0800, Vernon Yang wrote: > On Sat, Mar 18, 2023 at 11:13:19AM +0000, Lorenzo Stoakes wrote: > > We are only interested in next if end == next->vm_start (in which case we > > check to see if we can set merge_next), so perform this check alongside > > checking whether curr should be set. > > > > This groups all of the simple range checks together and establishes the > > invariant that, if prev, curr or next are non-NULL then their positions are > > as expected. > > > > Additionally, use the abstract 'vma' object to look up the possible curr or > > next VMA in order to avoid any confusion as to what these variables > > represent - now curr and next are assigned once and only once. > > Hi Lorenzo, > > Due to the "vma" variable is used as an intermediate member, I feels a bit > confusing, so cleanup this patch as below. Hi Vernon, if you read the commit message you'll see what you're undoing here is exactly what I have done on purpose. The point is to assign each of curr and next once and only once after we've determined which of the two we are assigning to. You also delete some of the explanation which I added explicitly to make the logic clearer and adjust _punctionation_ neither of which I feel are positive changes. The point is that existing logic treated either mid or curr as temporary variables that might get reassigned. By using a temporary value and explaining what we're doing, you can see _why_ we are assigning it. > > If this cleanup patch is issue, please let me know, and then ignore it > directly, thanks. So I'm afraid I am not in favour of your change, though thank you for your interest! I am happy to get feedback on the change, though I'd suggest commenting on anything you have issues with rather than attempting to rework my change as if we start getting patches within patches it's going to end like inception :) Cheers, Lorenzo > > ---- > From 7dac3ed8c1b747c2edf2a6c867c4e6342c7447c3 Mon Sep 17 00:00:00 2001 > From: Vernon Yang <vernon2gm@gmail.com> > Date: Mon, 20 Mar 2023 21:38:09 +0800 > Subject: [PATCH] mm/mmap/vma_merge: set next to NULL if not applicable-cleanup > > Make code logic simpler and more readable. > > Signed-off-by: Vernon Yang <vernon2gm@gmail.com> > --- > mm/mmap.c | 23 ++++++----------------- > 1 file changed, 6 insertions(+), 17 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 45bd43225013..78cb96774602 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -921,7 +921,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > * If there is a previous VMA specified, find the next, otherwise find > * the first. > */ > - vma = find_vma(mm, prev ? prev->vm_end : 0); > + curr = find_vma(mm, prev ? prev->vm_end : 0); > > /* > * Does the input range span an existing VMA? If so, we designate this > @@ -929,21 +929,19 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > * > * Cases 5 - 8. > */ > - if (vma && end > vma->vm_start) { > - curr = vma; > - > + if (curr && end > curr->vm_start) { > /* > * If the addr - end range spans this VMA entirely, then we > * check to see if another VMA follows it. > * > - * If it is _immediately_ adjacent (checked below), then we > + * If it is immediately adjacent (checked below), then we > * designate it 'next' (cases 6 - 8). > */ > if (curr->vm_end == end) > - vma = find_vma(mm, curr->vm_end); > + next = find_vma(mm, curr->vm_end); > else > /* Case 5. */ > - vma = NULL; > + next = NULL; > } else { > /* > * The addr - end range either spans the end of prev or spans no > @@ -952,19 +950,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > * > * Cases 1 - 4. > */ > + next = curr; > curr = NULL; > } > > - /* > - * We only actually examine the next VMA if it is immediately adjacent > - * to end which sits either at the end of a hole (cases 1 - 3), PPPP > - * (case 4) or CCCC (cases 6 - 8). > - */ > - if (vma && end == vma->vm_start) > - next = vma; > - else > - next = NULL; > - > /* > * By default, we return prev. Cases 3, 4, 8 will instead return next > * and cases 3, 8 will also update vma to point at next. > -- > 2.34.1 > > > > > > This has no functional impact. > > > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> > > --- > > mm/mmap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- > > 1 file changed, 49 insertions(+), 12 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index c9834364ac98..66893fc72e03 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -930,15 +930,53 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > if (vm_flags & VM_SPECIAL) > > return NULL; > > > > - 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 = curr; > > + /* > > + * If there is a previous VMA specified, find the next, otherwise find > > + * the first. > > + */ > > + vma = find_vma(mm, prev ? prev->vm_end : 0); > > + > > + /* > > + * Does the input range span an existing VMA? If so, we designate this > > + * VMA 'curr'. The caller will have ensured that curr->vm_start == addr. > > + * > > + * Cases 5 - 8. > > + */ > > + if (vma && end > vma->vm_start) { > > + curr = vma; > > > > - /* In cases 1 - 4 there's no CCCC vma */ > > - if (curr && end <= curr->vm_start) > > + /* > > + * If the addr - end range spans this VMA entirely, then we > > + * check to see if another VMA follows it. > > + * > > + * If it is _immediately_ adjacent (checked below), then we > > + * designate it 'next' (cases 6 - 8). > > + */ > > + if (curr->vm_end == end) > > + vma = find_vma(mm, curr->vm_end); > > + else > > + /* Case 5. */ > > + vma = NULL; > > + } else { > > + /* > > + * The addr - end range either spans the end of prev or spans no > > + * VMA at all - in either case we dispense with 'curr' and > > + * maintain only 'prev' and (possibly) 'next'. > > + * > > + * Cases 1 - 4. > > + */ > > curr = NULL; > > + } > > + > > + /* > > + * We only actually examine the next VMA if it is immediately adjacent > > + * to end which sits either at the end of a hole (cases 1 - 3), PPPP > > + * (case 4) or CCCC (cases 6 - 8). > > + */ > > + if (vma && end == vma->vm_start) > > + next = vma; > > + else > > + next = NULL; > > > > /* verify some invariant that must be enforced by the caller */ > > VM_WARN_ON(prev && addr <= prev->vm_start); > > @@ -959,11 +997,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > } > > } > > /* 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)) { > > + if (next && 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; > > } > > > > > > -- > > 2.39.2 > >
* Lorenzo Stoakes <lstoakes@gmail.com> [230318 07:15]: > We are only interested in next if end == next->vm_start (in which case we > check to see if we can set merge_next), so perform this check alongside > checking whether curr should be set. > > This groups all of the simple range checks together and establishes the > invariant that, if prev, curr or next are non-NULL then their positions are > as expected. > > Additionally, use the abstract 'vma' object to look up the possible curr or > next VMA in order to avoid any confusion as to what these variables > represent - now curr and next are assigned once and only once. > > This has no functional impact. > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> > --- > mm/mmap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 49 insertions(+), 12 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index c9834364ac98..66893fc72e03 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -930,15 +930,53 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > if (vm_flags & VM_SPECIAL) > return NULL; > > - 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 = curr; > + /* > + * If there is a previous VMA specified, find the next, otherwise find > + * the first. > + */ > + vma = find_vma(mm, prev ? prev->vm_end : 0); > + > + /* > + * Does the input range span an existing VMA? If so, we designate this > + * VMA 'curr'. The caller will have ensured that curr->vm_start == addr. > + * > + * Cases 5 - 8. > + */ > + if (vma && end > vma->vm_start) { > + curr = vma; It might be better to set: curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end); > > - /* In cases 1 - 4 there's no CCCC vma */ > - if (curr && end <= curr->vm_start) > + /* > + * If the addr - end range spans this VMA entirely, then we > + * check to see if another VMA follows it. > + * > + * If it is _immediately_ adjacent (checked below), then we > + * designate it 'next' (cases 6 - 8). > + */ > + if (curr->vm_end == end) > + vma = find_vma(mm, curr->vm_end); You can change this to: next = vma_lookup(mm, curr->vm_end); Then you don't need to validate below, in this case. > + else > + /* Case 5. */ > + vma = NULL; > + } else { > + /* > + * The addr - end range either spans the end of prev or spans no > + * VMA at all - in either case we dispense with 'curr' and > + * maintain only 'prev' and (possibly) 'next'. Possibly next here would be: next = vma_lookup(mm, end); I think? > + * > + * Cases 1 - 4. > + */ > curr = NULL; > + } > + > + /* > + * We only actually examine the next VMA if it is immediately adjacent > + * to end which sits either at the end of a hole (cases 1 - 3), PPPP > + * (case 4) or CCCC (cases 6 - 8). > + */ > + if (vma && end == vma->vm_start) > + next = vma; > + else > + next = NULL; If I'm correct above, then we can drop this next checking. > > /* verify some invariant that must be enforced by the caller */ > VM_WARN_ON(prev && addr <= prev->vm_start); > @@ -959,11 +997,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > } > } > /* 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)) { > + if (next && mpol_equal(policy, vma_policy(next)) && > + can_vma_merge_before(next, vm_flags, > + anon_vma, file, pgoff+pglen, > + vm_userfaultfd_ctx, anon_name)) { I think we can keep this chunk with the next = vma_lookup() changes as well. > merge_next = true; > } > > -- > 2.39.2 >
On Mon, Mar 20, 2023 at 12:27:08PM -0400, Liam R. Howlett wrote: > * Lorenzo Stoakes <lstoakes@gmail.com> [230318 07:15]: > > We are only interested in next if end == next->vm_start (in which case we > > check to see if we can set merge_next), so perform this check alongside > > checking whether curr should be set. > > > > This groups all of the simple range checks together and establishes the > > invariant that, if prev, curr or next are non-NULL then their positions are > > as expected. > > > > Additionally, use the abstract 'vma' object to look up the possible curr or > > next VMA in order to avoid any confusion as to what these variables > > represent - now curr and next are assigned once and only once. > > > > This has no functional impact. > > > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> > > --- > > mm/mmap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- > > 1 file changed, 49 insertions(+), 12 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index c9834364ac98..66893fc72e03 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -930,15 +930,53 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > if (vm_flags & VM_SPECIAL) > > return NULL; > > > > - 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 = curr; > > + /* > > + * If there is a previous VMA specified, find the next, otherwise find > > + * the first. > > + */ > > + vma = find_vma(mm, prev ? prev->vm_end : 0); > > + > > + /* > > + * Does the input range span an existing VMA? If so, we designate this > > + * VMA 'curr'. The caller will have ensured that curr->vm_start == addr. > > + * > > + * Cases 5 - 8. > > + */ > > + if (vma && end > vma->vm_start) { > > + curr = vma; > > It might be better to set: > curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end); > > > > > - /* In cases 1 - 4 there's no CCCC vma */ > > - if (curr && end <= curr->vm_start) > > + /* > > + * If the addr - end range spans this VMA entirely, then we > > + * check to see if another VMA follows it. > > + * > > + * If it is _immediately_ adjacent (checked below), then we > > + * designate it 'next' (cases 6 - 8). > > + */ > > + if (curr->vm_end == end) > > + vma = find_vma(mm, curr->vm_end); > > You can change this to: > next = vma_lookup(mm, curr->vm_end); > Then you don't need to validate below, in this case. > > > + else > > + /* Case 5. */ > > + vma = NULL; > > > > + } else { > > + /* > > + * The addr - end range either spans the end of prev or spans no > > + * VMA at all - in either case we dispense with 'curr' and > > + * maintain only 'prev' and (possibly) 'next'. > > Possibly next here would be: > next = vma_lookup(mm, end); > I think? > > > + * > > + * Cases 1 - 4. > > + */ > > curr = NULL; > > + } > > + > > + /* > > + * We only actually examine the next VMA if it is immediately adjacent > > + * to end which sits either at the end of a hole (cases 1 - 3), PPPP > > + * (case 4) or CCCC (cases 6 - 8). > > + */ > > + if (vma && end == vma->vm_start) > > + next = vma; > > + else > > + next = NULL; > > If I'm correct above, then we can drop this next checking. > > > > > /* verify some invariant that must be enforced by the caller */ > > VM_WARN_ON(prev && addr <= prev->vm_start); > > @@ -959,11 +997,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > } > > } > > /* 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)) { > > + if (next && mpol_equal(policy, vma_policy(next)) && > > + can_vma_merge_before(next, vm_flags, > > + anon_vma, file, pgoff+pglen, > > + vm_userfaultfd_ctx, anon_name)) { > > I think we can keep this chunk with the next = vma_lookup() changes as > well. > > > merge_next = true; > > } > > > > -- > > 2.39.2 > > Thanks, I will investigate all of these and will try to apply everything that is workable from here + respin.
diff --git a/mm/mmap.c b/mm/mmap.c index c9834364ac98..66893fc72e03 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -930,15 +930,53 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, if (vm_flags & VM_SPECIAL) return NULL; - 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 = curr; + /* + * If there is a previous VMA specified, find the next, otherwise find + * the first. + */ + vma = find_vma(mm, prev ? prev->vm_end : 0); + + /* + * Does the input range span an existing VMA? If so, we designate this + * VMA 'curr'. The caller will have ensured that curr->vm_start == addr. + * + * Cases 5 - 8. + */ + if (vma && end > vma->vm_start) { + curr = vma; - /* In cases 1 - 4 there's no CCCC vma */ - if (curr && end <= curr->vm_start) + /* + * If the addr - end range spans this VMA entirely, then we + * check to see if another VMA follows it. + * + * If it is _immediately_ adjacent (checked below), then we + * designate it 'next' (cases 6 - 8). + */ + if (curr->vm_end == end) + vma = find_vma(mm, curr->vm_end); + else + /* Case 5. */ + vma = NULL; + } else { + /* + * The addr - end range either spans the end of prev or spans no + * VMA at all - in either case we dispense with 'curr' and + * maintain only 'prev' and (possibly) 'next'. + * + * Cases 1 - 4. + */ curr = NULL; + } + + /* + * We only actually examine the next VMA if it is immediately adjacent + * to end which sits either at the end of a hole (cases 1 - 3), PPPP + * (case 4) or CCCC (cases 6 - 8). + */ + if (vma && end == vma->vm_start) + next = vma; + else + next = NULL; /* verify some invariant that must be enforced by the caller */ VM_WARN_ON(prev && addr <= prev->vm_start); @@ -959,11 +997,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, } } /* 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)) { + if (next && 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; }
We are only interested in next if end == next->vm_start (in which case we check to see if we can set merge_next), so perform this check alongside checking whether curr should be set. This groups all of the simple range checks together and establishes the invariant that, if prev, curr or next are non-NULL then their positions are as expected. Additionally, use the abstract 'vma' object to look up the possible curr or next VMA in order to avoid any confusion as to what these variables represent - now curr and next are assigned once and only once. This has no functional impact. Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> --- mm/mmap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 12 deletions(-)