From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
To: Wei Yang <richardw.yang@linux.intel.com>,
akpm@linux-foundation.org, kirill.shutemov@linux.intel.com,
jglisse@redhat.com, mike.kravetz@oracle.com, riel@surriel.com,
cai@lca.pw, shakeelb@google.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [Patch v4 2/2] mm/rmap.c: reuse mergeable anon_vma as parent when fork
Date: Fri, 11 Oct 2019 11:10:27 +0300 [thread overview]
Message-ID: <8b38be36-ab2b-2bdd-13a1-ff49aeef1f32@yandex-team.ru> (raw)
In-Reply-To: <20191011072256.16275-2-richardw.yang@linux.intel.com>
On 11/10/2019 10.22, Wei Yang wrote:
> In function __anon_vma_prepare(), we will try to find anon_vma if it is
> possible to reuse it. While on fork, the logic is different.
>
> Since commit 5beb49305251 ("mm: change anon_vma linking to fix
> multi-process server scalability issue"), function anon_vma_clone()
> tries to allocate new anon_vma for child process. But the logic here
> will allocate a new anon_vma for each vma, even in parent this vma
> is mergeable and share the same anon_vma with its sibling. This may do
> better for scalability issue, while it is not necessary to do so
> especially after interval tree is used.
>
> Commit 7a3ef208e662 ("mm: prevent endless growth of anon_vma hierarchy")
> tries to reuse some anon_vma by counting child anon_vma and attached
> vmas. While for those mergeable anon_vmas, we can just reuse it and not
> necessary to go through the logic.
>
> After this change, kernel build test reduces 20% anon_vma allocation.
>
> Do the same kernel build test, it shows run time in sys reduced 11.6%.
>
> Origin:
>
> real 2m50.467s
> user 17m52.002s
> sys 1m51.953s
>
> real 2m48.662s
> user 17m55.464s
> sys 1m50.553s
>
> real 2m51.143s
> user 17m59.687s
> sys 1m53.600s
>
> Patched:
>
> real 2m39.933s
> user 17m1.835s
> sys 1m38.802s
>
> real 2m39.321s
> user 17m1.634s
> sys 1m39.206s
>
> real 2m39.575s
> user 17m1.420s
> sys 1m38.845s
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Acked-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
> mm/rmap.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index c34414567474..2c13e2bfd393 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -268,6 +268,19 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> {
> struct anon_vma_chain *avc, *pavc;
> struct anon_vma *root = NULL;
> + struct vm_area_struct *prev = dst->vm_prev, *pprev = src->vm_prev;
> +
> + /*
> + * If parent share anon_vma with its vm_prev, keep this sharing in in
> + * child.
> + *
> + * 1. Parent has vm_prev, which implies we have vm_prev.
> + * 2. Parent and its vm_prev have the same anon_vma.
> + */
> + if (!dst->anon_vma && src->anon_vma &&
> + pprev && pprev->anon_vma == src->anon_vma)
> + dst->anon_vma = prev->anon_vma;
> +
I believe that in present code "prev" cannot be NULL if !dst->anon_vma && src->anon_vma is true.
It would be safer to check this explicitly.
>
> list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
> struct anon_vma *anon_vma;
>
next prev parent reply other threads:[~2019-10-11 8:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-11 7:22 [Patch v4 1/2] mm/rmap.c: don't reuse anon_vma if we just want a copy Wei Yang
2019-10-11 7:22 ` [Patch v4 2/2] mm/rmap.c: reuse mergeable anon_vma as parent when fork Wei Yang
2019-10-11 8:10 ` Konstantin Khlebnikov [this message]
2019-10-11 8:04 ` [Patch v4 1/2] mm/rmap.c: don't reuse anon_vma if we just want a copy Konstantin Khlebnikov
2019-10-11 8:39 ` Wei Yang
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=8b38be36-ab2b-2bdd-13a1-ff49aeef1f32@yandex-team.ru \
--to=khlebnikov@yandex-team.ru \
--cc=akpm@linux-foundation.org \
--cc=cai@lca.pw \
--cc=jglisse@redhat.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mike.kravetz@oracle.com \
--cc=richardw.yang@linux.intel.com \
--cc=riel@surriel.com \
--cc=shakeelb@google.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.