From e45609605934b1f5b916be5f22d32be19185f460 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Mon, 29 Feb 2016 10:21:08 +0800 Subject: [PATCH] Fix problem when update memory and swap memory Currently, if we start a container with: `docker run -ti --name foo --memory 300M --memory-swap 500M busybox sh` Then we want to update it with: `docker update --memory 600M --memory-swap 800M foo` It'll get error because we can't set memory to 600M with the 500M limit of swap memory. Signed-off-by: Qiang Huang --- libcontainer/cgroups/fs/memory.go | 53 +++++++++++++--- libcontainer/cgroups/fs/memory_test.go | 88 ++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 8 deletions(-) diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 3b8ff21abb3..f2d97ae4ee0 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -65,22 +65,59 @@ func (s *MemoryGroup) SetKernelMemory(path string, cgroup *configs.Cgroup) error return nil } -func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.Memory != 0 { - if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { +func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error { + // When memory and swap memory are both set, we need to handle the cases + // for updating container. + if cgroup.Resources.Memory != 0 && cgroup.Resources.MemorySwap != 0 { + memoryUsage, err := getMemoryData(path, "") + if err != nil { return err } + + // 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 memoryUsage.Limit < uint64(cgroup.Resources.MemorySwap) { + if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { + return err + } + if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { + return err + } + } else { + if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { + return err + } + if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { + return err + } + } + } else { + if cgroup.Resources.Memory != 0 { + if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { + return err + } + } + if cgroup.Resources.MemorySwap > 0 { + if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { + return err + } + } } + + return nil +} + +func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error { + if err := setMemoryAndSwap(path, cgroup); err != nil { + return err + } + if cgroup.Resources.MemoryReservation != 0 { if err := writeFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemoryReservation, 10)); err != nil { return err } } - if cgroup.Resources.MemorySwap > 0 { - if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { - return err - } - } if cgroup.Resources.KernelMemoryTCP != 0 { if err := writeFile(path, "memory.kmem.tcp.limit_in_bytes", strconv.FormatInt(cgroup.Resources.KernelMemoryTCP, 10)); err != nil { return err diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go index 75c9c88a9ee..b79d41d5752 100644 --- a/libcontainer/cgroups/fs/memory_test.go +++ b/libcontainer/cgroups/fs/memory_test.go @@ -86,6 +86,94 @@ func TestMemorySetMemoryswap(t *testing.T) { } } +func TestMemorySetMemoryLargerThanSwap(t *testing.T) { + helper := NewCgroupTestUtil("memory", t) + defer helper.cleanup() + + const ( + memoryBefore = 314572800 // 300M + memoryswapBefore = 524288000 // 500M + memoryAfter = 629145600 // 600M + memoryswapAfter = 838860800 // 800M + ) + + helper.writeFileContents(map[string]string{ + "memory.limit_in_bytes": strconv.Itoa(memoryBefore), + "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), + // Set will call getMemoryData when memory and swap memory are + // both set, fake these fields so we don't get error. + "memory.usage_in_bytes": "0", + "memory.max_usage_in_bytes": "0", + "memory.failcnt": "0", + }) + + helper.CgroupData.config.Resources.Memory = memoryAfter + helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + memory := &MemoryGroup{} + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + t.Fatal(err) + } + + value, err := getCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + if err != nil { + t.Fatalf("Failed to parse memory.limit_in_bytes - %s", err) + } + if value != memoryAfter { + t.Fatal("Got the wrong value, got %d, set memory.limit_in_bytes failed.", value) + } + value, err = getCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + if err != nil { + t.Fatalf("Failed to parse memory.memsw.limit_in_bytes - %s", err) + } + if value != memoryswapAfter { + t.Fatal("Got the wrong value, set memory.memsw.limit_in_bytes failed.") + } +} + +func TestMemorySetSwapSmallerThanMemory(t *testing.T) { + helper := NewCgroupTestUtil("memory", t) + defer helper.cleanup() + + const ( + memoryBefore = 629145600 // 600M + memoryswapBefore = 838860800 // 800M + memoryAfter = 314572800 // 300M + memoryswapAfter = 524288000 // 500M + ) + + helper.writeFileContents(map[string]string{ + "memory.limit_in_bytes": strconv.Itoa(memoryBefore), + "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), + // Set will call getMemoryData when memory and swap memory are + // both set, fake these fields so we don't get error. + "memory.usage_in_bytes": "0", + "memory.max_usage_in_bytes": "0", + "memory.failcnt": "0", + }) + + helper.CgroupData.config.Resources.Memory = memoryAfter + helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + memory := &MemoryGroup{} + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + t.Fatal(err) + } + + value, err := getCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + if err != nil { + t.Fatalf("Failed to parse memory.limit_in_bytes - %s", err) + } + if value != memoryAfter { + t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.") + } + value, err = getCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + if err != nil { + t.Fatalf("Failed to parse memory.memsw.limit_in_bytes - %s", err) + } + if value != memoryswapAfter { + t.Fatal("Got the wrong value, set memory.memsw.limit_in_bytes failed.") + } +} + func TestMemorySetKernelMemory(t *testing.T) { helper := NewCgroupTestUtil("memory", t) defer helper.cleanup()