Revert "Use update time to detect if kmem limits have been set"

Revert: #935
Fixes: #946

I can reproduce #946 on some machines, the problem is on
some machines, it could be very fast that modify time
of `memory.kmem.limit_in_bytes` could be the same as
before it's modified.

And now we'll call `SetKernelMemory` twice on container
creation which cause the second time failure.

Revert this before we find a better solution.

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

libcontainer/cgroups/fs/memory.go

@@ -5,14 +5,15 @@ package fs
 import (
 	"bufio"
 	"fmt"
+	"math"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runc/libcontainer/system"
 )
 
 type MemoryGroup struct {
@@ -27,65 +28,51 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
 	if err != nil && !cgroups.IsNotFound(err) {
 		return err
 	}
-	// reset error.
-	err = nil
-	if path == "" {
-		// Invalid input.
-		return fmt.Errorf("invalid path for memory cgroups: %+v", d)
+	if memoryAssigned(d.config) {
+		if path != "" {
+			if err := os.MkdirAll(path, 0755); err != nil {
+				return err
+			}
+		}
+		// We have to set kernel memory here, as we can't change it once
+		// processes have been attached to the cgroup.
+		if err := s.SetKernelMemory(path, d.config); err != nil {
+			return err
+		}
 	}
 	defer func() {
 		if err != nil {
 			os.RemoveAll(path)
 		}
 	}()
-	if !cgroups.PathExists(path) {
-		if err = os.MkdirAll(path, 0755); err != nil {
-			return err
-		}
-	}
-	if memoryAssigned(d.config) {
-		// We have to set kernel memory here, as we can't change it once
-		// processes have been attached to the cgroup.
-		if err = s.SetKernelMemory(path, d.config); err != nil {
-			return err
-		}
-	}
+
 	// We need to join memory cgroup after set memory limits, because
 	// kmem.limit_in_bytes can only be set when the cgroup is empty.
-	if _, jerr := d.join("memory"); jerr != nil && !cgroups.IsNotFound(jerr) {
-		err = jerr
+	_, err = d.join("memory")
+	if err != nil && !cgroups.IsNotFound(err) {
 		return err
 	}
 	return nil
 }
 
-func getModifyTime(path string) (time.Time, error) {
-	stat, err := os.Stat(path)
-	if err != nil {
-		return time.Time{}, fmt.Errorf("failed to get memory cgroups creation time: %v", err)
-	}
-	return stat.ModTime(), nil
-}
-
 func (s *MemoryGroup) SetKernelMemory(path string, cgroup *configs.Cgroup) error {
 	// This has to be done separately because it has special
 	// constraints (it can only be initialized before setting up a
 	// hierarchy or adding a task to the cgroups. However, if
 	// sucessfully initialized, it can be updated anytime afterwards)
 	if cgroup.Resources.KernelMemory != 0 {
+		kmemInitialized := false
 		// Is kmem.limit_in_bytes already set?
-		// memory.kmem.max_usage_in_bytes is a read-only file. Use it to get cgroups creation time.
-		kmemCreationTime, err := getModifyTime(filepath.Join(path, "memory.kmem.max_usage_in_bytes"))
+		kmemValue, err := getCgroupParamUint(path, "memory.kmem.limit_in_bytes")
 		if err != nil {
 			return err
 		}
-		kmemLimitsUpdateTime, err := getModifyTime(filepath.Join(path, "memory.kmem.limit_in_bytes"))
-		if err != nil {
-			return err
+		switch system.GetLongBit() {
+		case 32:
+			kmemInitialized = uint32(kmemValue) != uint32(math.MaxUint32)
+		case 64:
+			kmemInitialized = kmemValue != uint64(math.MaxUint64)
 		}
-		// kmem.limit_in_bytes has already been set if its update time is after that of creation time.
-		// We use `!=` op instead of `>` because updates are losing precision compared to creation.
-		kmemInitialized := !kmemLimitsUpdateTime.Equal(kmemCreationTime)
 		if !kmemInitialized {
 			// If there's already tasks in the cgroup, we can't change the limit either
 			tasks, err := getCgroupParamString(path, "tasks")
@@ -96,6 +83,7 @@ func (s *MemoryGroup) SetKernelMemory(path string, cgroup *configs.Cgroup) error
 				return fmt.Errorf("cannot set kmem.limit_in_bytes after task have joined this cgroup")
 			}
 		}
+
 		if err := writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(cgroup.Resources.KernelMemory, 10)); err != nil {
 			return err
 		}

libcontainer/cgroups/fs/memory_test.go

@@ -227,12 +227,6 @@ func TestMemorySetKernelMemory(t *testing.T) {
 	helper.writeFileContents(map[string]string{
 		"memory.kmem.limit_in_bytes": strconv.Itoa(kernelMemoryBefore),
 	})
-	helper.writeFileContents(map[string]string{
-		"memory.kmem.max_usage_in_bytes": strconv.Itoa(kernelMemoryBefore),
-	})
-	helper.writeFileContents(map[string]string{
-		"tasks": "",
-	})
 
 	helper.CgroupData.config.Resources.KernelMemory = kernelMemoryAfter
 	memory := &MemoryGroup{}

libcontainer/system/sysconfig.go

@@ -4,9 +4,28 @@ package system
 
 /*
 #include <unistd.h>
+#include <limits.h>
+
+int GetLongBit() {
+#ifdef _SC_LONG_BIT
+    int longbits;
+
+    longbits = sysconf(_SC_LONG_BIT);
+    if (longbits <  0) {
+        longbits = (CHAR_BIT * sizeof(long));
+    }
+    return longbits;
+#else
+    return (CHAR_BIT * sizeof(long));
+#endif
+}
 */
 import "C"
 
 func GetClockTicks() int {
 	return int(C.sysconf(C._SC_CLK_TCK))
 }
+
+func GetLongBit() int {
+	return int(C.GetLongBit())
+}