From 685f8db9932e3dbb07e4b03e8c953a48b74cbd4e Mon Sep 17 00:00:00 2001
From: ls-ggg <335814617@qq.com>
Date: Fri, 29 Mar 2024 11:42:22 +0800
Subject: [PATCH] libct/cg/v1:solution to the failure of setting rt_period and
 rt_runtime

As reported in issue #4094, sometimes setting rt_period_us and
rt_runtime_us at the same time will fail. The reason is that in
 cgroupv1, these two values cannot be set atomically.

When we set a new rt_period_us, the kernel will determine
whether the current configuration of new_limit1 = old_quota/new_period
exceeds the limit. If it exceeds the limit, an error will be reported.
Maybe it is reasonable to set rt_runtime_us first so that the
new_limit2 = new_quota/old_period.
for example:
The original state of cgv1 is rt_period_us: 10000 rt_runtime_us: 5000
The user wants to change it to rt_period_us: 1000 rt_runtime_us:300.
The new rt_runtime_us should be set first.

In the opposite case, if rt_runtime_us is set first, new_limit2 may still
exceed the limit, but new_limit1 will be valid.
for example:
The original state of cgv1 is rt_period_us: 1000 rt_runtime_us: 500
The user wants to change it to rt_period_us: 10000 rt_runtime_us:3000.
The new rt_period_us should be set first.

Therefore, new_limit1 and new_limit2 should be calculated in advance,
and the smaller corresponding setting order should be selected to set
rt_period_us and rt_runtime_us.

Signed-off-by: ls-ggg <335814617@qq.com>
---
 libcontainer/cgroups/fs/cpu.go | 63 ++++++++++++++++++++++++++++++----
 tests/integration/update.bats  |  9 +++++
 2 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go
index fbbb8f34eef..5160079f148 100644
--- a/libcontainer/cgroups/fs/cpu.go
+++ b/libcontainer/cgroups/fs/cpu.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"strconv"
+	"strings"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
@@ -35,17 +36,65 @@ func (s *CpuGroup) Apply(path string, r *configs.Resources, pid int) error {
 }
 
 func (s *CpuGroup) SetRtSched(path string, r *configs.Resources) error {
-	if r.CpuRtPeriod != 0 {
-		if err := cgroups.WriteFile(path, "cpu.rt_period_us", strconv.FormatUint(r.CpuRtPeriod, 10)); err != nil {
-			return err
-		}
+	const (
+		filePeriod  = "cpu.rt_period_us"
+		fileRuntime = "cpu.rt_runtime_us"
+	)
+
+	if r.CpuRtPeriod == 0 && r.CpuRtRuntime == 0 {
+		return nil
+	}
+
+	if r.CpuRtPeriod != 0 && r.CpuRtRuntime == 0 {
+		return cgroups.WriteFile(path, filePeriod, strconv.FormatUint(r.CpuRtPeriod, 10))
+	} else if r.CpuRtRuntime != 0 && r.CpuRtPeriod == 0 {
+		return cgroups.WriteFile(path, fileRuntime, strconv.FormatInt(r.CpuRtRuntime, 10))
+	}
+
+	// When neither CpuRtPeriod nor CpuRtRuntime is equal to 0, let's set them in the correct order.
+
+	sOldPeriod, err := cgroups.ReadFile(path, filePeriod)
+	if err != nil {
+		return err
+	}
+	oldPeriod, err := strconv.ParseInt(strings.TrimSpace(sOldPeriod), 10, 64)
+	if err != nil {
+		return err
 	}
-	if r.CpuRtRuntime != 0 {
-		if err := cgroups.WriteFile(path, "cpu.rt_runtime_us", strconv.FormatInt(r.CpuRtRuntime, 10)); err != nil {
+
+	sOldRuntime, err := cgroups.ReadFile(path, fileRuntime)
+	if err != nil {
+		return err
+	}
+	oldRuntime, err := strconv.ParseInt(strings.TrimSpace(sOldRuntime), 10, 64)
+	if err != nil {
+		return err
+	}
+
+	/*
+		When we set a new rt_period_us, the kernel will determine whether
+		the current configuration of new_limit1 = old_quota/new_period
+		exceeds the limit. If it exceeds the limit, an error will be reported.
+		Maybe it is reasonable to set rt_runtime_us first so that the
+		new_limit2 = new_quota/old_period.In the opposite case, if rt_runtime_us
+		is set first, new_limit2 may still exceed the limit, but new_limit1
+		will be valid.Therefore, new_limit1 and new_limit2 should be calculated
+		in advance,and the smaller corresponding setting order should be selected
+		to set rt_period_us and rt_runtime_us.
+	*/
+
+	// First set cpu.rt_period_us because of (oldRuntime / r.CpuRtPeriod) < (r.CpuRtRuntime / oldPeriod)
+	if uint64(oldRuntime*oldPeriod) < (uint64(r.CpuRtRuntime) * r.CpuRtPeriod) {
+		if err := cgroups.WriteFile(path, filePeriod, strconv.FormatUint(r.CpuRtPeriod, 10)); err != nil {
 			return err
 		}
+		return cgroups.WriteFile(path, fileRuntime, strconv.FormatInt(r.CpuRtRuntime, 10))
 	}
-	return nil
+	// First set cpu.rt_runtime_us because of (r.CpuRtRuntime / oldPeriod) < (oldRuntime / r.CpuRtPeriod)
+	if err := cgroups.WriteFile(path, fileRuntime, strconv.FormatInt(r.CpuRtRuntime, 10)); err != nil {
+		return err
+	}
+	return cgroups.WriteFile(path, filePeriod, strconv.FormatUint(r.CpuRtPeriod, 10))
 }
 
 func (s *CpuGroup) Set(path string, r *configs.Resources) error {
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index bc737d47f3b..a15ef5e2495 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -766,6 +766,15 @@ EOF
 
 	check_cgroup_value "cpu.rt_period_us" 900001
 	check_cgroup_value "cpu.rt_runtime_us" 600001
+	runc update test_update_rt --cpu-rt-period 10000 --cpu-rt-runtime 3000
+	[ "$status" -eq 0 ]
+	check_cgroup_value "cpu.rt_period_us" 10000
+	check_cgroup_value "cpu.rt_runtime_us" 3000
+
+	runc update test_update_rt --cpu-rt-period 100000 --cpu-rt-runtime 20000
+	[ "$status" -eq 0 ]
+	check_cgroup_value "cpu.rt_period_us" 100000
+	check_cgroup_value "cpu.rt_runtime_us" 20000
 }
 
 @test "update devices [minimal transition rules]" {