Переглянути джерело

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 <kolyshkin@gmail.com>
Kir Kolyshkin 5 роки тому
батько
коміт
e3cff19dd1
2 змінених файлів з 44 додано та 35 видалено
  1. 12 31
      daemon/daemon_unix.go
  2. 32 4
      daemon/oci_linux.go

+ 12 - 31
daemon/daemon_unix.go

@@ -1694,51 +1694,32 @@ func setupOOMScoreAdj(score int) error {
 	return err
 	return err
 }
 }
 
 
-func (daemon *Daemon) initCgroupsPath(path string) error {
+func (daemon *Daemon) initCPURtController(mnt, path string) error {
 	if path == "/" || path == "." {
 	if path == "/" || path == "." {
 		return nil
 		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
 	// 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.
 	// 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
 		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 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 {
 func (daemon *Daemon) setupSeccompProfile() error {

+ 32 - 4
daemon/oci_linux.go

@@ -824,15 +824,32 @@ func WithCgroups(daemon *Daemon, c *container.Container) coci.SpecOpts {
 			cgroupsPath = filepath.Join(parent, c.ID)
 			cgroupsPath = filepath.Join(parent, c.ID)
 		}
 		}
 		s.Linux.CgroupsPath = cgroupsPath
 		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
 		p := cgroupsPath
 		if useSystemd {
 		if useSystemd {
 			initPath, err := cgroups.GetInitCgroup("cpu")
 			initPath, err := cgroups.GetInitCgroup("cpu")
 			if err != nil {
 			if err != nil {
-				return err
+				return errors.Wrap(err, "unable to init CPU RT controller")
 			}
 			}
 			_, err = cgroups.GetOwnCgroup("cpu")
 			_, err = cgroups.GetOwnCgroup("cpu")
 			if err != nil {
 			if err != nil {
-				return err
+				return errors.Wrap(err, "unable to init CPU RT controller")
 			}
 			}
 			p = filepath.Join(initPath, s.Linux.CgroupsPath)
 			p = filepath.Join(initPath, s.Linux.CgroupsPath)
 		}
 		}
@@ -843,8 +860,19 @@ func WithCgroups(daemon *Daemon, c *container.Container) coci.SpecOpts {
 			parentPath = filepath.Clean("/" + parentPath)
 			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
 		return nil
 	}
 	}