Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgroupv2: fix setting MemorySwap #2288

Merged
merged 1 commit into from
Apr 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion libcontainer/cgroups/fs2/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,16 @@ func numToStr(value int64) (ret string) {
}

func setMemory(dirPath string, cgroup *configs.Cgroup) error {
if val := numToStr(cgroup.Resources.MemorySwap); val != "" {
swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(cgroup.Resources.MemorySwap, cgroup.Resources.Memory)
if err != nil {
return err
}
if val := numToStr(swap); val != "" {
if err := fscommon.WriteFile(dirPath, "memory.swap.max", val); err != nil {
return err
}
}

if val := numToStr(cgroup.Resources.Memory); val != "" {
if err := fscommon.WriteFile(dirPath, "memory.max", val); err != nil {
return err
Expand Down
9 changes: 9 additions & 0 deletions libcontainer/cgroups/systemd/unified_hierarchy.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ func (m *UnifiedManager) Apply(pid int) error {
newProp("MemoryMax", uint64(c.Resources.Memory)))
}

swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(c.Resources.MemorySwap, c.Resources.Memory)
if err != nil {
return err
}
if swap > 0 {
properties = append(properties,
newProp("MemorySwapMax", uint64(swap)))
}

if c.Resources.CpuWeight != 0 {
properties = append(properties,
newProp("CPUWeight", c.Resources.CpuWeight))
Expand Down
22 changes: 22 additions & 0 deletions libcontainer/cgroups/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,25 @@ func ConvertCPUQuotaCPUPeriodToCgroupV2Value(quota int64, period uint64) string
}
return fmt.Sprintf("%d %d", quota, period)
}

// ConvertMemorySwapToCgroupV2Value converts MemorySwap value from OCI spec
// for use by cgroup v2 drivers. A conversion is needed since Resources.MemorySwap
// is defined as memory+swap combined, while in cgroup v2 swap is a separate value.
func ConvertMemorySwapToCgroupV2Value(memorySwap, memory int64) (int64, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add UT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if memorySwap == -1 || memorySwap == 0 {
// -1 is "max", 0 is "unset", so treat as is
return memorySwap, nil
}
// sanity checks
if memory == 0 || memory == -1 {
return 0, errors.New("unable to set swap limit without memory limit")
}
if memory < 0 {
return 0, fmt.Errorf("invalid memory value: %d", memory)
}
if memorySwap < memory {
return 0, errors.New("memory+swap limit should be > memory limit")
}

return memorySwap - memory, nil
}
81 changes: 81 additions & 0 deletions libcontainer/cgroups/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,3 +530,84 @@ func TestConvertCPUQuotaCPUPeriodToCgroupV2Value(t *testing.T) {
}
}
}

func TestConvertMemorySwapToCgroupV2Value(t *testing.T) {
cases := []struct {
memswap, memory int64
expected int64
expErr bool
}{
{
memswap: 0,
memory: 0,
expected: 0,
},
{
memswap: -1,
memory: 0,
expected: -1,
},
{
memswap: -1,
memory: -1,
expected: -1,
},
{
memswap: -2,
memory: 0,
expErr: true,
},
{
memswap: -1,
memory: 1000,
expected: -1,
},
{
memswap: 1000,
memory: 1000,
expected: 0,
},
{
memswap: 500,
memory: 200,
expected: 300,
},
{
memswap: 300,
memory: 400,
expErr: true,
},
{
memswap: 300,
memory: 0,
expErr: true,
},
{
memswap: 300,
memory: -300,
expErr: true,
},
{
memswap: 300,
memory: -1,
expErr: true,
},
}

for _, c := range cases {
swap, err := ConvertMemorySwapToCgroupV2Value(c.memswap, c.memory)
if c.expErr {
if err == nil {
t.Errorf("memswap: %d, memory %d, expected error, got %d, nil", c.memswap, c.memory, swap)
}
// no more checks
continue
}
if err != nil {
t.Errorf("memswap: %d, memory %d, expected success, got error %s", c.memswap, c.memory, err)
}
if swap != c.expected {
t.Errorf("memswap: %d, memory %d, expected %d, got %d", c.memswap, c.memory, c.expected, swap)
}
}
}