Re: [PATCH v3] mm: simplify find_vma_prev

[Michal Hocko]

On Mon 12-12-11 14:26:16, 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?

Scratch that. I have misread the code. find_vma will return mm->mmap if
the given address is bellow all vmas. Sorry about noise.

The only concern left would be the caching. Are you sure this will not
break some workloads which benefit from mmap_cache usage and would
interfere with find_vma_prev callers now? Anyway this could be fixed
trivially.

Thanks
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic
--
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/