Messages in this thread | | | Date | Tue, 13 Aug 2019 13:25:34 +0800 | From | Wei Yang <> | Subject | Re: [PATCH] mm/mmap.c: rb_parent is not necessary in __vma_link_list |
| |
On Mon, Aug 12, 2019 at 08:39:58PM -0700, Matthew Wilcox wrote: >On Tue, Aug 13, 2019 at 11:26:56AM +0800, Wei Yang wrote: >> Now we use rb_parent to get next, while this is not necessary. >> >> When prev is NULL, this means vma should be the first element in the >> list. Then next should be current first one (mm->mmap), no matter >> whether we have parent or not. >> >> After removing it, the code shows the beauty of symmetry. > >Uhh ... did you test this? >
I reboot successfully with this patch.
>> @@ -273,12 +273,8 @@ void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma, >> next = prev->vm_next; >> prev->vm_next = vma; >> } else { >> + next = mm->mmap; >> mm->mmap = vma; >> - if (rb_parent) >> - next = rb_entry(rb_parent, >> - struct vm_area_struct, vm_rb); >> - else >> - next = NULL; >> } > >The full context is: > > if (prev) { > next = prev->vm_next; > prev->vm_next = vma; > } else { > mm->mmap = vma; > if (rb_parent) > next = rb_entry(rb_parent, > struct vm_area_struct, vm_rb); > else > next = NULL; > } > >Let's imagine we have a small tree with three ranges in it. > >A: 5-7 >B: 8-10 >C: 11-13 > >I would imagine an rbtree for this case has B at the top with A >to its left and B to its right. > >Now we're going to add range D at 3-4. 'next' should clearly be range A. >It will have NULL prev. Your code is going to make 'B' next, not A. >Right?
mm->mmap is not the rb_root.
mm->mmap is the first element in the ordered list, if my understanding is correct.
-- Wei Yang Help you, Help me
| |