From 3d9074ead33a5c27dc20bb49457c69c6d2ae6b57 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Fri, 23 Jun 2017 17:17:00 -0700 Subject: [PATCH] Update memory specs to use int64 not uint64 replace #1492 #1494 fix #1422 Since https://github.com/opencontainers/runtime-spec/pull/876 the memory specifications are now `int64`, as that better matches the visible interface where `-1` is a valid value. Otherwise finding the correct value was difficult as it was kernel dependent. Signed-off-by: Justin Cormack --- libcontainer/cgroups/fs/memory.go | 36 +++++++++++++--------------- libcontainer/configs/cgroup_linux.go | 10 ++++---- update.go | 14 +++++------ 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index da2cc9f8b6f..b739c6315c4 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -73,14 +73,14 @@ func EnableKernelMemoryAccounting(path string) error { // until a limit is set on the cgroup and limit cannot be set once the // cgroup has children, or if there are already tasks in the cgroup. for _, i := range []int64{1, -1} { - if err := setKernelMemory(path, uint64(i)); err != nil { + if err := setKernelMemory(path, i); err != nil { return err } } return nil } -func setKernelMemory(path string, kernelMemoryLimit uint64) error { +func setKernelMemory(path string, kernelMemoryLimit int64) error { if path == "" { return fmt.Errorf("no such directory for %s", cgroupKernelMemoryLimit) } @@ -88,7 +88,7 @@ func setKernelMemory(path string, kernelMemoryLimit uint64) error { // kernel memory is not enabled on the system so we should do nothing return nil } - if err := ioutil.WriteFile(filepath.Join(path, cgroupKernelMemoryLimit), []byte(strconv.FormatUint(kernelMemoryLimit, 10)), 0700); err != nil { + if err := ioutil.WriteFile(filepath.Join(path, cgroupKernelMemoryLimit), []byte(strconv.FormatInt(kernelMemoryLimit, 10)), 0700); err != nil { // Check if the error number returned by the syscall is "EBUSY" // The EBUSY signal is returned on attempts to write to the // memory.kmem.limit_in_bytes file if the cgroup has children or @@ -106,14 +106,12 @@ func setKernelMemory(path string, kernelMemoryLimit uint64) error { } func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error { - ulimited := -1 - - // If the memory update is set to uint64(-1) we should also - // set swap to uint64(-1), it means unlimited memory. - if cgroup.Resources.Memory == uint64(ulimited) { - // Only set swap if it's enbled in kernel + // If the memory update is set to -1 we should also + // set swap to -1, it means unlimited memory. + if cgroup.Resources.Memory == -1 { + // Only set swap if it's enabled in kernel if cgroups.PathExists(filepath.Join(path, cgroupMemorySwapLimit)) { - cgroup.Resources.MemorySwap = uint64(ulimited) + cgroup.Resources.MemorySwap = -1 } } @@ -128,29 +126,29 @@ func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error { // When update memory limit, we should adapt the write sequence // for memory and swap memory, so it won't fail because the new // value and the old value don't fit kernel's validation. - if cgroup.Resources.MemorySwap == uint64(ulimited) || memoryUsage.Limit < cgroup.Resources.MemorySwap { - if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatUint(cgroup.Resources.MemorySwap, 10)); err != nil { + if cgroup.Resources.MemorySwap == -1 || memoryUsage.Limit < uint64(cgroup.Resources.MemorySwap) { + if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { return err } - if err := writeFile(path, cgroupMemoryLimit, strconv.FormatUint(cgroup.Resources.Memory, 10)); err != nil { + if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { return err } } else { - if err := writeFile(path, cgroupMemoryLimit, strconv.FormatUint(cgroup.Resources.Memory, 10)); err != nil { + if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { return err } - if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatUint(cgroup.Resources.MemorySwap, 10)); err != nil { + if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { return err } } } else { if cgroup.Resources.Memory != 0 { - if err := writeFile(path, cgroupMemoryLimit, strconv.FormatUint(cgroup.Resources.Memory, 10)); err != nil { + if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { return err } } if cgroup.Resources.MemorySwap != 0 { - if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatUint(cgroup.Resources.MemorySwap, 10)); err != nil { + if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { return err } } @@ -171,13 +169,13 @@ func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error { } if cgroup.Resources.MemoryReservation != 0 { - if err := writeFile(path, "memory.soft_limit_in_bytes", strconv.FormatUint(cgroup.Resources.MemoryReservation, 10)); err != nil { + if err := writeFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemoryReservation, 10)); err != nil { return err } } if cgroup.Resources.KernelMemoryTCP != 0 { - if err := writeFile(path, "memory.kmem.tcp.limit_in_bytes", strconv.FormatUint(cgroup.Resources.KernelMemoryTCP, 10)); err != nil { + if err := writeFile(path, "memory.kmem.tcp.limit_in_bytes", strconv.FormatInt(cgroup.Resources.KernelMemoryTCP, 10)); err != nil { return err } } diff --git a/libcontainer/configs/cgroup_linux.go b/libcontainer/configs/cgroup_linux.go index 3e0509deb9c..e15a662f522 100644 --- a/libcontainer/configs/cgroup_linux.go +++ b/libcontainer/configs/cgroup_linux.go @@ -43,19 +43,19 @@ type Resources struct { Devices []*Device `json:"devices"` // Memory limit (in bytes) - Memory uint64 `json:"memory"` + Memory int64 `json:"memory"` // Memory reservation or soft_limit (in bytes) - MemoryReservation uint64 `json:"memory_reservation"` + MemoryReservation int64 `json:"memory_reservation"` // Total memory usage (memory + swap); set `-1` to enable unlimited swap - MemorySwap uint64 `json:"memory_swap"` + MemorySwap int64 `json:"memory_swap"` // Kernel memory limit (in bytes) - KernelMemory uint64 `json:"kernel_memory"` + KernelMemory int64 `json:"kernel_memory"` // Kernel memory limit for TCP use (in bytes) - KernelMemoryTCP uint64 `json:"kernel_memory_tcp"` + KernelMemoryTCP int64 `json:"kernel_memory_tcp"` // CPU shares (relative weight vs. other containers) CpuShares uint64 `json:"cpu_shares"` diff --git a/update.go b/update.go index 0ea90d607ac..133be999685 100644 --- a/update.go +++ b/update.go @@ -124,11 +124,11 @@ other options are ignored. r := specs.LinuxResources{ Memory: &specs.LinuxMemory{ - Limit: u64Ptr(0), - Reservation: u64Ptr(0), - Swap: u64Ptr(0), - Kernel: u64Ptr(0), - KernelTCP: u64Ptr(0), + Limit: i64Ptr(0), + Reservation: i64Ptr(0), + Swap: i64Ptr(0), + Kernel: i64Ptr(0), + KernelTCP: i64Ptr(0), }, CPU: &specs.LinuxCPU{ Shares: u64Ptr(0), @@ -213,7 +213,7 @@ other options are ignored. } for _, pair := range []struct { opt string - dest *uint64 + dest *int64 }{ {"memory", r.Memory.Limit}, {"memory-swap", r.Memory.Swap}, @@ -232,7 +232,7 @@ other options are ignored. } else { v = -1 } - *pair.dest = uint64(v) + *pair.dest = v } } r.Pids.Limit = int64(context.Int("pids-limit"))