On Sun, Feb 18, 2024 at 04:50:28PM +0800, Yajun Deng wrote: > If the prev vma exists and the end is less than the end of prev, we > can return NULL immediately. This reduces unnecessary operations. > > Signed-off-by: Yajun Deng <yajun.deng@xxxxxxxxx> Adding Vlastimil, while get_maintainers.pl might not show it very clearly, myself, Vlastimil and Liam often work with vma_merge() so it's handy to cc us on these if you can! > --- > mm/mmap.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 8f176027583c..b738849321c0 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -827,7 +827,7 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, > * > * **** **** **** > * PPPPPPNNNNNN PPPPPPNNNNNN PPPPPPCCCCCC > - * cannot merge might become might become > + * cannot merge 9 might become might become While I welcome your interest here :) I am not a fan of the 'case' approach to this function as-is and plan to heavily refactor this when I get a chance. But at any rate, an early-exit situation is not a merge case, merge cases describe cases where we _can_ merge, so we can drop this case 9 stuff (this is not your fault, it's understandable why you would label this, this function is just generally unclear). > * PPNNNNNNNNNN PPPPPPPPPPCC > * mmap, brk or case 4 below case 5 below > * mremap move: > @@ -890,6 +890,9 @@ static struct vm_area_struct > if (vm_flags & VM_SPECIAL) > return NULL; > > + if (prev && end < prev->vm_end) /* case 9 */ > + return NULL; > + I need to get back into vma_merge() head space, but I don't actually think a caller that's behaving correctly should ever do this. I know the ASCII diagram above lists it as a thing that can happen, but I think we implicitly avoid this from the way we invoke callers. Either prev == vma as per vma_merge_extend(), or the loops that invoke vma_merge_new_vma() wouldn't permit this to occur. Let me look into it more deeply + reply again a bit later, I mean we could perhaps do with asserting this somehow, but I don't think it's useful to do an early exit for something that ostensibly _shouldn't_ happen. > /* Does the input range span an existing VMA? (cases 5 - 8) */ > curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end); > > -- > 2.25.1 >
