linux-kernel - Re: [PATCH v3] mm: simplify find_vma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 12 Dec 2011 20:15:32 +0100
From:	Michal Hocko <mhocko@...e.cz>
To:	kosaki.motohiro@...il.com
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Shaohua Li <shaohua.li@...el.com>
Subject: Re: [PATCH v3] mm: simplify find_vma_prev

On Mon 12-12-11 14:26:16, Michal Hocko wrote:
> On Fri 09-12-11 17:48:40, kosaki.motohiro@...il.com wrote:
> > From: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> > 
> > 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@...fujitsu.com>
> > ---
> >  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/