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

[Zhong, Yang]

Hello Peter,

The cpu-memory and memory have same address space through info mtree tool.

address-space: memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system
    0000000000000000-000000007fffffff (prio 0, i/o): alias ram-below-4g @pc.ram 
0000000000000000-000000007fffffff
    0000000000000000-ffffffffffffffff (prio -1, i/o): pci
      00000000000c0000-00000000000dffff (prio 1, ram): pc.rom
      00000000000e0000-00000000000fffff (prio 1, i/o): alias isa-bios @pc.bios 
0000000000020000-000000000003ffff
      00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios

address-space: cpu-memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system
    0000000000000000-000000007fffffff (prio 0, i/o): alias ram-below-4g @pc.ram 
0000000000000000-000000007fffffff
    0000000000000000-ffffffffffffffff (prio -1, i/o): pci
      00000000000c0000-00000000000dffff (prio 1, ram): pc.rom
      00000000000e0000-00000000000fffff (prio 1, i/o): alias isa-bios @pc.bios 
0000000000020000-000000000003ffff
      00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios

So we can omit the cpu-memory address space allocated in the 
address_space_init_shareable(), which will same around 3M physical memory.

Maybe the below patch is much better, please help review, thanks!

diff --git a/memory.c b/memory.c
index 64b0a60..230f2cb 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->malloced || as == &address_space_memory)) 
{
             as->ref_count++;
             return as;
         } 


Regards,

ZhongYang




diff --git a/memory.c b/memory.c
index 64b0a60..230f2cb 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->malloced || as == &address_space_memory)) 
{
             as->ref_count++;
             return as;
         }




-----Original Message-----
From: Peter Maydell [mailto:address@hidden 
Sent: Wednesday, March 15, 2017 6:42 PM
To: Paolo Bonzini <address@hidden>
Cc: Zhong, Yang <address@hidden>; QEMU Developers <address@hidden>; Xu, Anthony 
<address@hidden>
Subject: Re: [PATCH v3] memory: reduce heap Rss size around 3M

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