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

[Peter Maydell]

On 15 March 2017 at 12:49, Zhong, Yang <address@hidden> wrote:
> So we can omit the cpu-memory address space allocated in the 
> address_space_init_shareable(), which will same around 3M physical memory.

I see what you want to do...

> 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;
>          }

...but all the stuff I said below applies to this patch.

> 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.

We should not be special casing address_space_memory here:
if you want a mechanism for allowing static "exists for
lifetime of QEMU" address spaces to be included in the
set which can be shared, then you need to create one.

thanks
-- PMM