Skip to content

Commit

Permalink
Handle memory swappiness as a pointer to handle default/unset case
Browse files Browse the repository at this point in the history
This prior fix to set "-1" explicitly was lost, and it is simpler to use
the same pointer type from the OCI spec to handle nil pointer == -1 ==
unset case.

Also, as a nearly humorous aside, there was a test for MemorySwappiness
that was actually setting Memory, and it was passing because of this
bug (as it was always setting everyone's MemorySwappiness to zero!)

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
  • Loading branch information
estesp committed Feb 24, 2016
1 parent ee6a72d commit 94ece21
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 13 deletions.
12 changes: 6 additions & 6 deletions libcontainer/cgroups/fs/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,14 @@ func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error {
return err
}
}
if cgroup.Resources.MemorySwappiness >= 0 && cgroup.Resources.MemorySwappiness <= 100 {
if err := writeFile(path, "memory.swappiness", strconv.FormatInt(cgroup.Resources.MemorySwappiness, 10)); err != nil {
if cgroup.Resources.MemorySwappiness == nil || int64(*cgroup.Resources.MemorySwappiness) == -1 {
return nil
} else if int64(*cgroup.Resources.MemorySwappiness) >= 0 && int64(*cgroup.Resources.MemorySwappiness) <= 100 {
if err := writeFile(path, "memory.swappiness", strconv.FormatInt(*cgroup.Resources.MemorySwappiness, 10)); err != nil {
return err
}
} else if cgroup.Resources.MemorySwappiness == -1 {
return nil
} else {
return fmt.Errorf("invalid value:%d. valid memory swappiness range is 0-100", cgroup.Resources.MemorySwappiness)
return fmt.Errorf("invalid value:%d. valid memory swappiness range is 0-100", int64(*cgroup.Resources.MemorySwappiness))
}

return nil
Expand Down Expand Up @@ -149,7 +149,7 @@ func memoryAssigned(cgroup *configs.Cgroup) bool {
cgroup.Resources.MemorySwap > 0 ||
cgroup.Resources.KernelMemory > 0 ||
cgroup.Resources.OomKillDisable ||
cgroup.Resources.MemorySwappiness != -1
(cgroup.Resources.MemorySwappiness != nil && *cgroup.Resources.MemorySwappiness != -1)
}

func getMemoryData(path, name string) (cgroups.MemoryData, error) {
Expand Down
8 changes: 4 additions & 4 deletions libcontainer/cgroups/fs/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,15 @@ func TestMemorySetMemorySwappinessDefault(t *testing.T) {
defer helper.cleanup()

const (
swappinessBefore = 60 //deafult is 60
swappinessAfter = 0
swappinessBefore = 60 //default is 60
swappinessAfter = int64(0)
)

helper.writeFileContents(map[string]string{
"memory.swappiness": strconv.Itoa(swappinessBefore),
})

helper.CgroupData.config.Resources.Memory = swappinessAfter
helper.CgroupData.config.Resources.MemorySwappiness = &swappinessAfter
memory := &MemoryGroup{}
if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
t.Fatal(err)
Expand All @@ -138,7 +138,7 @@ func TestMemorySetMemorySwappinessDefault(t *testing.T) {
t.Fatalf("Failed to parse memory.swappiness - %s", err)
}
if value != swappinessAfter {
t.Fatal("Got the wrong value, set memory.swappiness failed.")
t.Fatalf("Got the wrong value (%d), set memory.swappiness = %d failed.", value, swappinessAfter)
}
}

Expand Down
2 changes: 1 addition & 1 deletion libcontainer/configs/cgroup_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ type Resources struct {
OomKillDisable bool `json:"oom_kill_disable"`

// Tuning swappiness behaviour per cgroup
MemorySwappiness int64 `json:"memory_swappiness"`
MemorySwappiness *int64 `json:"memory_swappiness"`

// Set priority of network traffic for container
NetPrioIfpriomap []*IfPrioMap `json:"net_prio_ifpriomap"`
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/integration/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func newTemplateConfig(rootfs string) *configs.Config {
Cgroups: &configs.Cgroup{
Path: "integration/test",
Resources: &configs.Resources{
MemorySwappiness: -1,
MemorySwappiness: nil,
AllowAllDevices: false,
AllowedDevices: configs.DefaultAllowedDevices,
},
Expand Down
3 changes: 2 additions & 1 deletion spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,8 @@ func createCgroupConfig(name string, spec *specs.LinuxSpec) (*configs.Cgroup, er
c.Resources.KernelMemory = int64(*r.Memory.Kernel)
}
if r.Memory.Swappiness != nil {
c.Resources.MemorySwappiness = int64(*r.Memory.Swappiness)
swappiness := int64(*r.Memory.Swappiness)
c.Resources.MemorySwappiness = &swappiness
}
}
if r.CPU != nil {
Expand Down

0 comments on commit 94ece21

Please sign in to comment.