From afbeaf6f292cd34f686454967daa3d72e68f2c08 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 22 May 2020 14:18:06 -0700 Subject: [PATCH 1/2] pkg/sysinfo: rm duplicates The CPU CFS cgroup-aware scheduler is one single kernel feature, not two, so it does not make sense to have two separate booleans (CPUCfsQuota and CPUCfsPeriod). Merge these into CPUCfs. Same for CPU realtime. For compatibility reasons, /info stays the same for now. Signed-off-by: Kir Kolyshkin --- daemon/daemon_unix.go | 17 +++++------ daemon/info_unix.go | 4 +-- pkg/sysinfo/cgroup2_linux.go | 6 ++-- pkg/sysinfo/sysinfo.go | 14 +++------ pkg/sysinfo/sysinfo_linux.go | 24 +++++---------- runconfig/hostconfig_test.go | 58 ++++++++++++++++-------------------- runconfig/hostconfig_unix.go | 8 ++--- 7 files changed, 50 insertions(+), 81 deletions(-) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 1a577276ef..5bb61d5fe1 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -505,8 +505,8 @@ func verifyPlatformContainerResources(resources *containertypes.Resources, sysIn if resources.NanoCPUs > 0 && resources.CPUQuota > 0 { return warnings, fmt.Errorf("Conflicting options: Nano CPUs and CPU Quota cannot both be set") } - if resources.NanoCPUs > 0 && (!sysInfo.CPUCfsPeriod || !sysInfo.CPUCfsQuota) { - return warnings, fmt.Errorf("NanoCPUs can not be set, as your kernel does not support CPU cfs period/quota or the cgroup is not mounted") + if resources.NanoCPUs > 0 && !sysInfo.CPUCfs { + return warnings, fmt.Errorf("NanoCPUs can not be set, as your kernel does not support CPU CFS scheduler or the cgroup is not mounted") } // The highest precision we could get on Linux is 0.001, by setting // cpu.cfs_period_us=1000ms @@ -523,17 +523,14 @@ func verifyPlatformContainerResources(resources *containertypes.Resources, sysIn warnings = append(warnings, "Your kernel does not support CPU shares or the cgroup is not mounted. Shares discarded.") resources.CPUShares = 0 } - if resources.CPUPeriod > 0 && !sysInfo.CPUCfsPeriod { - warnings = append(warnings, "Your kernel does not support CPU cfs period or the cgroup is not mounted. Period discarded.") + if (resources.CPUPeriod != 0 || resources.CPUQuota != 0) && !sysInfo.CPUCfs { + warnings = append(warnings, "Your kernel does not support CPU CFS scheduler. CPU period/quota discarded.") resources.CPUPeriod = 0 + resources.CPUQuota = 0 } if resources.CPUPeriod != 0 && (resources.CPUPeriod < 1000 || resources.CPUPeriod > 1000000) { return warnings, fmt.Errorf("CPU cfs period can not be less than 1ms (i.e. 1000) or larger than 1s (i.e. 1000000)") } - if resources.CPUQuota > 0 && !sysInfo.CPUCfsQuota { - warnings = append(warnings, "Your kernel does not support CPU cfs quota or the cgroup is not mounted. Quota discarded.") - resources.CPUQuota = 0 - } if resources.CPUQuota > 0 && resources.CPUQuota < 1000 { return warnings, fmt.Errorf("CPU cfs quota can not be less than 1ms (i.e. 1000)") } @@ -1726,10 +1723,10 @@ func (daemon *Daemon) initCgroupsPath(path string) error { path = filepath.Join(mnt, root, path) sysInfo := daemon.RawSysInfo(true) - if err := maybeCreateCPURealTimeFile(sysInfo.CPURealtimePeriod, daemon.configStore.CPURealtimePeriod, "cpu.rt_period_us", path); err != nil { + if err := maybeCreateCPURealTimeFile(sysInfo.CPURealtime, daemon.configStore.CPURealtimePeriod, "cpu.rt_period_us", path); err != nil { return err } - return maybeCreateCPURealTimeFile(sysInfo.CPURealtimeRuntime, daemon.configStore.CPURealtimeRuntime, "cpu.rt_runtime_us", path) + return maybeCreateCPURealTimeFile(sysInfo.CPURealtime, daemon.configStore.CPURealtimeRuntime, "cpu.rt_runtime_us", path) } func maybeCreateCPURealTimeFile(sysinfoPresent bool, configValue int64, file string, path string) error { diff --git a/daemon/info_unix.go b/daemon/info_unix.go index b9d54101b0..ae0a224655 100644 --- a/daemon/info_unix.go +++ b/daemon/info_unix.go @@ -30,8 +30,8 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) v.KernelMemory = sysInfo.KernelMemory v.KernelMemoryTCP = sysInfo.KernelMemoryTCP v.OomKillDisable = sysInfo.OomKillDisable - v.CPUCfsPeriod = sysInfo.CPUCfsPeriod - v.CPUCfsQuota = sysInfo.CPUCfsQuota + v.CPUCfsPeriod = sysInfo.CPUCfs + v.CPUCfsQuota = sysInfo.CPUCfs v.CPUShares = sysInfo.CPUShares v.CPUSet = sysInfo.Cpuset v.PidsLimit = sysInfo.PidsLimit diff --git a/pkg/sysinfo/cgroup2_linux.go b/pkg/sysinfo/cgroup2_linux.go index b82829e9b1..1f9fbf4438 100644 --- a/pkg/sysinfo/cgroup2_linux.go +++ b/pkg/sysinfo/cgroup2_linux.go @@ -90,10 +90,8 @@ func applyCPUCgroupInfoV2(info *SysInfo, controllers map[string]struct{}, _ stri return warnings } info.CPUShares = true - info.CPUCfsPeriod = true - info.CPUCfsQuota = true - info.CPURealtimePeriod = false - info.CPURealtimeRuntime = false + info.CPUCfs = true + info.CPURealtime = false return warnings } diff --git a/pkg/sysinfo/sysinfo.go b/pkg/sysinfo/sysinfo.go index f64801c4da..6e4b16847a 100644 --- a/pkg/sysinfo/sysinfo.go +++ b/pkg/sysinfo/sysinfo.go @@ -62,17 +62,11 @@ type cgroupCPUInfo struct { // Whether CPU shares is supported or not CPUShares bool - // Whether CPU CFS(Completely Fair Scheduler) period is supported or not - CPUCfsPeriod bool + // Whether CPU CFS (Completely Fair Scheduler) is supported + CPUCfs bool - // Whether CPU CFS(Completely Fair Scheduler) quota is supported or not - CPUCfsQuota bool - - // Whether CPU real-time period is supported or not - CPURealtimePeriod bool - - // Whether CPU real-time runtime is supported or not - CPURealtimeRuntime bool + // Whether CPU real-time scheduler is supported + CPURealtime bool } type cgroupBlkioInfo struct { diff --git a/pkg/sysinfo/sysinfo_linux.go b/pkg/sysinfo/sysinfo_linux.go index ec37263598..0237dfcaef 100644 --- a/pkg/sysinfo/sysinfo_linux.go +++ b/pkg/sysinfo/sysinfo_linux.go @@ -144,27 +144,17 @@ func applyCPUCgroupInfo(info *SysInfo, cgMounts map[string]string) []string { info.CPUShares = cgroupEnabled(mountPoint, "cpu.shares") if !info.CPUShares { - warnings = append(warnings, "Your kernel does not support cgroup cpu shares") + warnings = append(warnings, "Your kernel does not support CPU shares") } - info.CPUCfsPeriod = cgroupEnabled(mountPoint, "cpu.cfs_period_us") - if !info.CPUCfsPeriod { - warnings = append(warnings, "Your kernel does not support cgroup cfs period") + info.CPUCfs = cgroupEnabled(mountPoint, "cpu.cfs_quota_us") + if !info.CPUCfs { + warnings = append(warnings, "Your kernel does not support CPU CFS scheduler") } - info.CPUCfsQuota = cgroupEnabled(mountPoint, "cpu.cfs_quota_us") - if !info.CPUCfsQuota { - warnings = append(warnings, "Your kernel does not support cgroup cfs quotas") - } - - info.CPURealtimePeriod = cgroupEnabled(mountPoint, "cpu.rt_period_us") - if !info.CPURealtimePeriod { - warnings = append(warnings, "Your kernel does not support cgroup rt period") - } - - info.CPURealtimeRuntime = cgroupEnabled(mountPoint, "cpu.rt_runtime_us") - if !info.CPURealtimeRuntime { - warnings = append(warnings, "Your kernel does not support cgroup rt runtime") + info.CPURealtime = cgroupEnabled(mountPoint, "cpu.rt_period_us") + if !info.CPURealtime { + warnings = append(warnings, "Your kernel does not support CPU realtime scheduler") } return warnings diff --git a/runconfig/hostconfig_test.go b/runconfig/hostconfig_test.go index aa137d2877..40b6db8155 100644 --- a/runconfig/hostconfig_test.go +++ b/runconfig/hostconfig_test.go @@ -240,46 +240,41 @@ func TestDecodeHostConfig(t *testing.T) { func TestValidateResources(t *testing.T) { type resourceTest struct { - ConfigCPURealtimePeriod int64 - ConfigCPURealtimeRuntime int64 - SysInfoCPURealtimePeriod bool - SysInfoCPURealtimeRuntime bool - ErrorExpected bool - FailureMsg string + ConfigCPURealtimePeriod int64 + ConfigCPURealtimeRuntime int64 + SysInfoCPURealtime bool + ErrorExpected bool + FailureMsg string } tests := []resourceTest{ { - ConfigCPURealtimePeriod: 1000, - ConfigCPURealtimeRuntime: 1000, - SysInfoCPURealtimePeriod: true, - SysInfoCPURealtimeRuntime: true, - ErrorExpected: false, - FailureMsg: "Expected valid configuration", + ConfigCPURealtimePeriod: 1000, + ConfigCPURealtimeRuntime: 1000, + SysInfoCPURealtime: true, + ErrorExpected: false, + FailureMsg: "Expected valid configuration", }, { - ConfigCPURealtimePeriod: 5000, - ConfigCPURealtimeRuntime: 5000, - SysInfoCPURealtimePeriod: false, - SysInfoCPURealtimeRuntime: true, - ErrorExpected: true, - FailureMsg: "Expected failure when cpu-rt-period is set but kernel doesn't support it", + ConfigCPURealtimePeriod: 5000, + ConfigCPURealtimeRuntime: 5000, + SysInfoCPURealtime: false, + ErrorExpected: true, + FailureMsg: "Expected failure when cpu-rt-period is set but kernel doesn't support it", }, { - ConfigCPURealtimePeriod: 5000, - ConfigCPURealtimeRuntime: 5000, - SysInfoCPURealtimePeriod: true, - SysInfoCPURealtimeRuntime: false, - ErrorExpected: true, - FailureMsg: "Expected failure when cpu-rt-runtime is set but kernel doesn't support it", + ConfigCPURealtimePeriod: 5000, + ConfigCPURealtimeRuntime: 5000, + SysInfoCPURealtime: false, + ErrorExpected: true, + FailureMsg: "Expected failure when cpu-rt-runtime is set but kernel doesn't support it", }, { - ConfigCPURealtimePeriod: 5000, - ConfigCPURealtimeRuntime: 10000, - SysInfoCPURealtimePeriod: true, - SysInfoCPURealtimeRuntime: false, - ErrorExpected: true, - FailureMsg: "Expected failure when cpu-rt-runtime is greater than cpu-rt-period", + ConfigCPURealtimePeriod: 5000, + ConfigCPURealtimeRuntime: 10000, + SysInfoCPURealtime: true, + ErrorExpected: true, + FailureMsg: "Expected failure when cpu-rt-runtime is greater than cpu-rt-period", }, } @@ -289,8 +284,7 @@ func TestValidateResources(t *testing.T) { hc.Resources.CPURealtimeRuntime = rt.ConfigCPURealtimeRuntime var si sysinfo.SysInfo - si.CPURealtimePeriod = rt.SysInfoCPURealtimePeriod - si.CPURealtimeRuntime = rt.SysInfoCPURealtimeRuntime + si.CPURealtime = rt.SysInfoCPURealtime if err := validateResources(&hc, &si); (err != nil) != rt.ErrorExpected { t.Fatal(rt.FailureMsg, err) diff --git a/runconfig/hostconfig_unix.go b/runconfig/hostconfig_unix.go index e579b06d9b..588cfa5644 100644 --- a/runconfig/hostconfig_unix.go +++ b/runconfig/hostconfig_unix.go @@ -85,12 +85,8 @@ func validateResources(hc *container.HostConfig, si *sysinfo.SysInfo) error { return nil } - if hc.Resources.CPURealtimePeriod > 0 && !si.CPURealtimePeriod { - return fmt.Errorf("Your kernel does not support cgroup cpu real-time period") - } - - if hc.Resources.CPURealtimeRuntime > 0 && !si.CPURealtimeRuntime { - return fmt.Errorf("Your kernel does not support cgroup cpu real-time runtime") + if (hc.Resources.CPURealtimePeriod != 0 || hc.Resources.CPURealtimeRuntime != 0) && !si.CPURealtime { + return fmt.Errorf("Your kernel does not support CPU real-time scheduler") } if hc.Resources.CPURealtimePeriod != 0 && hc.Resources.CPURealtimeRuntime != 0 && hc.Resources.CPURealtimeRuntime > hc.Resources.CPURealtimePeriod { From e3cff19dd1fe82aae6b592d91f54cd86978511f0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 22 May 2020 15:05:13 -0700 Subject: [PATCH 2/2] Untangle CPU RT controller init Commit 56f77d5ade945b added code that is doing some very ugly things. In partucular, calling cgroups.FindCgroupMountpointAndRoot() and daemon.SysInfoRaw() inside a recursively-called initCgroupsPath() not not a good thing to do. This commit tries to partially untangle this by moving some expensive checks and calls earlier, in a minimally invasive way (meaning I tried hard to not break any logic, however weird it is). This also removes double call to MkdirAll (not important, but it sticks out) and renames the function to better reflect what it's doing. Finally, this wraps some of the errors returned, and fixes the init function to not ignore the error from itself. This could be reworked more radically, but at least this this commit we are calling expensive functions once, and only if necessary. Signed-off-by: Kir Kolyshkin --- daemon/daemon_unix.go | 43 ++++++++++++------------------------------- daemon/oci_linux.go | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 5bb61d5fe1..4a0779b116 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -1694,51 +1694,32 @@ func setupOOMScoreAdj(score int) error { return err } -func (daemon *Daemon) initCgroupsPath(path string) error { +func (daemon *Daemon) initCPURtController(mnt, path string) error { if path == "/" || path == "." { return nil } - if daemon.configStore.CPURealtimePeriod == 0 && daemon.configStore.CPURealtimeRuntime == 0 { - return nil - } - - if cgroups.IsCgroup2UnifiedMode() { - return fmt.Errorf("daemon-scoped cpu-rt-period and cpu-rt-runtime are not implemented for cgroup v2") - } - // Recursively create cgroup to ensure that the system and all parent cgroups have values set // for the period and runtime as this limits what the children can be set to. - daemon.initCgroupsPath(filepath.Dir(path)) - - mnt, root, err := cgroups.FindCgroupMountpointAndRoot("", "cpu") - if err != nil { + if err := daemon.initCPURtController(mnt, filepath.Dir(path)); err != nil { return err } - // When docker is run inside docker, the root is based of the host cgroup. - // Should this be handled in runc/libcontainer/cgroups ? - if strings.HasPrefix(root, "/docker/") { - root = "/" - } - path = filepath.Join(mnt, root, path) - sysInfo := daemon.RawSysInfo(true) - if err := maybeCreateCPURealTimeFile(sysInfo.CPURealtime, daemon.configStore.CPURealtimePeriod, "cpu.rt_period_us", path); err != nil { + path = filepath.Join(mnt, path) + if err := os.MkdirAll(path, 0755); err != nil { return err } - return maybeCreateCPURealTimeFile(sysInfo.CPURealtime, daemon.configStore.CPURealtimeRuntime, "cpu.rt_runtime_us", path) + if err := maybeCreateCPURealTimeFile(daemon.configStore.CPURealtimePeriod, "cpu.rt_period_us", path); err != nil { + return err + } + return maybeCreateCPURealTimeFile(daemon.configStore.CPURealtimeRuntime, "cpu.rt_runtime_us", path) } -func maybeCreateCPURealTimeFile(sysinfoPresent bool, configValue int64, file string, path string) error { - if sysinfoPresent && configValue != 0 { - if err := os.MkdirAll(path, 0755); err != nil { - return err - } - if err := ioutil.WriteFile(filepath.Join(path, file), []byte(strconv.FormatInt(configValue, 10)), 0700); err != nil { - return err - } +func maybeCreateCPURealTimeFile(configValue int64, file string, path string) error { + if configValue == 0 { + return nil } - return nil + return ioutil.WriteFile(filepath.Join(path, file), []byte(strconv.FormatInt(configValue, 10)), 0700) } func (daemon *Daemon) setupSeccompProfile() error { diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index da1de47470..180cd4e992 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -824,15 +824,32 @@ func WithCgroups(daemon *Daemon, c *container.Container) coci.SpecOpts { cgroupsPath = filepath.Join(parent, c.ID) } s.Linux.CgroupsPath = cgroupsPath + + // the rest is only needed for CPU RT controller + + if daemon.configStore.CPURealtimePeriod == 0 && daemon.configStore.CPURealtimeRuntime == 0 { + return nil + } + + if cgroups.IsCgroup2UnifiedMode() { + return errors.New("daemon-scoped cpu-rt-period and cpu-rt-runtime are not implemented for cgroup v2") + } + + // FIXME this is very expensive way to check if cpu rt is supported + sysInfo := daemon.RawSysInfo(true) + if !sysInfo.CPURealtime { + return errors.New("daemon-scoped cpu-rt-period and cpu-rt-runtime are not supported by the kernel") + } + p := cgroupsPath if useSystemd { initPath, err := cgroups.GetInitCgroup("cpu") if err != nil { - return err + return errors.Wrap(err, "unable to init CPU RT controller") } _, err = cgroups.GetOwnCgroup("cpu") if err != nil { - return err + return errors.Wrap(err, "unable to init CPU RT controller") } p = filepath.Join(initPath, s.Linux.CgroupsPath) } @@ -843,8 +860,19 @@ func WithCgroups(daemon *Daemon, c *container.Container) coci.SpecOpts { parentPath = filepath.Clean("/" + parentPath) } - if err := daemon.initCgroupsPath(parentPath); err != nil { - return fmt.Errorf("linux init cgroups path: %v", err) + mnt, root, err := cgroups.FindCgroupMountpointAndRoot("", "cpu") + if err != nil { + return errors.Wrap(err, "unable to init CPU RT controller") + } + // When docker is run inside docker, the root is based of the host cgroup. + // Should this be handled in runc/libcontainer/cgroups ? + if strings.HasPrefix(root, "/docker/") { + root = "/" + } + mnt = filepath.Join(mnt, root) + + if err := daemon.initCPURtController(mnt, parentPath); err != nil { + return errors.Wrap(err, "unable to init CPU RT controller") } return nil }