From 75e38f94a066396f129fc23d84daf3bcbeee3f78 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 14 Jan 2016 01:05:08 +1100 Subject: [PATCH] cgroups: set memory cgroups in Set Modify the memory cgroup code such that kmem is not managed by Set(), in order to allow updating of memory constraints for containers by Docker. This also removes the need to make memory a special case cgroup. Signed-off-by: Aleksa Sarai --- libcontainer/cgroups/fs/apply_raw.go | 6 -- libcontainer/cgroups/fs/memory.go | 20 ++++--- libcontainer/cgroups/fs/memory_test.go | 2 +- libcontainer/cgroups/systemd/apply_systemd.go | 55 ++----------------- 4 files changed, 18 insertions(+), 65 deletions(-) diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index 3d1569b3..4da3b734 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -193,12 +193,6 @@ func (m *Manager) GetStats() (*cgroups.Stats, error) { func (m *Manager) Set(container *configs.Config) error { for _, sys := range subsystems { - // We can't set this here, because after being applied, memcg doesn't - // allow a non-empty cgroup from having its limits changed. - if sys.Name() == "memory" { - continue - } - // Generate fake cgroup data. d, err := getCgroupData(container.Cgroups, -1) if err != nil { diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 0329692a..8c3e963f 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -32,7 +32,9 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) { return err } } - if err := s.Set(path, d.config); err != nil { + // We have to set kernel memory here, as we can't change it once + // processes have been attached. + if err := s.SetKernelMemory(path, d.config); err != nil { return err } } @@ -49,7 +51,17 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) { if err != nil && !cgroups.IsNotFound(err) { return err } + return nil +} +func (s *MemoryGroup) SetKernelMemory(path string, cgroup *configs.Cgroup) error { + // This has to be done separately because it has special constraints (it + // can't be done after there are processes attached to the cgroup). + if cgroup.Resources.KernelMemory > 0 { + if err := writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(cgroup.Resources.KernelMemory, 10)); err != nil { + return err + } + } return nil } @@ -69,12 +81,6 @@ func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error { return err } } - if cgroup.Resources.KernelMemory > 0 { - if err := writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(cgroup.Resources.KernelMemory, 10)); err != nil { - return err - } - } - if cgroup.Resources.OomKillDisable { if err := writeFile(path, "memory.oom_control", "1"); err != nil { return err diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go index bec921f6..9464599b 100644 --- a/libcontainer/cgroups/fs/memory_test.go +++ b/libcontainer/cgroups/fs/memory_test.go @@ -100,7 +100,7 @@ func TestMemorySetKernelMemory(t *testing.T) { helper.CgroupData.config.Resources.KernelMemory = kernelMemoryAfter memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.SetKernelMemory(helper.CgroupPath, helper.CgroupData.config); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/systemd/apply_systemd.go b/libcontainer/cgroups/systemd/apply_systemd.go index 0e399773..5b070dd0 100644 --- a/libcontainer/cgroups/systemd/apply_systemd.go +++ b/libcontainer/cgroups/systemd/apply_systemd.go @@ -460,12 +460,6 @@ func (m *Manager) GetStats() (*cgroups.Stats, error) { func (m *Manager) Set(container *configs.Config) error { for _, sys := range subsystems { - // We can't set this here, because after being applied, memcg doesn't - // allow a non-empty cgroup from having its limits changed. - if sys.Name() == "memory" { - continue - } - // Get the subsystem path, but don't error out for not found cgroups. path, err := getSubsystemPath(container.Cgroups, sys.Name()) if err != nil && !cgroups.IsNotFound(err) { @@ -520,57 +514,16 @@ func setKernelMemory(c *configs.Cgroup) error { return err } - // Shamelessly copied from fs.(*MemoryGroup).Set(). This doesn't get called - // by manager.Set, so we need to do it here. - if c.Resources.KernelMemory > 0 { - if err := writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(c.Resources.KernelMemory, 10)); err != nil { - return err - } - } - - return nil + // This doesn't get called by manager.Set, so we need to do it here. + s := &fs.MemoryGroup{} + return s.SetKernelMemory(path, c) } func joinMemory(c *configs.Cgroup, pid int) error { - path, err := join(c, "memory", pid) + _, err := join(c, "memory", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - - // Shamelessly copied from fs.(*MemoryGroup).Set(). This doesn't include the - // kernel memory limit (which is done in setKernelMemory). All of these are - // not set by manager.Set, we have do do it here. - - if c.Resources.Memory != 0 { - if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(c.Resources.Memory, 10)); err != nil { - return err - } - } - if c.Resources.MemoryReservation != 0 { - if err := writeFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(c.Resources.MemoryReservation, 10)); err != nil { - return err - } - } - if c.Resources.MemorySwap > 0 { - if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(c.Resources.MemorySwap, 10)); err != nil { - return err - } - } - if c.Resources.OomKillDisable { - if err := writeFile(path, "memory.oom_control", "1"); err != nil { - return err - } - } - if c.Resources.MemorySwappiness >= 0 && c.Resources.MemorySwappiness <= 100 { - if err := writeFile(path, "memory.swappiness", strconv.FormatInt(c.Resources.MemorySwappiness, 10)); err != nil { - return err - } - } else if c.Resources.MemorySwappiness == -1 { - return nil - } else { - return fmt.Errorf("invalid value:%d. valid memory swappiness range is 0-100", c.Resources.MemorySwappiness) - } - return nil }