From 83f251933fbb139165d392e3b232ecfa8ed672cc Mon Sep 17 00:00:00 2001 From: Hironori Shiina Date: Sat, 8 Mar 2025 14:08:33 +0000 Subject: [PATCH] systemd: Write rounded CPU quota to cgroupfs When CPU quota is updated, the value is converted to CPUQuotaPerSecUSec property for passing to systemd. The value can be rounded in the following cases: - The value is rounded up to the nearest 10ms. - Depending on CPU period, the value may be rounded during division. Because of this rounding, systemd and systemd driver may write different values to cgroupfs. In order to avoid this inconsistency, this fix makes systemd driver write the same rounded value to cgroupfs. Even if systemd writes CPU quota and CPU period to cgroupfs, systemd driver still needs to write to cgroupfs for a case where CPU period is updated to less than the minimum value. In this case, systemd accepts this value by adjusting CPU period while direct update by systemd driver fails. In order to keep this case invalid, systemd driver still needs to update cgroupfs. Signed-off-by: Hironori Shiina --- systemd/common.go | 12 ++++++--- systemd/systemd_test.go | 58 +++++++++++++++++++++++++++++++++++++++++ systemd/v1.go | 5 +++- systemd/v2.go | 7 +++-- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/systemd/common.go b/systemd/common.go index b3077bd..d95c113 100644 --- a/systemd/common.go +++ b/systemd/common.go @@ -280,7 +280,9 @@ func systemdVersionAtoi(str string) (int, error) { return ver, nil } -func addCpuQuota(cm *dbusConnManager, properties *[]systemdDbus.Property, quota int64, period uint64) { +// addCPUQuota adds CPUQuotaPeriodUSec and CPUQuotaPerSecUSec to the properties. The passed quota may be modified +// along with round-up during calculation in order to write the same value to cgroupfs later. +func addCPUQuota(cm *dbusConnManager, properties *[]systemdDbus.Property, quota *int64, period uint64) { if period != 0 { // systemd only supports CPUQuotaPeriodUSec since v242 sdVer := systemdVersion(cm) @@ -292,10 +294,10 @@ func addCpuQuota(cm *dbusConnManager, properties *[]systemdDbus.Property, quota " (setting will still be applied to cgroupfs)", sdVer) } } - if quota != 0 || period != 0 { + if *quota != 0 || period != 0 { // corresponds to USEC_INFINITY in systemd cpuQuotaPerSecUSec := uint64(math.MaxUint64) - if quota > 0 { + if *quota > 0 { if period == 0 { // assume the default period = defCPUQuotaPeriod @@ -304,9 +306,11 @@ func addCpuQuota(cm *dbusConnManager, properties *[]systemdDbus.Property, quota // (integer percentage of CPU) internally. This means that if a fractional percent of // CPU is indicated by Resources.CpuQuota, we need to round up to the nearest // 10ms (1% of a second) such that child cgroups can set the cpu.cfs_quota_us they expect. - cpuQuotaPerSecUSec = uint64(quota*1000000) / period + cpuQuotaPerSecUSec = uint64(*quota*1000000) / period if cpuQuotaPerSecUSec%10000 != 0 { cpuQuotaPerSecUSec = ((cpuQuotaPerSecUSec / 10000) + 1) * 10000 + // Update the requested quota along with the round-up in order to write the same value to cgroupfs. + *quota = int64(cpuQuotaPerSecUSec) * int64(period) / 1000000 } } *properties = append(*properties, diff --git a/systemd/systemd_test.go b/systemd/systemd_test.go index dae851c..77f637c 100644 --- a/systemd/systemd_test.go +++ b/systemd/systemd_test.go @@ -178,3 +178,61 @@ func TestUnifiedResToSystemdProps(t *testing.T) { }) } } + +func TestAddCPUQuota(t *testing.T) { + if !IsRunningSystemd() { + t.Skip("Test requires systemd.") + } + + cm := newDbusConnManager(os.Geteuid() != 0) + + testCases := []struct { + name string + quota int64 + period uint64 + expectedCPUQuotaPerSecUSec uint64 + expectedQuota int64 + }{ + { + name: "No round up", + quota: 500000, + period: 1000000, + expectedCPUQuotaPerSecUSec: 500000, + expectedQuota: 500000, + }, + { + name: "With fraction", + quota: 123456, + expectedCPUQuotaPerSecUSec: 1240000, + expectedQuota: 124000, + }, + { + name: "Round up at division", + quota: 500000, + period: 900000, + expectedCPUQuotaPerSecUSec: 560000, + expectedQuota: 504000, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + props := []systemdDbus.Property{} + addCPUQuota(cm, &props, &tc.quota, tc.period) + var cpuQuotaPerSecUSec uint64 + for _, p := range props { + if p.Name == "CPUQuotaPerSecUSec" { + if err := p.Value.Store(&cpuQuotaPerSecUSec); err != nil { + t.Errorf("failed to parse CPUQuotaPerSecUSec: %v", err) + } + } + } + if cpuQuotaPerSecUSec != tc.expectedCPUQuotaPerSecUSec { + t.Errorf("CPUQuotaPerSecUSec is not set as expected (exp: %v, got: %v)", tc.expectedCPUQuotaPerSecUSec, cpuQuotaPerSecUSec) + } + if tc.quota != tc.expectedQuota { + t.Errorf("quota is not updated as expected (exp: %v, got: %v)", tc.expectedQuota, tc.quota) + } + }) + } +} diff --git a/systemd/v1.go b/systemd/v1.go index 8453e9b..b8959ad 100644 --- a/systemd/v1.go +++ b/systemd/v1.go @@ -90,7 +90,7 @@ func genV1ResourcesProperties(r *cgroups.Resources, cm *dbusConnManager) ([]syst newProp("CPUShares", r.CpuShares)) } - addCpuQuota(cm, &properties, r.CpuQuota, r.CpuPeriod) + addCPUQuota(cm, &properties, &r.CpuQuota, r.CpuPeriod) if r.BlkioWeight != 0 { properties = append(properties, @@ -334,6 +334,9 @@ func (m *LegacyManager) Set(r *cgroups.Resources) error { if r.Unified != nil { return cgroups.ErrV1NoUnified } + // Use a copy since CpuQuota in r may be modified. + rCopy := *r + r = &rCopy properties, err := genV1ResourcesProperties(r, m.dbus) if err != nil { return err diff --git a/systemd/v2.go b/systemd/v2.go index 42a6e35..636b9cb 100644 --- a/systemd/v2.go +++ b/systemd/v2.go @@ -113,7 +113,7 @@ func unifiedResToSystemdProps(cm *dbusConnManager, res map[string]string) (props return nil, fmt.Errorf("unified resource %q quota value conversion error: %w", k, err) } } - addCpuQuota(cm, &props, quota, period) + addCPUQuota(cm, &props, "a, period) case "cpu.weight": if shouldSetCPUIdle(cm, strings.TrimSpace(res["cpu.idle"])) { @@ -254,7 +254,7 @@ func genV2ResourcesProperties(dirPath string, r *cgroups.Resources, cm *dbusConn } } - addCpuQuota(cm, &properties, r.CpuQuota, r.CpuPeriod) + addCPUQuota(cm, &properties, &r.CpuQuota, r.CpuPeriod) if r.PidsLimit > 0 || r.PidsLimit == -1 { properties = append(properties, @@ -480,6 +480,9 @@ func (m *UnifiedManager) Set(r *cgroups.Resources) error { if r == nil { return nil } + // Use a copy since CpuQuota in r may be modified. + rCopy := *r + r = &rCopy properties, err := genV2ResourcesProperties(m.fsMgr.Path(""), r, m.dbus) if err != nil { return err