Bug 523130 - [RHEL-6 Xen]: 64-bit domU does not give memory back to the dom0
Summary: [RHEL-6 Xen]: 64-bit domU does not give memory back to the dom0
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: kernel
Version: 6.0
Hardware: All
OS: Linux
low
medium
Target Milestone: rc
: ---
Assignee: Andrew Jones
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On: 499644
Blocks: 523117
TreeView+ depends on / blocked
 
Reported: 2009-09-14 08:30 UTC by Chris Lalancette
Modified: 2011-08-30 07:26 UTC (History)
17 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of: 499644
Environment:
Last Closed: 2011-08-29 17:28:00 UTC


Attachments (Terms of Use)
return unused memory to dom0 (2.86 KB, patch)
2010-06-18 15:27 UTC, Andrew Jones
no flags Details | Diff

Description Chris Lalancette 2009-09-14 08:30:38 UTC
+++ This bug was initially created as a clone of Bug #499644 +++

Description of problem:
I'm following the test case here: https://fedoraproject.org/wiki/QA:Testcase_Virtualization_XenDomU_Cmdline_params.  My F-11 Xen domU is configured to start with 2047MB of memory.  However, while booting, I specified mem=1G on the kernel command-line, and the guest properly booted with only 1G of memory.  However, it does *not* give the spare memory back to the hypervisor.  That is, since we've set mem=, we can never balloon above that, so we should give that memory back to the HV so it can use it for other domains.  This works as expected in the RHEL-5 domU kernel, and in upstream linux-2.6.18-xen.hg.

--- Additional comment from clalance@redhat.com on 2009-05-08 11:23:54 EDT ---

Miroslav,
     This one is much less important.  It would be nice to get it to work, but it's not critical.

Chris Lalancette

--- Additional comment from fedora-triage-list@redhat.com on 2009-06-09 11:20:31 EDT ---


This bug appears to have been reported against 'rawhide' during the Fedora 11 development cycle.
Changing version to '11'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

--- Additional comment from mrezanin@redhat.com on 2009-08-20 04:01:51 EDT ---

Created an attachment (id=358049)
Patch for 2.6.29 kernel

Problem is caused by missing code for returning memory to HV. Patch adding this code to 2.6.29 kernel sent to Jeremy Fitzhardinge with CC na LKML and Xen-devel.

--- Additional comment from markmc@redhat.com on 2009-08-21 04:40:59 EDT ---

Thread is here:

  http://lists.xensource.com/archives/html/xen-devel/2009-08/threads.html#00573

Sounds like Jeremy wants to use the e820 map to find unused memory rather than just max_pfn

--- Additional comment from jeremy@goop.org on 2009-08-21 18:48:10 EDT ---

Yes, exactly.  Miroslav, would you mind reworking your patch along those lines?

Comment 1 RHEL Product and Program Management 2009-09-14 08:49:55 UTC
This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for
inclusion.

Comment 2 Paolo Bonzini 2010-02-23 17:05:42 UTC
Updates from bug 499644:

Comment 6 Miroslav Rezanina 2009-09-16 04:04:39 EDT
    
    Created an attachment (id=361207) [details]
    Returning memory  not contained in e820 map
    
    General solution - returns all not mapped memory.    
    
Comment 7 Justin M. Forbes 2009-10-07 12:44:41 EDT
    
    Grabbing this one to get the patch tested and into Fedora.    
    
Comment 8 Miroslav Rezanina 2009-10-08 01:02:52 EDT
   
    There should be optimized version of patch in Jeremy's repo. However, this
    version has wrong location of return_unused_memory - it is in
    xen_memory_setup not in xen_post_allocator_init.
    
    branch rebase/core-freemem branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git    
    
Comment 9 Justin M. Forbes 2009-10-14 14:18:14 EDT
    
    Indeed, the version of the patch in jeremy's repo does not actually seem to
    return the unused memory.    
    
Comment 10 Jeremy Fitzhardinge 2009-10-14 14:31:24 EDT
    
    No, there's a problem with it.  Aside from releasing a bit too early, the 
    test to make sure the page being released actually belongs to the domain 
    seems to always return false, so it never wants to release anything.  I'm 
    not sure why; it looks OK to me, but I haven't investigated it in detail 
    yet.

Comment 11 Paolo Bonzini 2010-02-23 12:03:31 EST

    Created an attachment (id=395766) [details]
    patch as committed to jeremy's repo
    
    Attaching patch from Jeremy's repo for posterity.

Comment 3 Andrew Jones 2010-03-19 07:21:21 UTC
An update from Jeremy was put on the Fedora counterpart for this bug.

"I just had another look at this and noticed a rather obvious bug in the patch:
"mfn = pfn_to_mfn(mfn);" rather than "mfn = pfn_to_mfn(*p*fn);".  Fixing this
makes it free memory properly as expected."

Comment 4 Andrew Jones 2010-06-17 12:01:01 UTC
This patch still isn't in Linus' upstream, but it is (and the corrected version) in Jeremy's tree. It's not that big of a deal for rhel 6.0 though, since ballooning doesn't work. Moving to 6.1 and will backport the patch when getting ballooning to work.

Comment 5 Jeremy Fitzhardinge 2010-06-17 12:23:11 UTC
(In reply to comment #4)
> Moving to 6.1 and will backport the patch when
> getting ballooning to work.    

What's wrong with ballooning?

Comment 6 Andrew Jones 2010-06-17 12:32:14 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Moving to 6.1 and will backport the patch when
> > getting ballooning to work.    
> 
> What's wrong with ballooning?    

It hasn't ever worked for the kernels I've worked with since Fedora 11, and I just never got a chance to debug it. I haven't tried your tree in quite a while though, so I can certainly give it a go and see if it works. Or, I would happily take pointers to specific patch(es) that you know enabled it :-)

Thanks,
Andrew

Comment 7 Andrew Jones 2010-06-17 13:24:27 UTC
Just for reference the ballooning bug is bug 523122. Jeremy has inspired me to take a look upstream now though, so I'm currently testing. I will update the ballooning bug with whatever I find out.

Comment 8 Andrew Jones 2010-06-17 15:50:08 UTC
All upstream patches needed are

7243521 xen: release unused free memory
8b539bf xen: make sure pages are really part of domain before freeing
552e061 xen: fix scanning for freeable pages
fe4461d xen: little formatting fix
485c44e xen: use phys_addr_t for last_end to fix 32-bit crash
28c3e64 xen/core: don't bother trying to free pages beyond the ones Xen gave us

I'll post a patch soon that includes all those.

Comment 11 Andrew Jones 2010-06-18 15:27:17 UTC
Created attachment 425155 [details]
return unused memory to dom0

Comment 12 Andrew Jones 2010-06-18 15:31:42 UTC
I've attached a single patch that gets all the patches listed in comment 8. I've also tested it a bit and confirmed it works as well as upstream. The problem is neither seem to do anything for the test case of this bug.  With both upstream and rhel6 with this patch I get the following message on boot

released 0 pages of unused memory

and sure enough, we still can't balloon into what we would expect as unused memory. So the conditions to free memory don't seem to be getting met by this test case, or something doesn't work. I'll have to test more before posting it.

Comment 14 Andrew Jones 2010-06-30 06:47:17 UTC
The poor bug already bounced 6.0 -> 6.1 -> 6.0, but seeing that even upstream refuses to release unused pages for our test case indicates to me that we need to look at this more closely before posting. The looking should be done during 6.1. Bouncing it again, 6.0 -> 6.1.

Comment 16 RHEL Product and Program Management 2010-11-18 16:19:57 UTC
This request was evaluated by Red Hat Product Management for inclusion
in a Red Hat Enterprise Linux maintenance release. Product Management has 
requested further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed 
products. This request is not yet committed for inclusion in an Update release.

Comment 17 Paolo Bonzini 2010-12-02 01:46:27 UTC
See bug 613606 comment #23 item 2.

Comment 19 Igor Mammedov 2011-06-28 14:35:11 UTC
Comment 8 patches in upstream kernel committed as:

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

Comment 20 Igor Mammedov 2011-06-28 17:34:23 UTC
After additional debugging these patches (comment 19) doesn't affect xen pv guest
in any way. 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 in upstream as well.

Comment 21 Igor Mammedov 2011-07-04 15:04:26 UTC
Asked upstream, no answer so far.
However found in bug 499644 comment 4 a link
http://lists.xensource.com/archives/html/xen-devel/2009-08/threads.html#00573

Looks like upstream idea is unmap holes in e820 and they don't care about what
mem=xxx parameter in guest kernel says.

So propose to close bug as WONTFIX?


Note You need to log in before you can comment on or make changes to this bug.