Re: [PATCH v3] mm: simplify find_vma_prev

[KOSAKI Motohiro]

(12/12/11 8:26 AM), Michal Hocko wrote:
> On Fri 09-12-11 17:48:40, kosaki.motohiro@xxxxxxxxx wrote:
> > From: KOSAKI Motohiro<kosaki.motohiro@xxxxxxxxxxxxxx>
> >
> > 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.
> >
> > Signed-off-by: KOSAKI Motohiro<kosaki.motohiro@xxxxxxxxxxxxxx>
> > ---
> >   mm/mmap.c |   36 ++++++++----------------------------
> >   1 files changed, 8 insertions(+), 28 deletions(-)
> >
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index eae90af..b9c0241 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1603,39 +1603,19 @@ 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)
> >   {
> > -	struct vm_area_struct *vma = NULL, *prev = NULL;
> > -	struct rb_node *rb_node;
> > -	if (!mm)
> > -		goto out;
> > -
> > -	/* Guard against addr being lower than the first VMA */
> > -	vma = mm->mmap;
>
> Why have you removed this guard? Previously we had pprev==NULL and
> returned mm->mmap.
> This seems like a semantic change without any explanation. Could you
> clarify?

IIUC, find_vma_prev() is module unexported and none of known caller use
pprev==NULL. Thus, I thought it can be also simplified. Am I missing 
something?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/