Re: [Xen-devel] [PATCH v2 2/2] xen/balloon: Enforce various limits on target

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2013-05-03 at 14:00 +0100, Daniel Kiper wrote:
> On Fri, May 03, 2013 at 09:15:32AM +0100, Ian Campbell wrote:
> > 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:
> 
> [...]
> 
> > > > 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...
> 
> Do you suggest to postopone this work until 4.4 merge window?

4.4 is a Xen version, this is a Linux patch so I'm not sure what you
mean.

> If yes, then I think that at least "xen/balloon: Enforce various limits on target"
> patch (without this crazy libxl hack) should be applied.

You mean this patch with only the MAX_DOMAIN_PAGES clamping? That seems
plausible.

> > > > 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.
> 
> Do not forget that guest may change target itself.

Yes it can, and that can fail either due to maxmem or due to ENOMEM, and
the kernel needs prepared to deal with that when it happens.

> Additionally, we would like to introduce xm compatibility
> mode which is a bit different then xl normal behavior.

When then you really don't want to be baking specifics of the current
model into the kernel, do you.

> I do not mention that it is always worth check the limits.
> It will save us a lot of trouble later.

On the contrary, it seems to me that baking magic numbers into the
kernel based on current toolstack behaviour is asking for trouble later.

Ian.