freeing unused memory

freeing unused memory

[Igor Mammedov]

Hi,

I have a question about how upstream patches:

093d7b4639951 xen: release unused free memory
f89e048e76da7 xen: make sure pages are really part of domain before freeing

are supposed to work. Was it intended for freeing unused memory if we boot
kernel with mem=XX option and domU is configured with more memory than XX?


-- 

Thanks,
  Igor

Re: freeing unused memory

[Igor Mammedov]

On 06/29/2011 11:53 AM, Igor Mammedov wrote:
> Hi,
>
> I have a question about how upstream patches:
>
> 093d7b4639951 xen: release unused free memory
> f89e048e76da7 xen: make sure pages are really part of domain before 
> freeing
>
> are supposed to work. Was it intended for freeing unused memory if we 
> boot
> kernel with mem=XX option and domU is configured with more memory than 
> XX?
>
>

In regard to RHBZ#523130, I made additional debugging after applying
above mentioned patches. Result: unsed memory is not freed if we pass
mex=XXXM option to pv guest kernel.

Here is call order:
============
setup_arch ->
  ->  setup_memory_map
    ->  x86_init.resources.memory_setup == xen_memory_setup
            before xen_return_unused_memory call we have e820 map like this
            that contains all memory provide by xen:
              BIOS-provided physical RAM map:
                Xen: 0000000000000000 - 00000000000a0000 (usable)
                Xen: 00000000000a0000 - 0000000000100000 (reserved)
                Xen: 0000000000100000 - 0000000200000000 (usable
*      ->  xen_return_unused_memory

** ->  parse_early_param
=============

*
code fragment:
@xen_return_unused_memory
         for (i = 0; i<  e820->nr_map&&  last_end<  max_addr; i++) {
                 phys_addr_t end = e820->map[i].addr;
                 end = min(max_addr, end);

                 released += xen_release_chunk(last_end, end);
                 last_end = e820->map[i].addr + e820->map[i].size;
         }
in best case is nop since xen_release_chunk will not do anything if end<=
last_end and with our example map pairs (last_end, end) will look like:
   last_end   end
   0x0        0x0
   0xa0000    0xa0000
   0x100000   0x100000
i.e. last_end == end =>  xen_release_chunk is nop
in worst case xen_release_chunk may try to release not existed gap if e820 is
sparse.

last fragment of xen_return_unused_memory
         if (last_end<  max_addr)
                 released += xen_release_chunk(last_end, max_addr);

has sense only if
    last_end(e820 val)<  max_addr (max hv provided addr)
and that could be only if "mem=" parameter was applied to e820 map.
However ** parse_early_param is called after x86_init.resources.memory_setup in
setup_memory_map. So patches:
093d7b4639951 xen: release unused free memory
f89e048e76da7 xen: make sure pages are really part of domain before freeing
are effectively nop code now.

Problem exist not only in rhel6 code but fc15 as well and I guess upstream
is affected to.


-- 
Thanks,
  Igor

Re: freeing unused memory

[Konrad Rzeszutek Wilk]

On Thu, Jun 30, 2011 at 04:20:13PM +0200, Igor Mammedov wrote:
> On 06/29/2011 11:53 AM, Igor Mammedov wrote:
> >Hi,
> >
> >I have a question about how upstream patches:
> >
> >093d7b4639951 xen: release unused free memory
> >f89e048e76da7 xen: make sure pages are really part of domain
> >before freeing
> >
> >are supposed to work. Was it intended for freeing unused memory if
> >we boot
> >kernel with mem=XX option and domU is configured with more memory
> >than XX?
> >
> >
> 
> In regard to RHBZ#523130, I made additional debugging after applying
> above mentioned patches. Result: unsed memory is not freed if we pass
> mex=XXXM option to pv guest kernel.
> 
> Here is call order:
> ============
> setup_arch ->
>  ->  setup_memory_map
>    ->  x86_init.resources.memory_setup == xen_memory_setup
>            before xen_return_unused_memory call we have e820 map like this
>            that contains all memory provide by xen:
>              BIOS-provided physical RAM map:
>                Xen: 0000000000000000 - 00000000000a0000 (usable)
>                Xen: 00000000000a0000 - 0000000000100000 (reserved)
>                Xen: 0000000000100000 - 0000000200000000 (usable
> *      ->  xen_return_unused_memory
> 
> ** ->  parse_early_param
> =============
> 
> *
> code fragment:
> @xen_return_unused_memory
>         for (i = 0; i<  e820->nr_map&&  last_end<  max_addr; i++) {
>                 phys_addr_t end = e820->map[i].addr;
>                 end = min(max_addr, end);
> 
>                 released += xen_release_chunk(last_end, end);
>                 last_end = e820->map[i].addr + e820->map[i].size;
>         }
> in best case is nop since xen_release_chunk will not do anything if end<=
> last_end and with our example map pairs (last_end, end) will look like:
>   last_end   end
>   0x0        0x0
>   0xa0000    0xa0000
>   0x100000   0x100000
> i.e. last_end == end =>  xen_release_chunk is nop
> in worst case xen_release_chunk may try to release not existed gap if e820 is
> sparse.

Those are OK for memory below that region. We actually don't want to free it
as the PV guest stops booting.
> 
> last fragment of xen_return_unused_memory
>         if (last_end<  max_addr)
>                 released += xen_release_chunk(last_end, max_addr);
> 
> has sense only if
>    last_end(e820 val)<  max_addr (max hv provided addr)
> and that could be only if "mem=" parameter was applied to e820 map.

Ah.
> However ** parse_early_param is called after x86_init.resources.memory_setup in
> setup_memory_map. So patches:
> 093d7b4639951 xen: release unused free memory
> f89e048e76da7 xen: make sure pages are really part of domain before freeing
> are effectively nop code now.
> 
> Problem exist not only in rhel6 code but fc15 as well and I guess upstream
> is affected to.

well, they do _some_ freeing. I see 'released XX pages' quite often on my boxes.

But yes, this a problem - we allocate more memory than we need. Do you have any
patches in mind?

> 
> 
> -- 
> Thanks,
>  Igor
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel