Re: [Qemu-devel] [PATCH 2/3] memory: seek FlatView sharing candidates among children subregions

[Paolo Bonzini <pbonzini@redhat.com>]

On 22/09/2017 06:45, Alexey Kardashevskiy wrote:
> On 21/09/17 23:57, Paolo Bonzini wrote:
>> On 21/09/2017 15:39, Alexey Kardashevskiy wrote:
>>> On 21/09/17 22:07, Paolo Bonzini wrote:
>>>> A container can be used instead of an alias to allow switching between
>>>> multiple subregions.  In this case we cannot directly share the
>>>> subregions (since they only belong to a single parent), but if the
>>>> subregions are aliases we can in turn walk those.
>>>>
>>>> While this does not reduce much the number of FlatViews that are created,
>>>> it makes it possible to share the PCI bus master FlatViews and their
>>>> AddressSpaceDispatch structures.  For 112 virtio-net-pci devices, boot time
>>>> is reduced from 25 to 10 seconds and memory consumption from 1.4 to 1 G.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>  memory.c | 41 +++++++++++++++++++++++++++++++++++------
>>>>  1 file changed, 35 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/memory.c b/memory.c
>>>> index 4952cc5d84..3207ae55e2 100644
>>>> --- a/memory.c
>>>> +++ b/memory.c
>>>> @@ -733,12 +733,41 @@ static void render_memory_region(FlatView *view,
>>>>  
>>>>  static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
>>>>  {
>>>> -    while (mr->alias && !mr->alias_offset &&
>>>> -           int128_ge(mr->size, mr->alias->size)) {
>>>> -        /* The alias is included in its entirety.  Use it as
>>>> -         * the "real" root, so that we can share more FlatViews.
>>>> -         */
>>>> -        mr = mr->alias;
>>>> +    while (mr->enabled) {
>>>> +        if (mr->alias && !mr->alias_offset &&
>>>> +            int128_ge(mr->size, mr->alias->size)) {
>>>> +            /* The alias is included in its entirety.  Use it as
>>>> +             * the "real" root, so that we can share more FlatViews.
>>>> +             */
>>>> +            mr = mr->alias;
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        if (!mr->terminates) {
>>>
>>> Can an MR have terminates==true and children by the same time? If so, what for?
>>
>> Yes, children override the parent. > But more important, !mr->terminates
>> means that patch 3 doesn't think that MR produces an empty FlatView.
> 
> 
> I see, after some debugging only MRs with @terminates==true appear in the
> dispatch tree which makes sense. But I am still struggling to understand
> what @terminates really means here in plain english.

As you found out, perhaps "dispatched" would be a better name than
"terminates".  A MR can be "alias", "terminates" or "!terminates".
Aliases are resolved early.  If a range is allocated to a "terminates"
region A (and not to any other "terminates" region in A's subtree), it
dispatches to A.

If a range is allocated to a "!terminates" region B (and to any other
"terminates" region in B's subtree, QEMU doesn't add it to the FlatView
and it will be dispatched to another memory region: B's parent, another
region in B's parent's subtree, B's grandparent, another region in B's
grandparent's subtree, and so on---if nobody accepts it, it goes to
unassigned_mem_ops.

Paolo