* Yajun Deng <yajun.deng@xxxxxxxxx> [240219 22:00]: > > On 2024/2/19 07:03, Lorenzo Stoakes wrote: > > 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! > Okay. > > > --- > > > 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). > > Yes, it's not a merge case. I label this to make it easier to understand. But it isn't. It's not a case at all, it's a failure to merge. > > > > * 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. > No, it will actually happen. That's why I submitted this patch. Can you elaborate on where it happens? I mean, you seem to have already looked into it but haven't shared what you found of where it reduces the unnecessary operations. Such a detail should also be added to the commit log so that, when the call sites change, this check could be dropped - or be seen as necessary.
