Re: [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView

[Alexey Kardashevskiy]

On 21/09/17 03:13, Paolo Bonzini wrote:
> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>> This shares an cached empty FlatView among address spaces. The empty
>> FV is used every time when a root MR renders into a FV without memory
>> sections which happens when MR or its children are not enabled or
>> zero-sized. The empty_view is not NULL to keep the rest of memory
>> API intact; it also has a dispatch tree for the same reason.
>>
>> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this halves
>> the amount of FlatView's in use (557 -> 260) and dispatch tables
>> (~80 -> ~37), however the total memory footprint is pretty much
>> the same as RCU is holding all these temporary FVs which are created
>> (and then released) to make sure that they are empty and can be replaced
>> with @empty_view.
>>
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>  memory.c | 14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/memory.c b/memory.c
>> index 4add0fd030..92b1304a20 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -48,6 +48,7 @@ static QTAILQ_HEAD(, AddressSpace) address_spaces
>>  = QTAILQ_HEAD_INITIALIZER(address_spaces);
>>  
>>  static GHashTable *flat_views;
>> +static FlatView *empty_view;
>>  
>>  typedef struct AddrRange AddrRange;
>>  
>> @@ -755,6 +756,19 @@ static FlatView *generate_memory_topology(MemoryRegion 
>> *mr)
>>  }
>>  flatview_simplify(view);
>>  
>> +if (!view->nr) {
>> +flatview_unref(view);
> 
> This can be changed to flatview_destroy directly to avoid overloading
> RCU with all these temporary FlatViews.


Yeah, this or just allocate every new FlatView on the stack first, I
thought about this a second after I posted this.


> 
> Paolo
> 
>> +if (!empty_view) {
>> +empty_view = flatview_new(NULL);
>> +}
>> +view = empty_view;
>> +flatview_ref(view);
>> +}
>> +
>> +if (view->dispatch) {
>> +return view;
>> +}
>> +
>>  view->dispatch = address_space_dispatch_new(view);
>>  for (i = 0; i < view->nr; i++) {
>>  MemoryRegionSection mrs =
>>
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView

[Paolo Bonzini]

On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> This shares an cached empty FlatView among address spaces. The empty
> FV is used every time when a root MR renders into a FV without memory
> sections which happens when MR or its children are not enabled or
> zero-sized. The empty_view is not NULL to keep the rest of memory
> API intact; it also has a dispatch tree for the same reason.
> 
> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this halves
> the amount of FlatView's in use (557 -> 260) and dispatch tables
> (~80 -> ~37), however the total memory footprint is pretty much
> the same as RCU is holding all these temporary FVs which are created
> (and then released) to make sure that they are empty and can be replaced
> with @empty_view.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  memory.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/memory.c b/memory.c
> index 4add0fd030..92b1304a20 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -48,6 +48,7 @@ static QTAILQ_HEAD(, AddressSpace) address_spaces
>  = QTAILQ_HEAD_INITIALIZER(address_spaces);
>  
>  static GHashTable *flat_views;
> +static FlatView *empty_view;
>  
>  typedef struct AddrRange AddrRange;
>  
> @@ -755,6 +756,19 @@ static FlatView *generate_memory_topology(MemoryRegion 
> *mr)
>  }
>  flatview_simplify(view);
>  
> +if (!view->nr) {
> +flatview_unref(view);

This can be changed to flatview_destroy directly to avoid overloading
RCU with all these temporary FlatViews.

Paolo

> +if (!empty_view) {
> +empty_view = flatview_new(NULL);
> +}
> +view = empty_view;
> +flatview_ref(view);
> +}
> +
> +if (view->dispatch) {
> +return view;
> +}
> +
>  view->dispatch = address_space_dispatch_new(view);
>  for (i = 0; i < view->nr; i++) {
>  MemoryRegionSection mrs =
>