Messages in this thread | | | Date | Mon, 12 Aug 2019 20:39:58 -0700 | From | Matthew Wilcox <> | Subject | Re: [PATCH] mm/mmap.c: rb_parent is not necessary in __vma_link_list |
| |
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?
> @@ -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?
| |