Merge pull request opencontainers#2494 from kolyshkin/cgroupv1-optimize-path

cgroups/v1: optimize path usage

Author: AkihiroSuda

libcontainer/cgroups/fs/fs.go

@@ -220,26 +220,26 @@ func (m *manager) Apply(pid int) (err error) {
 
 	var c = m.cgroups
 
-	d, err := getCgroupData(m.cgroups, pid)
-	if err != nil {
-		return err
-	}
-
 	m.paths = make(map[string]string)
 	if c.Paths != nil {
+		cgMap, err := cgroups.ParseCgroupFile("/proc/self/cgroup")
+		if err != nil {
+			return err
+		}
 		for name, path := range c.Paths {
-			_, err := d.path(name)
-			if err != nil {
-				if cgroups.IsNotFound(err) {
-					continue
-				}
-				return err
+			// XXX(kolyshkin@): why this check is needed?
+			if _, ok := cgMap[name]; ok {
+				m.paths[name] = path
 			}
-			m.paths[name] = path
 		}
 		return cgroups.EnterPid(m.paths, pid)
 	}
 
+	d, err := getCgroupData(m.cgroups, pid)
+	if err != nil {
+		return err
+	}
+
 	for _, sys := range subsystems {
 		p, err := d.path(sys.Name())
 		if err != nil {

libcontainer/cgroups/systemd/v1.go

@@ -105,16 +105,15 @@ func (m *legacyManager) Apply(pid int) error {
 	defer m.mu.Unlock()
 	if c.Paths != nil {
 		paths := make(map[string]string)
+		cgMap, err := cgroups.ParseCgroupFile("/proc/self/cgroup")
+		if err != nil {
+			return err
+		}
+		// XXX(kolyshkin@): why this check is needed?
 		for name, path := range c.Paths {
-			_, err := getSubsystemPath(m.cgroups, name)
-			if err != nil {
-				// Don't fail if a cgroup hierarchy was not found, just skip this subsystem
-				if cgroups.IsNotFound(err) {
-					continue
-				}
-				return err
+			if _, ok := cgMap[name]; ok {
+				paths[name] = path
 			}
-			paths[name] = path
 		}
 		m.paths = paths
 		return cgroups.EnterPid(m.paths, pid)
@@ -179,14 +178,16 @@ func (m *legacyManager) Apply(pid int) error {
 		return err
 	}
 
-	if err := joinCgroups(c, pid); err != nil {
-		return err
-	}
-
 	paths := make(map[string]string)
 	for _, s := range legacySubsystems {
 		subsystemPath, err := getSubsystemPath(m.cgroups, s.Name())
 		if err != nil {
+			// Even if it's `not found` error, we'll return err
+			// because devices cgroup is hard requirement for
+			// container security.
+			if s.Name() == "devices" {
+				return err
+			}
 			// Don't fail if a cgroup hierarchy was not found, just skip this subsystem
 			if cgroups.IsNotFound(err) {
 				continue
@@ -196,6 +197,11 @@ func (m *legacyManager) Apply(pid int) error {
 		paths[s.Name()] = subsystemPath
 	}
 	m.paths = paths
+
+	if err := m.joinCgroups(pid); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -228,48 +234,25 @@ func (m *legacyManager) Path(subsys string) string {
 	return m.paths[subsys]
 }
 
-func join(c *configs.Cgroup, subsystem string, pid int) (string, error) {
-	path, err := getSubsystemPath(c, subsystem)
-	if err != nil {
-		return "", err
-	}
-
-	if err := os.MkdirAll(path, 0755); err != nil {
-		return "", err
-	}
-	if err := cgroups.WriteCgroupProc(path, pid); err != nil {
-		return "", err
-	}
-	return path, nil
-}
-
-func joinCgroups(c *configs.Cgroup, pid int) error {
+func (m *legacyManager) joinCgroups(pid int) error {
 	for _, sys := range legacySubsystems {
 		name := sys.Name()
 		switch name {
 		case "name=systemd":
 			// let systemd handle this
 		case "cpuset":
-			path, err := getSubsystemPath(c, name)
-			if err != nil && !cgroups.IsNotFound(err) {
-				return err
-			}
-			s := &fs.CpusetGroup{}
-			if err := s.ApplyDir(path, c, pid); err != nil {
-				return err
+			if path, ok := m.paths[name]; ok {
+				s := &fs.CpusetGroup{}
+				if err := s.ApplyDir(path, m.cgroups, pid); err != nil {
+					return err
+				}
 			}
 		default:
-			_, err := join(c, name, pid)
-			if err != nil {
-				// Even if it's `not found` error, we'll return err
-				// because devices cgroup is hard requirement for
-				// container security.
-				if name == "devices" {
+			if path, ok := m.paths[name]; ok {
+				if err := os.MkdirAll(path, 0755); err != nil {
 					return err
 				}
-				// For other subsystems, omit the `not found` error
-				// because they are optional.
-				if !cgroups.IsNotFound(err) {
+				if err := cgroups.WriteCgroupProc(path, pid); err != nil {
 					return err
 				}
 			}
@@ -306,33 +289,32 @@ func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) {
 }
 
 func (m *legacyManager) Freeze(state configs.FreezerState) error {
-	path, err := getSubsystemPath(m.cgroups, "freezer")
-	if err != nil {
-		return err
+	path, ok := m.paths["freezer"]
+	if !ok {
+		return errSubsystemDoesNotExist
 	}
 	prevState := m.cgroups.Resources.Freezer
 	m.cgroups.Resources.Freezer = state
 	freezer := &fs.FreezerGroup{}
-	err = freezer.Set(path, m.cgroups)
-	if err != nil {
+	if err := freezer.Set(path, m.cgroups); err != nil {
 		m.cgroups.Resources.Freezer = prevState
 		return err
 	}
 	return nil
 }
 
 func (m *legacyManager) GetPids() ([]int, error) {
-	path, err := getSubsystemPath(m.cgroups, "devices")
-	if err != nil {
-		return nil, err
+	path, ok := m.paths["devices"]
+	if !ok {
+		return nil, errSubsystemDoesNotExist
 	}
 	return cgroups.GetPids(path)
 }
 
 func (m *legacyManager) GetAllPids() ([]int, error) {
-	path, err := getSubsystemPath(m.cgroups, "devices")
-	if err != nil {
-		return nil, err
+	path, ok := m.paths["devices"]
+	if !ok {
+		return nil, errSubsystemDoesNotExist
 	}
 	return cgroups.GetAllPids(path)
 }
@@ -403,9 +385,9 @@ func (m *legacyManager) Set(container *configs.Config) error {
 
 	for _, sys := range legacySubsystems {
 		// Get the subsystem path, but don't error out for not found cgroups.
-		path, err := getSubsystemPath(container.Cgroups, sys.Name())
-		if err != nil && !cgroups.IsNotFound(err) {
-			return err
+		path, ok := m.paths[sys.Name()]
+		if !ok {
+			continue
 		}
 		if err := sys.Set(path, container.Cgroups); err != nil {
 			return err
@@ -447,9 +429,9 @@ func (m *legacyManager) GetCgroups() (*configs.Cgroup, error) {
 }
 
 func (m *legacyManager) GetFreezerState() (configs.FreezerState, error) {
-	path, err := getSubsystemPath(m.cgroups, "freezer")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return configs.Undefined, err
+	path, ok := m.paths["freezer"]
+	if !ok {
+		return configs.Undefined, nil
 	}
 	freezer := &fs.FreezerGroup{}
 	return freezer.GetState(path)