Re: [PATCH 3/3] xen: use maximum reservation to limit dom0 memory

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Aug 16, 2011 at 03:41:03PM +0100, David Vrabel wrote:
> On 16/08/11 14:53, Konrad Rzeszutek Wilk wrote:
> > On Tue, Aug 16, 2011 at 11:00:38AM +0100, David Vrabel wrote:
> >> Use the maximum reservation hypercall to set limit the amount of
> >> usable dom0 memory.  This reduces the size of pages tables etc. if
> >> dom0 is to use less memory than the maximum available.
> > 
> > Ok, so it sounds like this patch by itself can fix the "more page tables
> > than we need" issue.
> 
> This patch with the Xen patch does, yes.
> 
> > If so, I would prefer that you stick the tiny piece of code that
> > calls the xen_get_max_pages() from the setup in this patch. This way
> > we can backport this particular patch to stable tree without including
> > the other patchsets you have posted. And it is a nicely contained
> > one-patch-fixes-the-problem.
> 
> Does this problem need to be fixed in stable?  It has a simple
> workaround (the 'mem' kernel command line option) and requires an
> updated Xen.

I would like it be ported to 3.0.3 so folks who are using it
can stop using the work-around.

> 
> I do think that patches #1 and #2 are useful because they allow 32-bit
> guests to have more low memory, rather than making all balloon memory
> high memory.

Sure, but they are a different subset of the problem.

> 
> I could rearrange the order. Make #3 first so it can also be applied to
> 3.0.n and 3.1 and then #1 and #2 could be queued for 3.2.

OK. I was thinking to cherry-pick this specific patch for 3.1 anyhow.

> 
> >> Note this requires a patched Xen that sets max_pages when creating dom0.
> > 
> > Please mention in the description the c/s and the name of the patch.
> 
> Ok.
> 
> >> --- a/arch/x86/xen/setup.c
> >> +++ b/arch/x86/xen/setup.c
> >> @@ -197,7 +197,12 @@ static unsigned long __init xen_get_max_pages(void)
> >>  	unsigned long max_pages = MAX_DOMAIN_PAGES; /* Limited by memory map. */
> >>  
> >>  	if (xen_initial_domain()) {
> >> -		/* FIXME: ask hypervisor for max pages. */
> >> +		domid_t domid = DOMID_SELF;
> >> +		int ret;
> >> +
> >> +		ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
> >> +		if (ret > 0)
> >> +			max_pages = ret;
> > Don't you want to clamp it? Say MAX_DOMAIN_PAGES is set to 1GB, and you
> > set it to 2GB here - that will blow the P2M out. Perhaps
> 
> It is...
> 
> >         max_pages = min(ret, max_pages); ?
> > 
> >>  	}
> >>  
> >>  	return min(max_pages, MAX_DOMAIN_PAGES);
> 
> ... here.

Duh!

> 
> David