From c1adc99a209bb40b3aff84842092cd537fc53a5c Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Sun, 31 May 2020 16:02:34 -0700
Subject: [PATCH] cgroup/fs: rework Apply()

In manager.Apply() method, a path to each subsystem is obtained by
calling d.path(sys.Name()), and the sys.Apply() is called that does
the same call to d.path() again.

d.path() is an expensive call, so rather than to call it twice, let's
reuse the result.

This results the number of times we parse mountinfo during container
start from 62 to 34 on my setup.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/blkio.go      |  8 ++------
 libcontainer/cgroups/fs/cpu.go        | 20 +++++---------------
 libcontainer/cgroups/fs/cpu_test.go   |  4 +++-
 libcontainer/cgroups/fs/cpuacct.go    |  9 ++-------
 libcontainer/cgroups/fs/cpuset.go     |  8 ++------
 libcontainer/cgroups/fs/devices.go    | 13 ++++++-------
 libcontainer/cgroups/fs/freezer.go    |  8 ++------
 libcontainer/cgroups/fs/fs.go         | 18 +++++++-----------
 libcontainer/cgroups/fs/hugetlb.go    |  8 ++------
 libcontainer/cgroups/fs/memory.go     | 13 +++----------
 libcontainer/cgroups/fs/name.go       |  4 ++--
 libcontainer/cgroups/fs/net_cls.go    |  8 ++------
 libcontainer/cgroups/fs/net_prio.go   |  8 ++------
 libcontainer/cgroups/fs/perf_event.go |  8 ++------
 libcontainer/cgroups/fs/pids.go       |  8 ++------
 15 files changed, 44 insertions(+), 101 deletions(-)

diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go
index 52c118d68..5cea822fb 100644
--- a/libcontainer/cgroups/fs/blkio.go
+++ b/libcontainer/cgroups/fs/blkio.go
@@ -22,12 +22,8 @@ func (s *BlkioGroup) Name() string {
 	return "blkio"
 }
 
-func (s *BlkioGroup) Apply(d *cgroupData) error {
-	_, err := d.join("blkio")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *BlkioGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *BlkioGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go
index 7386aace1..11bbe1097 100644
--- a/libcontainer/cgroups/fs/cpu.go
+++ b/libcontainer/cgroups/fs/cpu.go
@@ -21,17 +21,7 @@ func (s *CpuGroup) Name() string {
 	return "cpu"
 }
 
-func (s *CpuGroup) Apply(d *cgroupData) error {
-	// We always want to join the cpu group, to allow fair cpu scheduling
-	// on a container basis
-	path, err := d.path("cpu")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return s.ApplyDir(path, d.config, d.pid)
-}
-
-func (s *CpuGroup) ApplyDir(path string, cgroup *configs.Cgroup, pid int) error {
+func (s *CpuGroup) Apply(path string, d *cgroupData) error {
 	// This might happen if we have no cpu cgroup mounted.
 	// Just do nothing and don't fail.
 	if path == "" {
@@ -43,12 +33,12 @@ func (s *CpuGroup) ApplyDir(path string, cgroup *configs.Cgroup, pid int) error
 	// We should set the real-Time group scheduling settings before moving
 	// in the process because if the process is already in SCHED_RR mode
 	// and no RT bandwidth is set, adding it will fail.
-	if err := s.SetRtSched(path, cgroup); err != nil {
+	if err := s.SetRtSched(path, d.config); err != nil {
 		return err
 	}
-	// because we are not using d.join we need to place the pid into the procs file
-	// unlike the other subsystems
-	return cgroups.WriteCgroupProc(path, pid)
+	// Since we are not using join(), we need to place the pid
+	// into the procs file unlike other subsystems.
+	return cgroups.WriteCgroupProc(path, d.pid)
 }
 
 func (s *CpuGroup) SetRtSched(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/cpu_test.go b/libcontainer/cgroups/fs/cpu_test.go
index 2eeb489ef..4a8ecf995 100644
--- a/libcontainer/cgroups/fs/cpu_test.go
+++ b/libcontainer/cgroups/fs/cpu_test.go
@@ -182,7 +182,9 @@ func TestCpuSetRtSchedAtApply(t *testing.T) {
 	helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter
 	helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter
 	cpu := &CpuGroup{}
-	if err := cpu.ApplyDir(helper.CgroupPath, helper.CgroupData.config, 1234); err != nil {
+
+	helper.CgroupData.pid = 1234
+	if err := cpu.Apply(helper.CgroupPath, helper.CgroupData); err != nil {
 		t.Fatal(err)
 	}
 
diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go
index dd8aea0c2..123ba5e0a 100644
--- a/libcontainer/cgroups/fs/cpuacct.go
+++ b/libcontainer/cgroups/fs/cpuacct.go
@@ -40,13 +40,8 @@ func (s *CpuacctGroup) Name() string {
 	return "cpuacct"
 }
 
-func (s *CpuacctGroup) Apply(d *cgroupData) error {
-	// we just want to join this group even though we don't set anything
-	if _, err := d.join("cpuacct"); err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-
-	return nil
+func (s *CpuacctGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *CpuacctGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go
index 6a7725e35..624e6decd 100644
--- a/libcontainer/cgroups/fs/cpuset.go
+++ b/libcontainer/cgroups/fs/cpuset.go
@@ -23,12 +23,8 @@ func (s *CpusetGroup) Name() string {
 	return "cpuset"
 }
 
-func (s *CpusetGroup) Apply(d *cgroupData) error {
-	dir, err := d.path("cpuset")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return s.ApplyDir(dir, d.config, d.pid)
+func (s *CpusetGroup) Apply(path string, d *cgroupData) error {
+	return s.ApplyDir(path, d.config, d.pid)
 }
 
 func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go
index 6a8f1d354..6c86fc31f 100644
--- a/libcontainer/cgroups/fs/devices.go
+++ b/libcontainer/cgroups/fs/devices.go
@@ -22,17 +22,16 @@ func (s *DevicesGroup) Name() string {
 	return "devices"
 }
 
-func (s *DevicesGroup) Apply(d *cgroupData) error {
+func (s *DevicesGroup) Apply(path string, d *cgroupData) error {
 	if d.config.SkipDevices {
 		return nil
 	}
-	_, err := d.join("devices")
-	if err != nil {
-		// We will return error even it's `not found` error, devices
-		// cgroup is hard requirement for container's security.
-		return err
+	if path == "" {
+		// Return error here, since devices cgroup
+		// is a hard requirement for container's security.
+		return errSubsystemDoesNotExist
 	}
-	return nil
+	return join(path, d.pid)
 }
 
 func loadEmulator(path string) (*devices.Emulator, error) {
diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go
index b5b77da94..9f0181a26 100644
--- a/libcontainer/cgroups/fs/freezer.go
+++ b/libcontainer/cgroups/fs/freezer.go
@@ -22,12 +22,8 @@ func (s *FreezerGroup) Name() string {
 	return "freezer"
 }
 
-func (s *FreezerGroup) Apply(d *cgroupData) error {
-	_, err := d.join("freezer")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *FreezerGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *FreezerGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index b9f298b41..d55a19ab9 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -57,7 +57,7 @@ type subsystem interface {
 	// Removes the cgroup represented by 'cgroupData'.
 	Remove(*cgroupData) error
 	// Creates and joins the cgroup represented by 'cgroupData'.
-	Apply(*cgroupData) error
+	Apply(path string, c *cgroupData) error
 	// Set the cgroup represented by cgroup.
 	Set(path string, cgroup *configs.Cgroup) error
 }
@@ -211,7 +211,7 @@ func (m *manager) Apply(pid int) (err error) {
 		}
 		m.paths[sys.Name()] = p
 
-		if err := sys.Apply(d); err != nil {
+		if err := sys.Apply(p, d); err != nil {
 			// In the case of rootless (including euid=0 in userns), where an
 			// explicit cgroup path hasn't been set, we don't bail on error in
 			// case of permission problems. Cases where limits have been set
@@ -374,18 +374,14 @@ func (raw *cgroupData) path(subsystem string) (string, error) {
 	return filepath.Join(parentPath, raw.innerPath), nil
 }
 
-func (raw *cgroupData) join(subsystem string) (string, error) {
-	path, err := raw.path(subsystem)
-	if err != nil {
-		return "", err
+func join(path string, pid int) error {
+	if path == "" {
+		return nil
 	}
 	if err := os.MkdirAll(path, 0755); err != nil {
-		return "", err
+		return err
 	}
-	if err := cgroups.WriteCgroupProc(path, raw.pid); err != nil {
-		return "", err
-	}
-	return path, nil
+	return cgroups.WriteCgroupProc(path, pid)
 }
 
 func removePath(p string, err error) error {
diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go
index 68719c2e6..ae9084b67 100644
--- a/libcontainer/cgroups/fs/hugetlb.go
+++ b/libcontainer/cgroups/fs/hugetlb.go
@@ -19,12 +19,8 @@ func (s *HugetlbGroup) Name() string {
 	return "hugetlb"
 }
 
-func (s *HugetlbGroup) Apply(d *cgroupData) error {
-	_, err := d.join("hugetlb")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *HugetlbGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *HugetlbGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go
index f7b8f999e..32655cc3a 100644
--- a/libcontainer/cgroups/fs/memory.go
+++ b/libcontainer/cgroups/fs/memory.go
@@ -37,11 +37,8 @@ func (s *MemoryGroup) Name() string {
 	return "memory"
 }
 
-func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
-	path, err := d.path("memory")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	} else if path == "" {
+func (s *MemoryGroup) Apply(path string, d *cgroupData) (err error) {
+	if path == "" {
 		return nil
 	}
 	if memoryAssigned(d.config) {
@@ -66,11 +63,7 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
 
 	// We need to join memory cgroup after set memory limits, because
 	// kmem.limit_in_bytes can only be set when the cgroup is empty.
-	_, err = d.join("memory")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+	return join(path, d.pid)
 }
 
 func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/name.go b/libcontainer/cgroups/fs/name.go
index d8cf1d87c..d30e75149 100644
--- a/libcontainer/cgroups/fs/name.go
+++ b/libcontainer/cgroups/fs/name.go
@@ -16,10 +16,10 @@ func (s *NameGroup) Name() string {
 	return s.GroupName
 }
 
-func (s *NameGroup) Apply(d *cgroupData) error {
+func (s *NameGroup) Apply(path string, d *cgroupData) error {
 	if s.Join {
 		// ignore errors if the named cgroup does not exist
-		d.join(s.GroupName)
+		join(path, d.pid)
 	}
 	return nil
 }
diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go
index 021201525..b8723b3f1 100644
--- a/libcontainer/cgroups/fs/net_cls.go
+++ b/libcontainer/cgroups/fs/net_cls.go
@@ -17,12 +17,8 @@ func (s *NetClsGroup) Name() string {
 	return "net_cls"
 }
 
-func (s *NetClsGroup) Apply(d *cgroupData) error {
-	_, err := d.join("net_cls")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *NetClsGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *NetClsGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go
index 2bdeedf80..57fbeff1f 100644
--- a/libcontainer/cgroups/fs/net_prio.go
+++ b/libcontainer/cgroups/fs/net_prio.go
@@ -15,12 +15,8 @@ func (s *NetPrioGroup) Name() string {
 	return "net_prio"
 }
 
-func (s *NetPrioGroup) Apply(d *cgroupData) error {
-	_, err := d.join("net_prio")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *NetPrioGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *NetPrioGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go
index 5693676d3..02dc04f0e 100644
--- a/libcontainer/cgroups/fs/perf_event.go
+++ b/libcontainer/cgroups/fs/perf_event.go
@@ -14,12 +14,8 @@ func (s *PerfEventGroup) Name() string {
 	return "perf_event"
 }
 
-func (s *PerfEventGroup) Apply(d *cgroupData) error {
-	// we just want to join this group even though we don't set anything
-	if _, err := d.join("perf_event"); err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *PerfEventGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *PerfEventGroup) Set(path string, cgroup *configs.Cgroup) error {
diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go
index 7bf680135..6b7a03c83 100644
--- a/libcontainer/cgroups/fs/pids.go
+++ b/libcontainer/cgroups/fs/pids.go
@@ -19,12 +19,8 @@ func (s *PidsGroup) Name() string {
 	return "pids"
 }
 
-func (s *PidsGroup) Apply(d *cgroupData) error {
-	_, err := d.join("pids")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *PidsGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *PidsGroup) Set(path string, cgroup *configs.Cgroup) error {