From 11fb94965cab5b2a54da9dca50d0e36d0ee36ef0 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Sun, 31 May 2020 14:56:43 -0700
Subject: [PATCH 1/4] cgroups/fs: rm Remove method from controllers

To my surprise, those are not used anywhere in the code.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/blkio.go      | 4 ----
 libcontainer/cgroups/fs/cpu.go        | 4 ----
 libcontainer/cgroups/fs/cpuacct.go    | 4 ----
 libcontainer/cgroups/fs/cpuset.go     | 4 ----
 libcontainer/cgroups/fs/devices.go    | 4 ----
 libcontainer/cgroups/fs/freezer.go    | 4 ----
 libcontainer/cgroups/fs/fs.go         | 2 --
 libcontainer/cgroups/fs/hugetlb.go    | 4 ----
 libcontainer/cgroups/fs/memory.go     | 4 ----
 libcontainer/cgroups/fs/name.go       | 7 -------
 libcontainer/cgroups/fs/net_cls.go    | 4 ----
 libcontainer/cgroups/fs/net_prio.go   | 4 ----
 libcontainer/cgroups/fs/perf_event.go | 4 ----
 libcontainer/cgroups/fs/pids.go       | 4 ----
 14 files changed, 57 deletions(-)

diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go
index 52c118d68..80c8cf09c 100644
--- a/libcontainer/cgroups/fs/blkio.go
+++ b/libcontainer/cgroups/fs/blkio.go
@@ -74,10 +74,6 @@ func (s *BlkioGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *BlkioGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("blkio"))
-}
-
 /*
 examples:
 
diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go
index 7386aace1..854a1bfc5 100644
--- a/libcontainer/cgroups/fs/cpu.go
+++ b/libcontainer/cgroups/fs/cpu.go
@@ -96,10 +96,6 @@ func (s *CpuGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return s.SetRtSched(path, cgroup)
 }
 
-func (s *CpuGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("cpu"))
-}
-
 func (s *CpuGroup) GetStats(path string, stats *cgroups.Stats) error {
 	f, err := os.Open(filepath.Join(path, "cpu.stat"))
 	if err != nil {
diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go
index dd8aea0c2..bda343855 100644
--- a/libcontainer/cgroups/fs/cpuacct.go
+++ b/libcontainer/cgroups/fs/cpuacct.go
@@ -53,10 +53,6 @@ func (s *CpuacctGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *CpuacctGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("cpuacct"))
-}
-
 func (s *CpuacctGroup) GetStats(path string, stats *cgroups.Stats) error {
 	userModeUsage, kernelModeUsage, err := getCpuUsageBreakdown(path)
 	if err != nil {
diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go
index 6a7725e35..9e5a588da 100644
--- a/libcontainer/cgroups/fs/cpuset.go
+++ b/libcontainer/cgroups/fs/cpuset.go
@@ -45,10 +45,6 @@ func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *CpusetGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("cpuset"))
-}
-
 func (s *CpusetGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go
index 6a8f1d354..af8929c2b 100644
--- a/libcontainer/cgroups/fs/devices.go
+++ b/libcontainer/cgroups/fs/devices.go
@@ -106,10 +106,6 @@ func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *DevicesGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("devices"))
-}
-
 func (s *DevicesGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go
index b5b77da94..e8260aff0 100644
--- a/libcontainer/cgroups/fs/freezer.go
+++ b/libcontainer/cgroups/fs/freezer.go
@@ -61,10 +61,6 @@ func (s *FreezerGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *FreezerGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("freezer"))
-}
-
 func (s *FreezerGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index f9034f09f..8260c5445 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -54,8 +54,6 @@ type subsystem interface {
 	Name() string
 	// Returns the stats, as 'stats', corresponding to the cgroup under 'path'.
 	GetStats(path string, stats *cgroups.Stats) error
-	// Removes the cgroup represented by 'cgroupData'.
-	Remove(*cgroupData) error
 	// Creates and joins the cgroup represented by 'cgroupData'.
 	Apply(*cgroupData) error
 	// Set the cgroup represented by cgroup.
diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go
index 68719c2e6..68f784625 100644
--- a/libcontainer/cgroups/fs/hugetlb.go
+++ b/libcontainer/cgroups/fs/hugetlb.go
@@ -37,10 +37,6 @@ func (s *HugetlbGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *HugetlbGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("hugetlb"))
-}
-
 func (s *HugetlbGroup) GetStats(path string, stats *cgroups.Stats) error {
 	hugetlbStats := cgroups.HugetlbStats{}
 	for _, pageSize := range HugePageSizes {
diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go
index f7b8f999e..431f59385 100644
--- a/libcontainer/cgroups/fs/memory.go
+++ b/libcontainer/cgroups/fs/memory.go
@@ -165,10 +165,6 @@ func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *MemoryGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("memory"))
-}
-
 func (s *MemoryGroup) GetStats(path string, stats *cgroups.Stats) error {
 	// Set stats from memory.stat.
 	statsFile, err := os.Open(filepath.Join(path, "memory.stat"))
diff --git a/libcontainer/cgroups/fs/name.go b/libcontainer/cgroups/fs/name.go
index d8cf1d87c..259c41754 100644
--- a/libcontainer/cgroups/fs/name.go
+++ b/libcontainer/cgroups/fs/name.go
@@ -28,13 +28,6 @@ func (s *NameGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *NameGroup) Remove(d *cgroupData) error {
-	if s.Join {
-		removePath(d.path(s.GroupName))
-	}
-	return nil
-}
-
 func (s *NameGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go
index 021201525..de0a9022f 100644
--- a/libcontainer/cgroups/fs/net_cls.go
+++ b/libcontainer/cgroups/fs/net_cls.go
@@ -35,10 +35,6 @@ func (s *NetClsGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *NetClsGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("net_cls"))
-}
-
 func (s *NetClsGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go
index 2bdeedf80..7b609f7d3 100644
--- a/libcontainer/cgroups/fs/net_prio.go
+++ b/libcontainer/cgroups/fs/net_prio.go
@@ -33,10 +33,6 @@ func (s *NetPrioGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *NetPrioGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("net_prio"))
-}
-
 func (s *NetPrioGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go
index 5693676d3..0b03027fd 100644
--- a/libcontainer/cgroups/fs/perf_event.go
+++ b/libcontainer/cgroups/fs/perf_event.go
@@ -26,10 +26,6 @@ func (s *PerfEventGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *PerfEventGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("perf_event"))
-}
-
 func (s *PerfEventGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go
index 7bf680135..1db0bc045 100644
--- a/libcontainer/cgroups/fs/pids.go
+++ b/libcontainer/cgroups/fs/pids.go
@@ -44,10 +44,6 @@ func (s *PidsGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *PidsGroup) Remove(d *cgroupData) error {
-	return removePath(d.path("pids"))
-}
-
 func (s *PidsGroup) GetStats(path string, stats *cgroups.Stats) error {
 	current, err := fscommon.GetCgroupParamUint(path, "pids.current")
 	if err != nil {

From 2e225799469c50f28c46ae2f7f511052696beca5 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Fri, 3 Jul 2020 12:35:31 -0700
Subject: [PATCH 2/4] libct/cgroups/fs.GetStats: drop PathExists check

Half of controllers' GetStats just return nil, and most of the others
ignore ENOENT on files, so it will be cheaper to not check that the
path exists in the main GetStats method, offloading that to the
controllers.

Drop PathExists check from GetStats, add it to those controllers'
GetStats where it was missing.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/cpuacct.go | 3 +++
 libcontainer/cgroups/fs/fs.go      | 2 +-
 libcontainer/cgroups/fs/hugetlb.go | 3 +++
 libcontainer/cgroups/fs/pids.go    | 3 +++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go
index bda343855..1797fb786 100644
--- a/libcontainer/cgroups/fs/cpuacct.go
+++ b/libcontainer/cgroups/fs/cpuacct.go
@@ -54,6 +54,9 @@ func (s *CpuacctGroup) Set(path string, cgroup *configs.Cgroup) error {
 }
 
 func (s *CpuacctGroup) GetStats(path string, stats *cgroups.Stats) error {
+	if !cgroups.PathExists(path) {
+		return nil
+	}
 	userModeUsage, kernelModeUsage, err := getCpuUsageBreakdown(path)
 	if err != nil {
 		return err
diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index 8260c5445..7ac0bdbe5 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -250,7 +250,7 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
 	stats := cgroups.NewStats()
 	for name, path := range m.paths {
 		sys, err := m.getSubsystems().Get(name)
-		if err == errSubsystemDoesNotExist || !cgroups.PathExists(path) {
+		if err == errSubsystemDoesNotExist {
 			continue
 		}
 		if err := sys.GetStats(path, stats); err != nil {
diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go
index 68f784625..a5e4c0145 100644
--- a/libcontainer/cgroups/fs/hugetlb.go
+++ b/libcontainer/cgroups/fs/hugetlb.go
@@ -39,6 +39,9 @@ func (s *HugetlbGroup) Set(path string, cgroup *configs.Cgroup) error {
 
 func (s *HugetlbGroup) GetStats(path string, stats *cgroups.Stats) error {
 	hugetlbStats := cgroups.HugetlbStats{}
+	if !cgroups.PathExists(path) {
+		return nil
+	}
 	for _, pageSize := range HugePageSizes {
 		usage := strings.Join([]string{"hugetlb", pageSize, "usage_in_bytes"}, ".")
 		value, err := fscommon.GetCgroupParamUint(path, usage)
diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go
index 1db0bc045..1d7b1c168 100644
--- a/libcontainer/cgroups/fs/pids.go
+++ b/libcontainer/cgroups/fs/pids.go
@@ -45,6 +45,9 @@ func (s *PidsGroup) Set(path string, cgroup *configs.Cgroup) error {
 }
 
 func (s *PidsGroup) GetStats(path string, stats *cgroups.Stats) error {
+	if !cgroups.PathExists(path) {
+		return nil
+	}
 	current, err := fscommon.GetCgroupParamUint(path, "pids.current")
 	if err != nil {
 		return fmt.Errorf("failed to parse pids.current - %s", err)

From daf30cb7cad4cd13b4f66d00fbade1d4296db6df Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Sun, 31 May 2020 11:16:58 -0700
Subject: [PATCH 3/4] cgroups/fs: rm getSubsystems

It does not add any value.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/fs.go | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index 7ac0bdbe5..0f0a7a865 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -18,7 +18,7 @@ import (
 )
 
 var (
-	subsystemsLegacy = subsystemSet{
+	subsystems = subsystemSet{
 		&CpusetGroup{},
 		&DevicesGroup{},
 		&MemoryGroup{},
@@ -164,10 +164,6 @@ func isIgnorableError(rootless bool, err error) bool {
 	return false
 }
 
-func (m *manager) getSubsystems() subsystemSet {
-	return subsystemsLegacy
-}
-
 func (m *manager) Apply(pid int) (err error) {
 	if m.cgroups == nil {
 		return nil
@@ -197,7 +193,7 @@ func (m *manager) Apply(pid int) (err error) {
 		return cgroups.EnterPid(m.paths, pid)
 	}
 
-	for _, sys := range m.getSubsystems() {
+	for _, sys := range subsystems {
 		p, err := d.path(sys.Name())
 		if err != nil {
 			// The non-presence of the devices subsystem is
@@ -249,7 +245,7 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()
 	for name, path := range m.paths {
-		sys, err := m.getSubsystems().Get(name)
+		sys, err := subsystems.Get(name)
 		if err == errSubsystemDoesNotExist {
 			continue
 		}
@@ -273,7 +269,7 @@ func (m *manager) Set(container *configs.Config) error {
 
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	for _, sys := range m.getSubsystems() {
+	for _, sys := range subsystems {
 		path := m.paths[sys.Name()]
 		if err := sys.Set(path, container.Cgroups); err != nil {
 			if m.rootless && sys.Name() == "devices" {

From 2a322e91ecaf642f339adac794dc3a7e361342bd Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Wed, 3 Jun 2020 09:11:20 -0700
Subject: [PATCH 4/4] cgroupv1: remove subsystemSet.Get()

Instead of iterating over m.paths, iterate over subsystems and look up
the path for each. This is faster since a map lookup is faster than
iterating over the names in Get. A quick benchmark shows that the new
way is 2.5x faster than the old one.

Note though that this is not done to make things faster, as savings are
negligible, but to make things simpler by removing some code.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/fs.go      | 19 ++++---------------
 libcontainer/cgroups/systemd/v1.go | 19 ++++---------------
 2 files changed, 8 insertions(+), 30 deletions(-)

diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index 0f0a7a865..dd62dc1e3 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -18,7 +18,7 @@ import (
 )
 
 var (
-	subsystems = subsystemSet{
+	subsystems = []subsystem{
 		&CpusetGroup{},
 		&DevicesGroup{},
 		&MemoryGroup{},
@@ -38,17 +38,6 @@ var (
 
 var errSubsystemDoesNotExist = errors.New("cgroup: subsystem does not exist")
 
-type subsystemSet []subsystem
-
-func (s subsystemSet) Get(name string) (subsystem, error) {
-	for _, ss := range s {
-		if ss.Name() == name {
-			return ss, nil
-		}
-	}
-	return nil, errSubsystemDoesNotExist
-}
-
 type subsystem interface {
 	// Name returns the name of the subsystem.
 	Name() string
@@ -244,9 +233,9 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()
-	for name, path := range m.paths {
-		sys, err := subsystems.Get(name)
-		if err == errSubsystemDoesNotExist {
+	for _, sys := range subsystems {
+		path := m.paths[sys.Name()]
+		if path == "" {
 			continue
 		}
 		if err := sys.GetStats(path, stats); err != nil {
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
index c3cc9c8b3..7217b0af7 100644
--- a/libcontainer/cgroups/systemd/v1.go
+++ b/libcontainer/cgroups/systemd/v1.go
@@ -41,18 +41,7 @@ type subsystem interface {
 
 var errSubsystemDoesNotExist = errors.New("cgroup: subsystem does not exist")
 
-type subsystemSet []subsystem
-
-func (s subsystemSet) Get(name string) (subsystem, error) {
-	for _, ss := range s {
-		if ss.Name() == name {
-			return ss, nil
-		}
-	}
-	return nil, errSubsystemDoesNotExist
-}
-
-var legacySubsystems = subsystemSet{
+var legacySubsystems = []subsystem{
 	&fs.CpusetGroup{},
 	&fs.DevicesGroup{},
 	&fs.MemoryGroup{},
@@ -355,9 +344,9 @@ func (m *legacyManager) GetStats() (*cgroups.Stats, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()
-	for name, path := range m.paths {
-		sys, err := legacySubsystems.Get(name)
-		if err == errSubsystemDoesNotExist || !cgroups.PathExists(path) {
+	for _, sys := range legacySubsystems {
+		path := m.paths[sys.Name()]
+		if path == "" {
 			continue
 		}
 		if err := sys.GetStats(path, stats); err != nil {