From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
To: Wei Yang <richardw.yang@linux.intel.com>
Cc: Li Xinhai <lixinhai.lxh@gmail.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
akpm <akpm@linux-foundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Rik van Riel <riel@redhat.com>,
"kirill.shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v2 1/2] mm/rmap: fix and simplify reusing mergeable anon_vma as parent when fork
Date: Sun, 12 Jan 2020 12:55:45 +0300 [thread overview]
Message-ID: <a6a7bb3b-434e-277c-694f-d5a18e629d2c@yandex-team.ru> (raw)
In-Reply-To: <20200111223820.GA15506@richard>
On 12/01/2020 01.38, Wei Yang wrote:
> On Fri, Jan 10, 2020 at 11:11:23AM +0300, Konstantin Khlebnikov wrote:
> [...]
>>>>>>
>>>>>> series of vma in parent with shared AV:
>>>>>>
>>>>>> SRC1 - AV0
>>>>>> SRC2 - AV0
>>>>>> SRC3 - AV0
>>>>>> ...
>>>>>> SRCn - AV0
>>>>>>
>>>>>> in child after fork
>>>>>>
>>>>>> DST1 - AV_OLD_1 (some old vma, picked by anon_vma_clone) plus DST1 is attached to same AVs as SRC1
>>>>>> DST2 - AV_OLD_2 (other old vma) plus DST1 is attached to same AVs as SRC2
>>>>>> DST2 - AV1 prev AV parent does not match AV0, no old vma found for reusing -> allocate new one (child of AV0)
>>>>>> DST3 - AV1 - DST2->AV->parent == SRC3->AV (AV0) -> share AV with prev
>>>>>> DST4 - AV1 - same thing
>>>>>> ...
>>>>>> DSTn - AV1
>>>>>>
>
> To focus on the point, I rearranged the order a little. Suppose your following
> comments is explaining the above behavior.
>
> I've illustrated how two heuristics (reusing-old and sharing-prev) _could_ work together.
> But they both are optional.
>
> At cloning first vma SRC1 -> DST1 there is no prev to share anon vma,
> thus works common code which _could_ reuse old vma because it have to.
>
> If there is no old anon-vma which have to be reused then DST1 will allocate
> new anon-vma (AV1) and it will be used by DST2 and so on like on your picture.
>
> I agree with your 3rd paragraph, but confused with 2nd.
>
> At cloning first vma SRC1 -> DST1, there is no prev so anon_vma_clone() would
> pick up a reusable anon_vma. Here you named it AV_OLD_1. This looks good to
> me. But I am not sure why you would picked up AV_OLD_2 for DST2? In parent,
> SRC1 and SRC2 has the same anon_vma, AV0. So in child, DST1 and DST2 could
> also share the same anon_vma, AV_OLD_1.
>
> Sorry for my poor understanding, would you mind giving me more hint on this
> change?
For DST2 heuristic "share-with-prev" will not work because if prev (DST1)
uses old AV (AV_OLD_1) and AV_OLD_1->parent isn't SRC2->AV (AV0).
So DST2 could only pick another old AV or allocate new.
My patch uses condition dst->prev->anon_vma->parent == src->anon_vma rather
than obvious src->prev->anon_vma == src->anon_vma because in this way it
eliminates all unwanted corner cases and explicitly verifies that we going to
share related anon-vma.
Heuristic "reuse-old" uses fact that VMA links and AV parent chain are tracked
independently: when VMA reuses old AV it still links to all related AV even
if VMA->AV points into some old AV in the middle of inheritance chain.
>
>>>>>
>>>>> Yes, your code works for DST3..DSTn. They will pick up AV1 since
>>>>> (DST2->AV->parent == SRC3->AV).
>>>>>
>>>>> My question is why DST1 and DST2 has different AV? The purpose of my patch
>>>>> tries to make child has the same topology and parent. So the ideal look of
>>>>> child is:
>>>>>
>>>>> DST1 - AV1
>>>>> DST2 - AV1
>>>>> DST2 - AV1
>>>>> DST3 - AV1
>>>>> DST4 - AV1
>>>>>
>>>>> Would you mind putting more words on DST1 and DST2? I didn't fully understand
>>>>> the logic here.
>>>>>
>>>>> Thanks
>>>>>
>>>>
>>>> I think that the first version is doing the work as you expected, but been
>>>> revised in second version, to limits the number of users of reused old
>>>> anon(which is picked in anon_vma_clone() and keep the tree structure.
>>>>
>>>
>>> Any reason to reduce the reuse? Maybe I lost some point.
>>
>>>
>>>>> --
>>>>> Wei Yang
>>>>> Help you, Help me
>>>
>
next prev parent reply other threads:[~2020-01-12 9:55 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-07 10:19 [PATCH v2 1/2] mm/rmap: fix and simplify reusing mergeable anon_vma as parent when fork Konstantin Khlebnikov
2020-01-07 10:19 ` [PATCH v2 2/2] kernel/fork: set VMA's mm/prev/next right after vm_area_dup in dup_mmap Konstantin Khlebnikov
2020-01-08 2:32 ` [PATCH v2 1/2] mm/rmap: fix and simplify reusing mergeable anon_vma as parent when fork Wei Yang
2020-01-08 10:40 ` Konstantin Khlebnikov
2020-01-09 2:52 ` Wei Yang
2020-01-09 8:54 ` Konstantin Khlebnikov
2020-01-10 2:30 ` Wei Yang
[not found] ` <20200110112357351531132@gmail.com>
2020-01-10 5:34 ` Wei Yang
2020-01-10 8:11 ` Konstantin Khlebnikov
2020-01-11 22:38 ` Wei Yang
2020-01-12 9:55 ` Konstantin Khlebnikov [this message]
2020-01-13 0:33 ` Wei Yang
2020-01-13 11:07 ` Konstantin Khlebnikov
2020-01-14 2:09 ` Wei Yang
[not found] ` <2020011422424965556826@gmail.com>
2020-01-15 1:20 ` Wei Yang
2020-01-18 8:04 ` Konstantin Khlebnikov
2020-01-18 14:00 ` 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=a6a7bb3b-434e-277c-694f-d5a18e629d2c@yandex-team.ru \
--to=khlebnikov@yandex-team.ru \
--cc=akpm@linux-foundation.org \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lixinhai.lxh@gmail.com \
--cc=richardw.yang@linux.intel.com \
--cc=riel@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).