Messages in this thread | | | Subject | Re: [PATCH v2] mm: simplify find_vma_prev | From | Joe Perches <> | Date | Fri, 09 Dec 2011 13:35:26 -0800 |
| |
On Fri, 2011-12-09 at 16:23 -0500, kosaki.motohiro@gmail.com wrote: > commit 297c5eee37 (mm: make the vma list be doubly linked) added > vm_prev member into vm_area_struct. Therefore we can simplify > find_vma_prev() by using it. Also, this change help to improve > page fault performance because it has strong locality of reference.
trivia:
> diff --git a/mm/mmap.c b/mm/mmap.c [] > @@ -1603,39 +1603,21 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) > > EXPORT_SYMBOL(find_vma); > > -/* Same as find_vma, but also return a pointer to the previous VMA in *pprev. */ > +/* > + * Same as find_vma, but also return a pointer to the previous VMA in *pprev. > + * Note: pprev is set to NULL when return value is NULL. > + */ > struct vm_area_struct * > -find_vma_prev(struct mm_struct *mm, unsigned long addr, > - struct vm_area_struct **pprev)
> +find_vma_prev(struct mm_struct *mm, unsigned long addr, struct vm_area_struct **pprev)
eh. This declaration change seems gratuitous and it exceeds 80 columns.
> + *pprev = NULL; > + vma = find_vma(mm, addr); > + if (vma) > + *pprev = vma->vm_prev;
There's no need to possibly set *pprev twice.
Maybe { struct vm_area_struct *vma = find_vma(mm, addr);
*pprev = vma ? vma->vm_prev : NULL; or if (vma) *pprev = vma->vm_prev; else *pprev = NULL;
return vma; } > -out: > - *pprev = prev; > - return prev ? prev->vm_next : vma; > + return vma; > } > > /*
| |