[libvirt] [PATCH v3] cgroup: Use system reported "unlimited" value for comparison

[Viktor Mihajlovski]

With kernel 3.18 (since commit 3e32cb2e0a12b6915056ff04601cf1bb9b44f967)
the "unlimited" value for cgroup memory limits has changed once again as
its byte value is now computed from a page counter.
The new "unlimited" value reported by the cgroup fs is therefore 2**51-1
pages which is (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - 3072). This results
e.g. in virsh memtune displaying 9007199254740988 instead of unlimited
for the limits.

This patch uses the value of memory.limit_in_bytes from the cgroup
memory root which is the system's "real" unlimited value for comparison.

See also libvirt commit 231656bbeb9e4d3bedc44362784c35eee21cf0f4 for the
history for kernel 3.12 and before.

Signed-off-by: Viktor Mihajlovski <mihajlov at linux.vnet.ibm.com>
---

v2:
- removed RFC verbiage from commit message
- per Martin's review comment, cache the cgroup memory.limit_in_bytes
- used the cgroup detection logic proposed by Martin, much nicer now indeed
- other than initially planned, fall back to VIR_DOMAIN_MEMORY_PARAM_UNLIMITED
  in case of cgroup read failure, since the usual paranoia fits in nicely with
  the "already initialized" check. With the current code structure this will
  never be called when the memory controller is not configured or mounted
  anyway...
- while at it, replaced the goto cleanups with direct returns as there's
  really no cleanup to be done in the GetMemoryxxxLimit functions

v3:
- use thread safe one time initialization for the "unlimited" value, to 
  account for 64-bit architectures which do not guarantee atomic memory
  updates of long long values (which reportedly is the case for ARM).

  src/util/vircgroup.c | 73 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 18 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index f151193..b6affe3 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -60,6 +60,7 @@
 #include "virsystemd.h"
 #include "virtypedparam.h"
 #include "virhostcpu.h"
+#include "virthread.h"
 
 VIR_LOG_INIT("util.cgroup");
 
@@ -2452,6 +2453,51 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group,
 }
 
 
+/*
+ * Retrieve the "memory.limit_in_bytes" value from the memory controller
+ * root dir. This value cannot be modified by userspace and therefore
+ * is the maximum limit value supported by cgroups on the local system.
+ * Returns this value scaled to KB or falls back to the original
+ * VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. Either way, remember the return
+ * value to avoid unnecessary cgroup filesystem access.
+ */
+static unsigned long long int virCgroupMemoryUnlimitedKB;
+static virOnceControl virCgroupMemoryOnce = VIR_ONCE_CONTROL_INITIALIZER;
+
+static void
+virCgroupMemoryOnceInit(void)
+{
+    virCgroupPtr group;
+    unsigned long long int mem_unlimited = 0ULL;
+
+    if (virCgroupNew(-1, "/", NULL, -1, &group) < 0)
+        goto cleanup;
+
+    if (!virCgroupHasController(group, VIR_CGROUP_CONTROLLER_MEMORY))
+        goto cleanup;
+
+    ignore_value(virCgroupGetValueU64(group,
+                                      VIR_CGROUP_CONTROLLER_MEMORY,
+                                      "memory.limit_in_bytes",
+                                      &mem_unlimited));
+ cleanup:
+    virCgroupFree(&group);
+    virCgroupMemoryUnlimitedKB = mem_unlimited >> 10;
+}
+
+static unsigned long long int
+virCgroupGetMemoryUnlimitedKB(void)
+{
+    if (virOnce(&virCgroupMemoryOnce, virCgroupMemoryOnceInit) < 0)
+        VIR_DEBUG("Init failed, will fall back to defaults.");
+
+    if (virCgroupMemoryUnlimitedKB)
+        return virCgroupMemoryUnlimitedKB;
+    else
+        return VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
+}
+
+
 /**
  * virCgroupSetMemory:
  *
@@ -2534,20 +2580,17 @@ int
 virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb)
 {
     long long unsigned int limit_in_bytes;
-    int ret = -1;
 
     if (virCgroupGetValueU64(group,
                              VIR_CGROUP_CONTROLLER_MEMORY,
                              "memory.limit_in_bytes", &limit_in_bytes) < 0)
-        goto cleanup;
+        return -1;
 
     *kb = limit_in_bytes >> 10;
-    if (*kb > VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
+    if (*kb >= virCgroupGetMemoryUnlimitedKB())
         *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
 
-    ret = 0;
- cleanup:
-    return ret;
+    return 0;
 }
 
 
@@ -2596,20 +2639,17 @@ int
 virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb)
 {
     long long unsigned int limit_in_bytes;
-    int ret = -1;
 
     if (virCgroupGetValueU64(group,
                              VIR_CGROUP_CONTROLLER_MEMORY,
                              "memory.soft_limit_in_bytes", &limit_in_bytes) < 0)
-        goto cleanup;
+        return -1;
 
     *kb = limit_in_bytes >> 10;
-    if (*kb > VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
+    if (*kb >= virCgroupGetMemoryUnlimitedKB())
         *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
 
-    ret = 0;
- cleanup:
-    return ret;
+    return 0;
 }
 
 
@@ -2658,20 +2698,17 @@ int
 virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb)
 {
     long long unsigned int limit_in_bytes;
-    int ret = -1;
 
     if (virCgroupGetValueU64(group,
                              VIR_CGROUP_CONTROLLER_MEMORY,
                              "memory.memsw.limit_in_bytes", &limit_in_bytes) < 0)
-        goto cleanup;
+        return -1;
 
     *kb = limit_in_bytes >> 10;
-    if (*kb > VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
+    if (*kb >= virCgroupGetMemoryUnlimitedKB())
         *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
 
-    ret = 0;
- cleanup:
-    return ret;
+    return 0;
 }
 
 
-- 
1.9.1