On Mon, 6 Aug 2012, Hillf Danton wrote: > Both find_vma and find_vma_prev have code for walking rb tree, and we can > walk less. > > To cut the walk in find_vma_prev off, find_vma is changed to take care of > vm_prev while walking rb tree, and we end up wrapping find_vma_prev with > find_vma.
No thank you. Please take a look at what uses find_vma_prev() and what uses find_vma() nowadays - since Linus added vma->vm_prev, and Kosaki-san and Mikulas Patocka rewrote find_vma_prev() to use find_vma(). Almost nothing uses find_vma_prev(): parisc (stack grows upwards) and ia64 (register backing store grows upwards) have particular need for it (when they want to know the vm_prev of top of memory). madvise and tile are still using it, but I expect should just be converted to find_vma(). And rarely is its prev search actually required. Whereas there are lots of users of find_vma(), who want it to be as quick as possible: it should not be burdened with almost-never-needed extras. When he has a moment, Kosaki-san intends to remove find_vma_prev() altogether, replacing it by find_last_vma() for parisc and ia64. > > btw, what happened to LKML? I don't know what you're referring to: what happened to LKML? Hugh > > Signed-off-by: Hillf Danton <[email protected]> > --- > > --- a/mm/mmap.c Fri Aug 3 07:38:10 2012 > +++ b/mm/mmap.c Mon Aug 6 20:10:18 2012 > @@ -1602,11 +1602,18 @@ get_unmapped_area(struct file *file, uns > > EXPORT_SYMBOL(get_unmapped_area); > > -/* Look up the first VMA which satisfies addr < vm_end, NULL if none. */ > -struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) > +/* > + * Look up the first VMA which satisfies addr < vm_end, NULL if none. > + * Also return a pointer to the previous VMA. > + */ > +struct vm_area_struct * > +find_vma_prev(struct mm_struct *mm, unsigned long addr, > + struct vm_area_struct **pprev) > { > struct vm_area_struct *vma = NULL; > > + *pprev = NULL; /* Should be removed with WARN_ON_ONCE(!mm) */ > + > if (WARN_ON_ONCE(!mm)) /* Remove this in linux-3.6 */ > return NULL; > > @@ -1630,39 +1637,29 @@ struct vm_area_struct *find_vma(struct m > if (vma_tmp->vm_start <= addr) > break; > rb_node = rb_node->rb_left; > - } else > + } else { > rb_node = rb_node->rb_right; > + *pprev = vma_tmp; > + } > } > - if (vma) > + if (vma) { > mm->mmap_cache = vma; > + /* remove false positive produced while walking rb tree > */ > + *pprev = vma->vm_prev; > + } > + } else { > + *pprev = vma->vm_prev; > } > return vma; > } > > -EXPORT_SYMBOL(find_vma); > - > -/* > - * Same as find_vma, but also return a pointer to the previous VMA in *pprev. > - */ > -struct vm_area_struct * > -find_vma_prev(struct mm_struct *mm, unsigned long addr, > - struct vm_area_struct **pprev) > +struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) > { > - struct vm_area_struct *vma; > + struct vm_area_struct *prev; > > - vma = find_vma(mm, addr); > - if (vma) { > - *pprev = vma->vm_prev; > - } else { > - struct rb_node *rb_node = mm->mm_rb.rb_node; > - *pprev = NULL; > - while (rb_node) { > - *pprev = rb_entry(rb_node, struct vm_area_struct, > vm_rb); > - rb_node = rb_node->rb_right; > - } > - } > - return vma; > + return find_vma_prev(mm, addr, &prev); > } > +EXPORT_SYMBOL(find_vma); > > /* > * Verify that the stack growth is acceptable and > -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
