RE: [PATCH v2] xen: Fix selfballooning and ensure it doesn't go too far

[Dan Magenheimer <dan.magenheimer@oracle.com>]

> From: David Vrabel [mailto:david.vrabel@citrix.com]
> Subject: Re: [PATCH v2] xen: Fix selfballooning and ensure it doesn't go too far
> 
> On 27/09/11 16:03, Dan Magenheimer wrote:
> > Note: This patch is also now in a git tree at:
> >
> > git://oss.oracle.com/git/djm/tmem.git#selfballoon-fix-v2
> >
> > The balloon driver's "current_pages" is very different from
> > totalram_pages.  Self-ballooning needs to be driven by
> > the latter.

Hi David --

Thanks for the feedback!
 
> I don't think this part of the change makes any difference. It looks like it
> rearranges the maths without changing the end result (other than
> slightly increasing the rate of change).
> I think this (partial, untested) patch is equivalent:

Actually it does.  The key difference is the parameter to the
call to balloon_set_new_target.  The math in my patch is
done in "internal" math (e.g. kernel-relevant variables)
and the math in your patch is done in "external" math (e.g.
Xen-relevant variables).  Balloon_set_new_target requires
"external" math, so I convert at the point of call.
 
> The sysfs file isn't documented (but then neither are any of the other
> (self-)balloon driver sysfs files).

Yep.  This is a bug fix, so I'm not trying to fix all the sins
of others (and myself).  Since you are familiar with the
meaning of all the core balloon driver variables exposed through
sysfs, perhaps you might submit a patch to document them
and/or suggest which ones should be in debugfs instead?
 
> I don't think "safety_margin" is the right name.  Perhaps,
> "min_reservation_ratio" or something like that?

Yeah, I struggled with the name because the concept that
the variable implements is pretty complex. I finally decided
on safety_margin because I think it will draw the attention
of a user who has reason to look for it.  I don't expect that
it will be used anyway, but it is there in case I am wrong.