lkml.org 
[lkml]   [2023]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 02/10] mm/mmap/vma_merge: use the proper vma pointer in case 3
    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>

    \
     
     \ /
      Last update: 2023-03-27 01:01    [W:4.147 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site