Re: [Qemu-devel] [PATCH v3] memory: reduce heap Rss size around 3M

[Peter Maydell]

On 15 March 2017 at 08:12, Paolo Bonzini <address@hidden> wrote:
>
>
> On 15/03/2017 14:39, Yang Zhong wrote:
>> Since cpu-memory and memory have same address space,one malloced
>> memory is enough. This patch will skip memory malloc for memory
>> address space,which will reduce around 3M physical memory in heap.
>>
>> Signed-off-by: Yang Zhong <address@hidden>
>> ---
>>  memory.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/memory.c b/memory.c
>> index 64b0a60..0003b1e 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -2422,7 +2422,7 @@ AddressSpace 
>> *address_space_init_shareable(MemoryRegion *root, const char *name)
>>      AddressSpace *as;
>>
>>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>> -        if (root == as->root && as->malloced) {
>> +        if (root == as->root && as == &address_space_memory) {
>
> Of course you need to keep the as->malloced check, don't you?
>
>>              as->ref_count++;
>>              return as;
>>          }
>>
>
> This is not really beautiful, but compared to v1 and v2 it has the
> advantage that it works (with the as->malloced check reintroduced).
> Peter, you introduced address_space_init_shareable, what do you think?

It looks wrong to me. address_space_memory is not allocated
via address_space_init_shareable(), so it's not correct
to treat it that way (we're implicitly relying on it never
being destroyed). It works by accident, not by design.

If we want to support sharing of address spaces which are
constant and exist for the lifetime of QEMU then we should
probably do it with a new function something like
    address_space_init_static_shareable()
which marks the AS as being (1) shareable and (2) invalid
to ever try to destroy. Then you could use that for both
address_space_memory and address_space_io.

thanks
-- PMM