Merge pull request #935 from vishh/cgo

Use update time to detect if kmem limits have been set

Author: hqhq

libcontainer/cgroups/fs/memory.go

@@ -5,15 +5,14 @@ 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 {
@@ -28,53 +27,65 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
 	if err != nil && !cgroups.IsNotFound(err) {
 		return err
 	}
-	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
-		}
+	// reset error.
+	err = nil
+	if path == "" {
+		// Invalid input.
+		return fmt.Errorf("invalid path for memory cgroups: %+v", d)
 	}
-
 	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.
-	_, err = d.join("memory")
-	if err != nil && !cgroups.IsNotFound(err) {
+	if _, jerr := d.join("memory"); jerr != nil && !cgroups.IsNotFound(jerr) {
+		err = jerr
 		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?
-		kmemValue, err := getCgroupParamUint(path, "memory.kmem.limit_in_bytes")
+		// 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"))
 		if err != nil {
 			return err
 		}
-		switch system.GetLongBit() {
-		case 32:
-			kmemInitialized = uint32(kmemValue) != uint32(math.MaxUint32)
-		case 64:
-			kmemInitialized = kmemValue != uint64(math.MaxUint64)
+		kmemLimitsUpdateTime, err := getModifyTime(filepath.Join(path, "memory.kmem.limit_in_bytes"))
+		if err != nil {
+			return err
 		}
-
+		// 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")
@@ -85,7 +96,6 @@ 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,6 +227,12 @@ 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,28 +4,9 @@ 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())
-}