* Vlastimil Babka <vbabka@xxxxxxx> [220118 06:57]: > On 12/1/21 15:30, Liam Howlett wrote: > > From: "Liam R. Howlett" <Liam.Howlett@xxxxxxxxxx> > > > > do_brk_munmap() has already aligned the address and has a maple tree > > state to be used. Use the new do_mas_align_munmap() to avoid > > unnecessary alignment and error checks. > > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> > > --- > > mm/mmap.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 14190306a483..79b8494d83c6 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -2926,12 +2926,15 @@ static int do_brk_munmap(struct ma_state *mas, struct vm_area_struct *vma, > > struct mm_struct *mm = vma->vm_mm; > > struct vm_area_struct unmap; > > unsigned long unmap_pages; > > - int ret = 1; > > + int ret; > > > > arch_unmap(mm, newbrk, oldbrk); > > > > - if (likely(vma->vm_start >= newbrk)) { // remove entire mapping(s) > > - ret = do_mas_munmap(mas, mm, newbrk, oldbrk-newbrk, uf, true); > > + if (likely((vma->vm_end < oldbrk) || > > + ((vma->vm_start == newbrk) && (vma->vm_end == oldbrk)))) { > > Can you describe this change of conditions in the commit log as well? Yes, that's fair as it's not easy to see. The second part is to catch an exact match of the vma - simple enough. The first part however, was changed to catch a tricky mprotect scenario that can land us into a VMA which is located at a lower address than the end of the brk. So we know we have to at least munmap one entire VMA. Checking the start of the VMA against the newbrk may not catch this scenario. And now that I've explained it, I think this change should be backported to the earlier patch in the series: mm/mmap: Change do_brk_flags() to expand existing VMA and add do_brk_munmap() > > > + // remove entire mapping(s) > > + ret = do_mas_align_munmap(mas, vma, mm, newbrk, oldbrk, uf, > > + true); > > goto munmap_full_vma; > > } > > >