From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Oleg Nesterov <oleg@redhat.com>
Cc: yalin wang <yalin.wang2010@gmail.com>,
akpm@linux-foundation.org, kirill.shutemov@linux.intel.com,
gang.chen.5i5j@gmail.com, mhocko@suse.com,
kwapulinski.piotr@gmail.com, aarcange@redhat.com,
dcashman@google.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [RFC] mm: change find_vma() function
Date: Mon, 14 Dec 2015 23:11:32 +0200 [thread overview]
Message-ID: <20151214211132.GA7390@node.shutemov.name> (raw)
In-Reply-To: <20151214175509.GA25681@redhat.com>
On Mon, Dec 14, 2015 at 06:55:09PM +0100, Oleg Nesterov wrote:
> On 12/14, Kirill A. Shutemov wrote:
> >
> > On Mon, Dec 14, 2015 at 07:02:25PM +0800, yalin wang wrote:
> > > change find_vma() to break ealier when found the adderss
> > > is not in any vma, don't need loop to search all vma.
> > >
> > > Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
> > > ---
> > > mm/mmap.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/mm/mmap.c b/mm/mmap.c
> > > index b513f20..8294c9b 100644
> > > --- a/mm/mmap.c
> > > +++ b/mm/mmap.c
> > > @@ -2064,6 +2064,9 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
> > > vma = tmp;
> > > if (tmp->vm_start <= addr)
> > > break;
> > > + if (!tmp->vm_prev || tmp->vm_prev->vm_end <= addr)
> > > + break;
> > > +
> >
> > This 'break' would return 'tmp' as found vma.
>
> But this would be right?
Hm. Right. Sorry for my tone.
I think the right condition is 'tmp->vm_prev->vm_end < addr', not '<=' as
vm_end is the first byte after the vma. But it's equivalent in practice
here.
Anyway, I don't think it's possible to gain anything measurable from this
optimization.
>
> Not that I think this optimization makes sense, I simply do not know,
> but to me this change looks technically correct at first glance...
>
> But the changelog is wrong or I missed something. This change can stop
> the main loop earlier; if "tmp" is the first vma,
For the first vma, we don't get anything comparing to what we have now:
check for !rb_node on the next iteration would have the same trade off and
effect as the proposed check.
> or if the previous one is below the address.
Yes, but would it compensate additional check on each 'tmp->vm_end > addr'
iteration to the point? That's not obvious.
> Or perhaps I just misread that "not in any vma" note in the changelog.
>
> No?
>
> Oleg.
>
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Oleg Nesterov <oleg@redhat.com>
Cc: yalin wang <yalin.wang2010@gmail.com>,
akpm@linux-foundation.org, kirill.shutemov@linux.intel.com,
gang.chen.5i5j@gmail.com, mhocko@suse.com,
kwapulinski.piotr@gmail.com, aarcange@redhat.com,
dcashman@google.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [RFC] mm: change find_vma() function
Date: Mon, 14 Dec 2015 23:11:32 +0200 [thread overview]
Message-ID: <20151214211132.GA7390@node.shutemov.name> (raw)
In-Reply-To: <20151214175509.GA25681@redhat.com>
On Mon, Dec 14, 2015 at 06:55:09PM +0100, Oleg Nesterov wrote:
> On 12/14, Kirill A. Shutemov wrote:
> >
> > On Mon, Dec 14, 2015 at 07:02:25PM +0800, yalin wang wrote:
> > > change find_vma() to break ealier when found the adderss
> > > is not in any vma, don't need loop to search all vma.
> > >
> > > Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
> > > ---
> > > mm/mmap.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/mm/mmap.c b/mm/mmap.c
> > > index b513f20..8294c9b 100644
> > > --- a/mm/mmap.c
> > > +++ b/mm/mmap.c
> > > @@ -2064,6 +2064,9 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
> > > vma = tmp;
> > > if (tmp->vm_start <= addr)
> > > break;
> > > + if (!tmp->vm_prev || tmp->vm_prev->vm_end <= addr)
> > > + break;
> > > +
> >
> > This 'break' would return 'tmp' as found vma.
>
> But this would be right?
Hm. Right. Sorry for my tone.
I think the right condition is 'tmp->vm_prev->vm_end < addr', not '<=' as
vm_end is the first byte after the vma. But it's equivalent in practice
here.
Anyway, I don't think it's possible to gain anything measurable from this
optimization.
>
> Not that I think this optimization makes sense, I simply do not know,
> but to me this change looks technically correct at first glance...
>
> But the changelog is wrong or I missed something. This change can stop
> the main loop earlier; if "tmp" is the first vma,
For the first vma, we don't get anything comparing to what we have now:
check for !rb_node on the next iteration would have the same trade off and
effect as the proposed check.
> or if the previous one is below the address.
Yes, but would it compensate additional check on each 'tmp->vm_end > addr'
iteration to the point? That's not obvious.
> Or perhaps I just misread that "not in any vma" note in the changelog.
>
> No?
>
> Oleg.
>
--
Kirill A. Shutemov
next prev parent reply other threads:[~2015-12-14 21:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-14 11:02 [RFC] mm: change find_vma() function yalin wang
2015-12-14 11:02 ` yalin wang
2015-12-14 12:11 ` Kirill A. Shutemov
2015-12-14 12:11 ` Kirill A. Shutemov
2015-12-14 17:55 ` Oleg Nesterov
2015-12-14 17:55 ` Oleg Nesterov
2015-12-14 21:11 ` Kirill A. Shutemov [this message]
2015-12-14 21:11 ` Kirill A. Shutemov
2015-12-15 6:41 ` yalin wang
2015-12-15 6:41 ` yalin wang
2015-12-15 11:47 ` Andrea Arcangeli
2015-12-15 11:47 ` Andrea Arcangeli
2015-12-15 11:53 ` Kirill A. Shutemov
2015-12-15 11:53 ` Kirill A. Shutemov
2015-12-22 5:31 ` yalin wang
2015-12-22 5:31 ` yalin wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151214211132.GA7390@node.shutemov.name \
--to=kirill@shutemov.name \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dcashman@google.com \
--cc=gang.chen.5i5j@gmail.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kwapulinski.piotr@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=oleg@redhat.com \
--cc=yalin.wang2010@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.