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 09:53:52AM -0400, 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.
> 
> 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.
> 
> > 
> > Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> > ---
> > 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.
> 
> > ---
> >  arch/x86/xen/setup.c |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> > index 3421c9e..584e7dc 100644
> > --- 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()) {

This actually is not neccessary? You could remove this and it would work
fine under DomU cases too I think. The only issue would be when running
this under older hypervisors as dom0 and getting ~0 as the max_pages - but
the 'min' clamping should solve that?

> > -		/* 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
>         max_pages = min(ret, max_pages); ?
> 
> >  	}
> >  
> >  	return min(max_pages, MAX_DOMAIN_PAGES);
> > -- 
> > 1.7.4.1