From 8dd3d634556427c22769125ef2b0aa2ce613a2a7 Mon Sep 17 00:00:00 2001 From: Vishnu kannan Date: Wed, 6 Jul 2016 15:14:25 -0700 Subject: [PATCH 1/2] Look at modify time to check if kmem limits are initialized. Signed-off-by: Vishnu kannan --- libcontainer/cgroups/fs/memory.go | 62 +++++++++++++++----------- libcontainer/cgroups/fs/memory_test.go | 6 +++ 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 01c25effac4..b8371282913 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/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 } diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go index af3a438f8e0..b9637cd3e3c 100644 --- a/libcontainer/cgroups/fs/memory_test.go +++ b/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{} From c501cc038a2f58c2f2a99047c7697f1624b942b8 Mon Sep 17 00:00:00 2001 From: Vishnu kannan Date: Wed, 6 Jul 2016 15:23:01 -0700 Subject: [PATCH 2/2] Remove unused GetLongBit() function. Signed-off-by: Vishnu kannan --- libcontainer/system/sysconfig.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/libcontainer/system/sysconfig.go b/libcontainer/system/sysconfig.go index 4fba6c2b704..b3a07cba3ef 100644 --- a/libcontainer/system/sysconfig.go +++ b/libcontainer/system/sysconfig.go @@ -4,28 +4,9 @@ package system /* #include -#include - -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()) -}