Merge pull request #2426 from kolyshkin/mem-swap-unlim

Fix some cases of swap setting

Author: AkihiroSuda

libcontainer/cgroups/fs/memory.go

@@ -74,9 +74,9 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
 }
 
 func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {
-	// If the memory update is set to -1 we should also
-	// set swap to -1, it means unlimited memory.
-	if cgroup.Resources.Memory == -1 {
+	// If the memory update is set to -1 and the swap is not explicitly
+	// set, we should also set swap to -1, it means unlimited memory.
+	if cgroup.Resources.Memory == -1 && cgroup.Resources.MemorySwap == 0 {
 		// Only set swap if it's enabled in kernel
 		if cgroups.PathExists(filepath.Join(path, cgroupMemorySwapLimit)) {
 			cgroup.Resources.MemorySwap = -1

libcontainer/cgroups/systemd/v2.go

@@ -57,12 +57,12 @@ func genV2ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error)
 		properties = append(properties,
 			newProp("MemoryLow", uint64(c.Resources.MemoryReservation)))
 	}
-	// swap is set
-	if c.Resources.MemorySwap != 0 {
-		swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(c.Resources.MemorySwap, c.Resources.Memory)
-		if err != nil {
-			return nil, err
-		}
+
+	swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(c.Resources.MemorySwap, c.Resources.Memory)
+	if err != nil {
+		return nil, err
+	}
+	if swap != 0 {
 		properties = append(properties,
 			newProp("MemorySwapMax", uint64(swap)))
 	}

libcontainer/cgroups/utils.go

@@ -612,6 +612,12 @@ func ConvertCPUQuotaCPUPeriodToCgroupV2Value(quota int64, period uint64) string
 // 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) {
+	// for compatibility with cgroup1 controller, set swap to unlimited in
+	// case the memory is set to unlimited, and swap is not explicitly set,
+	// treating the request as "set both memory and swap to unlimited".
+	if memory == -1 && memorySwap == 0 {
+		return -1, nil
+	}
 	if memorySwap == -1 || memorySwap == 0 {
 		// -1 is "max", 0 is "unset", so treat as is
 		return memorySwap, nil

tests/integration/update.bats

@@ -80,8 +80,7 @@ EOF
         SD_MEM_SWAP="MemorySwapMax"
         SYSTEM_MEM="max"
         SYSTEM_MEM_SWAP="max"
-        # checking swap is currently disabled for v2
-        #CGROUP_MEMORY=$CGROUP_PATH
+        CGROUP_MEMORY=$CGROUP_PATH
         ;;
     esac
     SD_UNLIMITED="infinity"
@@ -135,10 +134,19 @@ EOF
         check_systemd_value "$SD_MEM_SWAP" $SD_UNLIMITED
 
         # update memory swap
-        runc update test_update --memory-swap 96468992
-        [ "$status" -eq 0 ]
-        check_cgroup_value "$MEM_SWAP" 96468992
-        check_systemd_value "$SD_MEM_SWAP" 96468992
+        if [ "$CGROUP_UNIFIED" = "yes" ]; then
+            # for cgroupv2, memory and swap can only be set together
+            runc update test_update --memory 52428800 --memory-swap 96468992
+            [ "$status" -eq 0 ]
+            # for cgroupv2, swap is a separate limit (it does not include mem)
+            check_cgroup_value "$MEM_SWAP" $((96468992-52428800))
+            check_systemd_value "$SD_MEM_SWAP" $((96468992-52428800))
+        else
+            runc update test_update --memory-swap 96468992
+            [ "$status" -eq 0 ]
+            check_cgroup_value "$MEM_SWAP" 96468992
+            check_systemd_value "$SD_MEM_SWAP" 96468992
+        fi
     fi
 
     # try to remove memory limit