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