Message-ID: <1367568932.28742.19.camel@zakaz.uk.xensource.com>
Date:	Fri, 3 May 2013 09:15:32 +0100
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	Stefano Stabellini <Stefano.Stabellini@...citrix.com>,
	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	"Keir (Xen.org)" <keir@....org>,
	Dave Scott <Dave.Scott@...citrix.com>,
	"james-xen@...gwall.me.uk" <james-xen@...gwall.me.uk>,
	Daniel Kiper <daniel.kiper@...cle.com>,
	Ian Jackson <Ian.Jackson@...citrix.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Jonathan Ludlam" <Jonathan.Ludlam@...citrix.com>,
	"darren.s.shepherd@...il.com" <darren.s.shepherd@...il.com>,
	David Vrabel <david.vrabel@...rix.com>,
	"carsten@...iers.de" <carsten@...iers.de>
Subject: Re: [Xen-devel] [PATCH v2 2/2] xen/balloon: Enforce various limits
 on target

On Thu, 2013-05-02 at 19:04 +0100, Konrad Rzeszutek Wilk wrote:
> On Thu, May 02, 2013 at 12:34:32PM +0100, Stefano Stabellini wrote:
> > On Tue, 30 Apr 2013, Daniel Kiper wrote:
> > > > > > > +/*
> > > > > > > + * Extra internal memory reserved by libxl.
> > > > > > > + * Check tools/libxl/libxl_memory.txt file in Xen source for more details.
> > > > > > > + */
> > > > > > > +#define LIBXL_MAXMEM_CONSTANT_PAGES	(1024 * 1024 / PAGE_SIZE)
> > > > > >
> > > > > > I think we need to find a way to achieve your aims which doesn't require
> > > > > > leaking internal implementation details of libxl into the guest kernels.
> > > > > > What happens if libxl decides to double this?
> > > > >
> > > > > I agree that this is not elegant solution. However, if we would like to
> > > > > be in line with docs/misc/libxl_memory.txt (this is correct path) this
> > > > > is a must.
> > > >
> > > > I'm not sure about this, that file describes the toolstacks view of the
> > > > memory in a system. That need not necessarily correspond with the
> > > > guest's ideas (although you would hope it would be a superset).
> > > >
> > > > Surely it is logically wrong to bake toolstack specific knowledge in the
> > > > guest? If someone can describe a meaningful semantic for this number in
> > > > a toolstack independent way then perhaps it would be appropriate to do
> > > > something with it. I've no idea, having looked at both the document and
> > > > the code, what this value actually is.
> > > 
> > > This was added by commit 9905ac2b90a3e7cecd9e7dfe21c252362e7080b2
> > > (libxenlight: implement libxl_set_memory_target). It was written
> > > by Keir and signed off by Stefano (both are CCed here). Guys,
> > > why did you added LIBXL_MAXMEM_CONSTANT? What does it mean?
> > 
> > libxl inherits the memory model from xapi.
> > LIBXL_MAXMEM_CONSTANT corresponds to "extra internal" in xapi, an amount
> > of memory that is not allocated to the domain but it is left as a slack
> > on top of the actual memory target to determine the Xen maxmem.
> > I believe it comes from empirical measurements and stress testing on the
> > platform.
> 
> What is this 'slack' used for?

Did you read Stefano's next sentence?

> > The xapi guys, CC'ed, might have more insights on what exactly is.

I think that unless someone can remember what this issue was we should
just chalk it up to a historical artefact of something xapi (or maybe
some historical guest) was doing which we have no reason to believe
needs to be carried over to libxl.

IOW I'm suggesting we set LIBXL_MAXMEM_CONSTANT to 0 early in the 4.4
cycle and see how it goes. If someone can show either empirical evidence
or (better) logically argue that a fudge is required then we can always
put it back (or it may turn out to be the caller's issue, in which case
they can deal with it, hopefully xapi-on-libxl won't apply this fudge
twice...).

Alternatively I'm also strongly considering having debug builds of the
toolstack randomise the amount of slack, that ought to shake out any
lingering issues...

> > I dislike having to pull this "hack" into Linux, but if it is actually
> > important to leave LIBXL_MAXMEM_CONSTANT unused, then it is worth doing.
> > I would add a big comment on top saying:
> > 
> > "libxl seems to think that we need to leave LIBXL_MAXMEM_CONSTANT
> > kilobytes unused, let's be gentle and do that."

It seems to me that this change in Linux is really just papering over
the underlying issue. Or at the very least no one has adequately
explained what that real issue is and why this change is relevant to it
and/or an appropriate fix for it.

The guest knows what target the toolstack has set for it is (its in the
target xenstore node), I don't see any reason for the guest to be
further second guessing that value by examining maxmem (adjusted by a
fudge factor or otherwise). If the guest is seeing failures to increase
its reservation when trying to meet that target then either the
toolstack was buggy in asking it to hit a target greater than its maxmem
or it is hitting one of the other reason for increase reservation
failures. Since it needs to deal with the latter anyway I don't see any
reason to special case maxmem as a cause for a failure.

Ian.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists