Skip to content

Commit 18ebc51

Browse files
boynuxboynux
committed
Reset Swap when memory is set to unlimited (-1)
Kernel validation fails if memory set to -1 which is unlimited but swap is not set so. Signed-off-by: Mohammad Arab <[email protected]>
1 parent 1e4ca86 commit 18ebc51

4 files changed

Lines changed: 60 additions & 59 deletions

File tree

‎libcontainer/cgroups/fs/memory.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ import (
1616
"github.com/opencontainers/runc/libcontainer/configs"
1717
)
1818

19-
const cgroupKernelMemoryLimit = "memory.kmem.limit_in_bytes"
19+
const (
20+
cgroupKernelMemoryLimit = "memory.kmem.limit_in_bytes"
21+
cgroupMemorySwapLimit = "memory.memsw.limit_in_bytes"
22+
cgroupMemoryLimit = "memory.limit_in_bytes"
23+
)
2024

2125
type MemoryGroup struct {
2226
}
@@ -96,9 +100,18 @@ func setKernelMemory(path string, kernelMemoryLimit int64) error {
96100
}
97101

98102
func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {
103+
// If the memory update is set to -1 we should also set swap to -1
104+
// -1 means unlimited memory
105+
if cgroup.Resources.Memory == -1 {
106+
// Only set swap if it's enbled in kernel
107+
if cgroups.PathExists(filepath.Join(path, cgroupMemorySwapLimit)) {
108+
cgroup.Resources.MemorySwap = -1
109+
}
110+
}
111+
99112
// When memory and swap memory are both set, we need to handle the cases
100113
// for updating container.
101-
if cgroup.Resources.Memory != 0 && cgroup.Resources.MemorySwap > 0 {
114+
if cgroup.Resources.Memory != 0 && cgroup.Resources.MemorySwap != 0 {
102115
memoryUsage, err := getMemoryData(path, "")
103116
if err != nil {
104117
return err
@@ -107,29 +120,29 @@ func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {
107120
// When update memory limit, we should adapt the write sequence
108121
// for memory and swap memory, so it won't fail because the new
109122
// value and the old value don't fit kernel's validation.
110-
if memoryUsage.Limit < uint64(cgroup.Resources.MemorySwap) {
111-
if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
123+
if cgroup.Resources.MemorySwap == -1 || memoryUsage.Limit < uint64(cgroup.Resources.MemorySwap) {
124+
if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
112125
return err
113126
}
114-
if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
127+
if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
115128
return err
116129
}
117130
} else {
118-
if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
131+
if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
119132
return err
120133
}
121-
if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
134+
if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
122135
return err
123136
}
124137
}
125138
} else {
126139
if cgroup.Resources.Memory != 0 {
127-
if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
140+
if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
128141
return err
129142
}
130143
}
131-
if cgroup.Resources.MemorySwap > 0 {
132-
if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
144+
if cgroup.Resources.MemorySwap != 0 {
145+
if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
133146
return err
134147
}
135148
}

‎libcontainer/cgroups/fs/memory_test.go‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -86,47 +86,6 @@ func TestMemorySetMemoryswap(t *testing.T) {
8686
}
8787
}
8888

89-
func TestMemorySetNegativeMemoryswap(t *testing.T) {
90-
helper := NewCgroupTestUtil("memory", t)
91-
defer helper.cleanup()
92-
93-
const (
94-
memoryBefore = 314572800 // 300M
95-
memoryAfter = 524288000 // 500M
96-
memoryswapBefore = 629145600 // 600M
97-
memoryswapAfter = 629145600 // 600M
98-
)
99-
100-
helper.writeFileContents(map[string]string{
101-
"memory.limit_in_bytes": strconv.Itoa(memoryBefore),
102-
"memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore),
103-
})
104-
105-
helper.CgroupData.config.Resources.Memory = memoryAfter
106-
// Negative value means not change
107-
helper.CgroupData.config.Resources.MemorySwap = -1
108-
memory := &MemoryGroup{}
109-
if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
110-
t.Fatal(err)
111-
}
112-
113-
value, err := getCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes")
114-
if err != nil {
115-
t.Fatalf("Failed to parse memory.limit_in_bytes - %s", err)
116-
}
117-
if value != memoryAfter {
118-
t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.")
119-
}
120-
121-
value, err = getCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes")
122-
if err != nil {
123-
t.Fatalf("Failed to parse memory.memsw.limit_in_bytes - %s", err)
124-
}
125-
if value != memoryswapAfter {
126-
t.Fatal("Got the wrong value, set memory.memsw.limit_in_bytes failed.")
127-
}
128-
}
129-
13089
func TestMemorySetMemoryLargerThanSwap(t *testing.T) {
13190
helper := NewCgroupTestUtil("memory", t)
13291
defer helper.cleanup()

‎tests/integration/update.bats‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function check_cgroup_value() {
4545
expected=$3
4646

4747
current=$(cat $cgroup/$source)
48-
[ "$current" -eq "$expected" ]
48+
[ "$current" == "$expected" ]
4949
}
5050

5151
# TODO: test rt cgroup updating
@@ -62,6 +62,8 @@ function check_cgroup_value() {
6262
eval CGROUP_${g}="${base_path}/runc-update-integration-test"
6363
done
6464

65+
CGROUP_SYSTEM_MEMORY=$(grep "cgroup" /proc/self/mountinfo | gawk 'toupper($NF) ~ /\<'MEMORY'\>/ { print $5; exit }')
66+
6567
# check that initial values were properly set
6668
check_cgroup_value $CGROUP_BLKIO "blkio.weight" 1000
6769
check_cgroup_value $CGROUP_CPU "cpu.cfs_period_us" 1000000
@@ -110,17 +112,38 @@ function check_cgroup_value() {
110112
[ "$status" -eq 0 ]
111113
check_cgroup_value $CGROUP_MEMORY "memory.limit_in_bytes" 52428800
112114

113-
114115
# update memory soft limit
115116
runc update test_update --memory-reservation 33554432
116117
[ "$status" -eq 0 ]
117118
check_cgroup_value $CGROUP_MEMORY "memory.soft_limit_in_bytes" 33554432
118119

119-
# update memory swap (if available)
120+
# Run swap memory tests if swap is avaialble
120121
if [ -f "$CGROUP_MEMORY/memory.memsw.limit_in_bytes" ]; then
122+
# try to remove memory swap limit
123+
runc update test_update --memory-swap -1
124+
[ "$status" -eq 0 ]
125+
# Get System memory swap limit
126+
SYSTEM_MEMORY_SW=$(cat "${CGROUP_SYSTEM_MEMORY}/memory.memsw.limit_in_bytes")
127+
check_cgroup_value $CGROUP_MEMORY "memory.memsw.limit_in_bytes" ${SYSTEM_MEMORY_SW}
128+
129+
# update memory swap
121130
runc update test_update --memory-swap 96468992
122131
[ "$status" -eq 0 ]
123132
check_cgroup_value $CGROUP_MEMORY "memory.memsw.limit_in_bytes" 96468992
133+
fi;
134+
135+
# try to remove memory limit
136+
runc update test_update --memory -1
137+
[ "$status" -eq 0 ]
138+
139+
# Get System memory limit
140+
SYSTEM_MEMORY=$(cat "${CGROUP_SYSTEM_MEMORY}/memory.limit_in_bytes")
141+
# check memory limited is gone
142+
check_cgroup_value $CGROUP_MEMORY "memory.limit_in_bytes" ${SYSTEM_MEMORY}
143+
144+
# check swap memory limited is gone
145+
if [ -f "$CGROUP_MEMORY/memory.memsw.limit_in_bytes" ]; then
146+
check_cgroup_value $CGROUP_MEMORY "memory.memsw.limit_in_bytes" ${SYSTEM_MEMORY}
124147
fi
125148

126149
# update kernel memory limit

‎update.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,16 +193,22 @@ other options are ignored.
193193
opt string
194194
dest *uint64
195195
}{
196+
{"memory", r.Memory.Limit},
197+
{"memory-swap", r.Memory.Swap},
196198
{"kernel-memory", r.Memory.Kernel},
197199
{"kernel-memory-tcp", r.Memory.KernelTCP},
198-
{"memory", r.Memory.Limit},
199200
{"memory-reservation", r.Memory.Reservation},
200-
{"memory-swap", r.Memory.Swap},
201201
} {
202202
if val := context.String(pair.opt); val != "" {
203-
v, err := units.RAMInBytes(val)
204-
if err != nil {
205-
return fmt.Errorf("invalid value for %s: %s", pair.opt, err)
203+
var v int64
204+
205+
if val != "-1" {
206+
v, err = units.RAMInBytes(val)
207+
if err != nil {
208+
return fmt.Errorf("invalid value for %s: %s", pair.opt, err)
209+
}
210+
} else {
211+
v = -1
206212
}
207213
*pair.dest = uint64(v)
208214
}

0 commit comments

Comments
 (0)