| Message ID | 20230309111258.24079-3-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:50PM +0100, Vlastimil Babka wrote: > In case 3 we we use 'next' for everything but vma_pgoff. So use 'next' > for that as well, instead of 'mid', for consistency. Then in case 8 we > have to use 'mid' explicitly, which should also make the intent more > obvious. > > Adjust the diagram for cases 1-3 in the comment to match the code - we > are using 'next' for case 3 so mark the range with XXXX instead of NNNN. > For case 2 that's a no-op as the code doesn't touch 'next' or 'mid'. For > case 1 it's now wrong but that will be fixed next. > > No functional change. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/mmap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 0a8b052e3022..1af4c9bc2c87 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -857,11 +857,11 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, > * mmap, brk or case 4 below case 5 below > * mremap move: > * AAAA AAAA > - * PPPP NNNN PPPPNNNNXXXX > + * PPPP XXXX PPPPNNNNXXXX > * might become might become > * PPPPPPPPPPPP 1 or PPPPPPPPPPPP 6 or > - * PPPPPPPPNNNN 2 or PPPPPPPPXXXX 7 or > - * PPPPNNNNNNNN 3 PPPPXXXXXXXX 8 > + * PPPPPPPPXXXX 2 or PPPPPPPPXXXX 7 or > + * PPPPXXXXXXXX 3 PPPPXXXXXXXX 8 > * I'm glad you're making things more consistent and what you're addressing here is a real clanger, but these diagrams while great to have do definitely feel quite confusing even now. But that's something for a future patch! > * It is important for case 8 that the vma NNNN overlapping the > * region AAAA is never going to extended over XXXX. Instead XXXX must > @@ -978,9 +978,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > vma = next; /* case 3 */ > vma_start = addr; > vma_end = next->vm_end; > - vma_pgoff = mid->vm_pgoff; > + vma_pgoff = next->vm_pgoff; > err = 0; > if (mid != next) { /* case 8 */ > + vma_pgoff = mid->vm_pgoff; > remove = mid; > err = dup_anon_vma(next, mid); > } > -- > 2.39.2 > This does fix a big incongruity in that previously everything but vm_pgoff was relative to next, while in the non-8 case mid is equal to next anyway. Good, clarifying improvement! Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com>
diff --git a/mm/mmap.c b/mm/mmap.c index 0a8b052e3022..1af4c9bc2c87 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -857,11 +857,11 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, * mmap, brk or case 4 below case 5 below * mremap move: * AAAA AAAA - * PPPP NNNN PPPPNNNNXXXX + * PPPP XXXX PPPPNNNNXXXX * might become might become * PPPPPPPPPPPP 1 or PPPPPPPPPPPP 6 or - * PPPPPPPPNNNN 2 or PPPPPPPPXXXX 7 or - * PPPPNNNNNNNN 3 PPPPXXXXXXXX 8 + * PPPPPPPPXXXX 2 or PPPPPPPPXXXX 7 or + * PPPPXXXXXXXX 3 PPPPXXXXXXXX 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 @@ -978,9 +978,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, vma = next; /* case 3 */ vma_start = addr; vma_end = next->vm_end; - vma_pgoff = mid->vm_pgoff; + vma_pgoff = next->vm_pgoff; err = 0; if (mid != next) { /* case 8 */ + vma_pgoff = mid->vm_pgoff; remove = mid; err = dup_anon_vma(next, mid); }
In case 3 we we use 'next' for everything but vma_pgoff. So use 'next' for that as well, instead of 'mid', for consistency. Then in case 8 we have to use 'mid' explicitly, which should also make the intent more obvious. Adjust the diagram for cases 1-3 in the comment to match the code - we are using 'next' for case 3 so mark the range with XXXX instead of NNNN. For case 2 that's a no-op as the code doesn't touch 'next' or 'mid'. For case 1 it's now wrong but that will be fixed next. No functional change. Signed-off-by: Vlastimil Babka <vbabka@suse.cz> --- mm/mmap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)