| Message ID | 20210129164858.3280477-1-george.dunlap@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for,4.16] xl: Add xl.conf-based dom0 autoballoon limits | expand |
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@citrix.com> 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). Jan
> On Feb 1, 2021, at 1:39 PM, Jan Beulich <jbeulich@suse.com> 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@citrix.com> > > 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
George Dunlap writes ("Re: [PATCH for 4.16] xl: Add xl.conf-based dom0 autoballoon limits"):
> 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.
I agree.
If the memory ballooning accounting area wasn't already a swamp I
would be considering this patch for 4.15. As it is I think it unwise
to make this change now because I don't see how to be confident it
won't break anything.
Ian.
diff --git a/docs/man/xl.conf.5.pod b/docs/man/xl.conf.5.pod index 41ee428744..efe259e47d 100644 --- a/docs/man/xl.conf.5.pod +++ b/docs/man/xl.conf.5.pod @@ -72,6 +72,25 @@ of memory given to domain 0 by default. Default: C<"auto"> +=item B<autoballoon_dom0_min_memmb=NUMBER> + +When autoballooning, C<xl> will refuse to balloon dom0 down below this +number of megabytes. + +Note that C<libxl> has its own internal limit for ballooning dom0. + +If unset, C<autoballoon_dom0_min_mempct> will be used instead. + +=item B<autoballoon_dom0_min_mempct=NUMBER> + +When autoballooning, C<xl> will refuse to balloon dom0 down below this +percentage of host memory. + +If C<autoballoon_dom0_min_memmb> is set, then this value will be +ignored. + +Default: C<25> + =item B<run_hotplug_scripts=BOOLEAN> If disabled hotplug scripts will be called from udev, as it used to diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf index 95f2f442d3..c4cec25989 100644 --- a/tools/examples/xl.conf +++ b/tools/examples/xl.conf @@ -9,6 +9,15 @@ # starts with all the host's memory. #autoballoon="auto" +# If autoballooning is enabled, control the minimum value below which +# xl will not balloon dom0. autoballoon_dom0_min_memmb is the minimum +# value in megabytes; autoballoon_dom0_min_mempct is a percentage of +# host memory. If autoballoon_dom0_min_memmb is set, +# autoballoon_dom0_min_mempct will be ignored. If neither is set, the +# default value of autoballoon_dom0_min_mempct will be used. +#autoballoon_dom0_min_memmb = 1024 +#autoballoon_dom0_min_mempct = 25 + # full path of the lockfile used by xl during domain creation #lockfile="/var/lock/xl" diff --git a/tools/xl/xl.c b/tools/xl/xl.c index 2a5ddd4390..ab6ee09a9d 100644 --- a/tools/xl/xl.c +++ b/tools/xl/xl.c @@ -37,6 +37,8 @@ xentoollog_logger_stdiostream *logger; int dryrun_only; int force_execution; int autoballoon = -1; +uint64_t autoballoon_dom0_min_memkb; +bool autoballoon_dom0_min_is_default; char *blkdev_start; int run_hotplug_scripts = 1; char *lockfile; @@ -124,6 +126,42 @@ static void parse_global_config(const char *configfile, if (autoballoon == -1) autoballoon = auto_autoballoon(); + if (!xlu_cfg_get_long(config, "autoballoon_dom0_min_memmb", &l, 0)) { + /* Convert from megabytes to kilobytes */ + autoballoon_dom0_min_memkb = l * 1024; + } else { + /* Minimum KB not set; look for a minimum percentage of host RAM */ + libxl_physinfo info; + const libxl_version_info *vinfo; + + if (!xlu_cfg_get_long(config, "autoballoon_dom0_min_mempct", &l, 0)) { + if (l < 0 || l > 100) { + fprintf(stderr, "Invalid autoballoon_dom0_min_memkb option"); + exit(EXIT_FAILURE); + } + } else { + /* + * Default to 25% of host memory. Lower than this should + * require admin intervention. + */ + l = 25; + autoballoon_dom0_min_is_default = true; + } + + if (libxl_get_physinfo(ctx, &info) != 0) { + fprintf(stderr, "libxl_get_physinfo failed.\n"); + exit(EXIT_FAILURE); + } + + vinfo = libxl_get_version_info(ctx); + + /* First calculate total host memory in kb */ + autoballoon_dom0_min_memkb = (info.total_pages * vinfo->pagesize) / (1 << 10); + + /* Then apply the percentage */ + autoballoon_dom0_min_memkb = autoballoon_dom0_min_memkb * l / 100; + } + if (!xlu_cfg_get_long (config, "run_hotplug_scripts", &l, 0)) run_hotplug_scripts = l; diff --git a/tools/xl/xl.h b/tools/xl/xl.h index 06569c6c4a..d027656ed4 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -265,6 +265,8 @@ int child_report(xlchildnum child); /* global options */ extern int autoballoon; +uint64_t autoballoon_dom0_min_memkb; +bool autoballoon_dom0_min_is_default; extern int run_hotplug_scripts; extern int dryrun_only; extern int claim_mode; diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c index 435155a033..def8e6ee1a 100644 --- a/tools/xl/xl_vmcontrol.c +++ b/tools/xl/xl_vmcontrol.c @@ -322,7 +322,7 @@ static int domain_wait_event(uint32_t domid, libxl_event **event_r) static bool freemem(uint32_t domid, libxl_domain_config *d_config) { int rc, retries = 3; - uint64_t need_memkb, free_memkb; + uint64_t need_memkb; if (!autoballoon) return true; @@ -332,6 +332,8 @@ static bool freemem(uint32_t domid, libxl_domain_config *d_config) return false; do { + uint64_t free_memkb, target_memkb; + rc = libxl_get_free_memory(ctx, &free_memkb); if (rc < 0) return false; @@ -339,7 +341,24 @@ static bool freemem(uint32_t domid, libxl_domain_config *d_config) if (free_memkb >= need_memkb) return true; - rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0); + rc = libxl_get_memory_target(ctx, 0, &target_memkb); + if (rc < 0) + return false; + + target_memkb -= need_memkb - free_memkb; + + if (target_memkb < autoballoon_dom0_min_memkb) { + fprintf(stderr, "Autoballoon: %"PRIu64"kb below minimum dom0 target %"PRIu64"kb\n", + target_memkb, autoballoon_dom0_min_memkb); + if (autoballoon_dom0_min_is_default) { + fprintf(stderr, "\nConsider setting autoballoon minimum target explicitly,\n" \ + "or setting dom0's memory allocation explicitly.\n" \ + "see the xl.conf man page for more information.\n\n"); + } + return false; + } + + rc = libxl_set_memory_target(ctx, 0, target_memkb, 0, 0); if (rc < 0) return false;
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@citrix.com> --- NB this is targeted at 4.16, having missed the last posting date. Just trying to get early feedback. CC: Ian Jackson <ian.jackson@citrix.com> CC: Wei Liu <wl@xen.org> CC: Anthony Perard <anthony.perard@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> --- docs/man/xl.conf.5.pod | 19 +++++++++++++++++++ tools/examples/xl.conf | 9 +++++++++ tools/xl/xl.c | 38 ++++++++++++++++++++++++++++++++++++++ tools/xl/xl.h | 2 ++ tools/xl/xl_vmcontrol.c | 23 +++++++++++++++++++++-- 5 files changed, 89 insertions(+), 2 deletions(-)