Skip to content

Commit

Permalink
Update memory specs to use int64 not uint64
Browse files Browse the repository at this point in the history 
replace opencontainers#1492 opencontainers#1494
fix opencontainers#1422

Since opencontainers/runtime-spec#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 <justin.cormack@docker.com>
  • Loading branch information
@justincormack @adrianreber
justincormack authored and adrianreber committed Jul 27, 2017
1 parent cf34527 commit eec6814
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 31 deletions.
36 changes: 17 additions & 19 deletions libcontainer/cgroups/fs/memory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,22 @@ 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)
}
if !cgroups.PathExists(filepath.Join(path, cgroupKernelMemoryLimit)) {
// 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
Expand All @@ -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
}
}

Expand All @@ -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
}
}
Expand All @@ -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
}
}
Expand Down
10 changes: 5 additions & 5 deletions libcontainer/configs/cgroup_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
14 changes: 7 additions & 7 deletions update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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},
Expand All @@ -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"))
Expand Down

0 comments on commit eec6814

Please sign in to comment.