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

[Alexey Kardashevskiy]

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 <address@hidden>
> ---
>  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?


> +            unsigned int found = 0;
> +            MemoryRegion *child, *next = NULL;
> +            QTAILQ_FOREACH(child, &mr->subregions, subregions_link) {
> +                if (child->enabled) {
> +                    if (++found > 1) {
> +                        next = NULL;
> +                        break;
> +                    }
> +                    if (!child->addr && int128_ge(mr->size, child->size)) {


Ah, I tried this one but in v4's 18/18 (that hinting thing), did not work
out but for a different reason.


> +                        /* A child is included in its entirety.  If it's the 
> only
> +                         * enabled one, use it in the hope of finding an 
> alias down the
> +                         * way. This will also let us share FlatViews.
> +                         */
> +                        next = child;
> +                    }
> +                }
> +            }
> +            if (next) {
> +                mr = next;
> +                continue;
> +            }
> +        }
> +
> +        break;
>      }
>  
>      return mr;
> 


-- 
Alexey