[libvirt] [PATCH 1/3] cgroups: Redefine what "unlimited" means wrt memory limits

[Daniel P. Berrange]

On Mon, Dec 09, 2013 at 09:22:51AM +0100, Martin Kletzander wrote:
> Since kernel 3.12 (commit 34ff8dc08956098563989d8599840b130be81252 in
> linux-stable.git in particular) the value for 'unlimited' in cgroup
> memory limits changed from LLONG_MAX to ULLONG_MAX.  Due to rather
> unfortunate choice of our VIR_DOMAIN_MEMORY_PARAM_UNLIMITED constant
> (which we transfer as an unsigned long long in Kibibytes), we ended up
> with the situation described below (applies to x86_64):
> 
>  - 2^64-1 (ULLONG_MAX) -- "unlimited" in kernel = 3.12
> 
>  - 2^63-1 (LLONG_MAX) -- "unlimited" in kernel < 3.12
>  - 2^63-1024 -- our PARAM_UNLIMITED scaled to Bytes
> 
>  - 2^53-1 -- our PARAM_UNLIMITED unscaled (in Kibibytes)
> 
> This means that when any number within (2^63-1, 2^64-1] is read from
> memory cgroup, we are transferring that number instead of "unlimited".
> Unfortunately, changing VIR_DOMAIN_MEMORY_PARAM_UNLIMITED would break
> ABI compatibility and thus we have to resort to a different solution.
> 
> With this patch every value greater than PARAM_UNLIMITED means
> "unlimited".  Even though this may seem misleading, we are already in
> such unclear situation when running 3.12 kernel with memory limits set
> to 2^63.
> 
> One example showing most of the problems at once (with kernel 3.12.2):
>  # virsh memtune asdf --hard-limit 9007199254740991 --swap-hard-limit -1
>  # echo 12345678901234567890 >\
> /sys/fs/cgroup/memory/machine/asdf.libvirt-qemu/memory.soft_limit_in_bytes
>  # virsh memtune asdf
>  hard_limit     : 18014398509481983
>  soft_limit     : 12056327051986884
>  swap_hard_limit: 18014398509481983
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/util/vircgroup.c | 57 +++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 9674328..43eb649 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -1957,12 +1957,19 @@ int
>  virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb)
>  {
>      long long unsigned int limit_in_bytes;
> -    int ret;
> -    ret = virCgroupGetValueU64(group,
> -                               VIR_CGROUP_CONTROLLER_MEMORY,
> -                               "memory.limit_in_bytes", &limit_in_bytes);
> -    if (ret == 0)
> -        *kb = limit_in_bytes >> 10;
> +    int ret = -1;
> +
> +    if (virCgroupGetValueU64(group,
> +                             VIR_CGROUP_CONTROLLER_MEMORY,
> +                             "memory.limit_in_bytes", &limit_in_bytes) < 0)
> +        goto cleanup;
> +
> +    *kb = limit_in_bytes >> 10;
> +    if (*kb > VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
> +        *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> +
> +    ret = 0;
> + cleanup:
>      return ret;
>  }
> 
> @@ -2012,12 +2019,19 @@ int
>  virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb)
>  {
>      long long unsigned int limit_in_bytes;
> -    int ret;
> -    ret = virCgroupGetValueU64(group,
> -                               VIR_CGROUP_CONTROLLER_MEMORY,
> -                               "memory.soft_limit_in_bytes", &limit_in_bytes);
> -    if (ret == 0)
> -        *kb = limit_in_bytes >> 10;
> +    int ret = -1;
> +
> +    if (virCgroupGetValueU64(group,
> +                             VIR_CGROUP_CONTROLLER_MEMORY,
> +                             "memory.soft_limit_in_bytes", &limit_in_bytes) < 0)
> +        goto cleanup;
> +
> +    *kb = limit_in_bytes >> 10;
> +    if (*kb > VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
> +        *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> +
> +    ret = 0;
> + cleanup:
>      return ret;
>  }
> 
> @@ -2067,12 +2081,19 @@ int
>  virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb)
>  {
>      long long unsigned int limit_in_bytes;
> -    int ret;
> -    ret = virCgroupGetValueU64(group,
> -                               VIR_CGROUP_CONTROLLER_MEMORY,
> -                               "memory.memsw.limit_in_bytes", &limit_in_bytes);
> -    if (ret == 0)
> -        *kb = limit_in_bytes >> 10;
> +    int ret = -1;
> +
> +    if (virCgroupGetValueU64(group,
> +                             VIR_CGROUP_CONTROLLER_MEMORY,
> +                             "memory.memsw.limit_in_bytes", &limit_in_bytes) < 0)
> +        goto cleanup;
> +
> +    *kb = limit_in_bytes >> 10;
> +    if (*kb > VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
> +        *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> +
> +    ret = 0;
> + cleanup:
>      return ret;
>  }

ACK

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|