Re: [PATCH for 4.16] xl: Add xl.conf-based dom0 autoballoon limits

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Feb 1, 2021, at 1:39 PM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 29.01.2021 17:48, George Dunlap wrote:
>> When in "autoballoon" mode, xl will attempt to balloon dom0 down in
>> order to free up memory to start a guest, based on the expected amount
>> of memory reported by libxl.  Currently, this is limited in libxl with
>> a hard-coded value of 128MiB, to prevent squeezing dom0 so hard that
>> it OOMs.  On many systems, however, 128MiB is far too low a limit, and
>> dom0 may crash anyway.
>> 
>> Furthermore, "autoballoon" mode, although strongly recommended
>> against, must be the default mode for most distros: Having the memory
>> available to Linux drop to 1GiB would be too much of an unexpected
>> surprise for those not familiar with Xen.  This leads to a situation,
>> however, where starting too many guests on a large-ish system can
>> unexpectedly cause the system to slow down and crash with no warning,
>> as xl squeezes dom0 until it OOMs.
>> 
>> Ideally what we want is to retain the "just works after install"
>> functionality that we have now with respect to autoballooning, but
>> prompts the admin to consider autoballooning issues once they've
>> demonstrated that they intend to use a significant percentage of the
>> host memory to start guests, and also allow knowledgable users the
>> flexibility to configure the system as they see fit.
>> 
>> To do this, introduce two new xl.conf-based dom0 autoballoon limits:
>> autoballoon_dom0_min_memmb, and autoballoon_dom0_min_mempct.
>> 
>> When parsing xl.conf, xl will always calculate a minimum value for
>> dom0 target.  If autoballoon_dom0_min_memmb is set, it will just use
>> that; if that is not set and _min_mempct is set, it will calculate the
>> minimum target based on a percentage of host memory.  If neither is
>> set, it will default to 25% of host memory.
>> 
>> Add a more useful error message when autoballoon fails due to missing
>> the target.  Additionally, if the autoballoon target was automatic,
>> post an additional message prompting the admin to consider autoballoon
>> explicitly.  Hopefully this will balance things working out of the box
>> (and make it possible for advanced users to configure their systems as
>> they wish), yet prompt admins to explore further when it's
>> appropriate.
>> 
>> NB that there's a race in the resulting code between
>> libxl_get_memory_target() and libxl_set_memory_target(); but there was
>> already a race between the latter and libxl_get_free_memory() anyway;
>> this doesn't really make the situation worse.
>> 
>> While here, reduce the scope of the free_memkb variable, which isn't
>> used outside the do{} loop in freemem().
>> 
>> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> I'm not convinced it is the tool stack to set a lower limit here.
> Imo the kernel should guard itself from too aggressive ballooning.
> In fact, the old XenoLinux driver did, as of
> https://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/b61443b1bf76,
> which in our forward ports we then extended to have exposure in
> /proc and /sys, alongside an upper limit (for purely informational
> purposes iirc).

Just to be clear, the limit in this patch will *only* affect:

1. autoballooning
2. done by xl
3. in response to an `xl create`

You can still crash your dom0 just fine by running `xl mem-set 0 $TOO_SMALL`; 
and this patch won’t affect the behavior of alternate toolstacks implemented on 
libxl (e.g., libvirt).

I’d certainly be in favor of getting something like b61443b1bf76 upstream.  I 
still think a patch like this one would be useful though:

1. The error message will be given right away, rather than timing out on the 
dom0 balloon driver

2. The error message can be more informative, and point people to the whole 
“fixed dom0 memory” thing.

 -George