Merge pull request #320 from utam0k/refactoring/introduce-lifetime-to…

…-ControllerOpt Avoid cloning LinuxResources because it is a large structure.
youki-dev · Sep 22, 2021 · 9f669d7 · 9f669d7
2 parents 4bab88d + 8023ff4
commit 9f669d7
Show file tree

Hide file tree

Showing 19 changed files with 64 additions and 47 deletions.
diff --git a/cgroups/src/common.rs b/cgroups/src/common.rs
@@ -72,10 +72,10 @@ pub enum FreezerState {
 }
 
 /// ControllerOpt is given all cgroup controller for applying cgroup configuration.
-#[derive(Clone, Debug, Default)]
-pub struct ControllerOpt {
+#[derive(Clone, Debug)]
+pub struct ControllerOpt<'a> {
     /// Resources contain cgroup information for handling resource constraints for the container.
-    pub resources: LinuxResources,
+    pub resources: &'a LinuxResources,
     /// Disables the OOM killer for out of memory conditions.
     pub disable_oom_killer: bool,
     /// Specify an oom_score_adj for container.

diff --git a/cgroups/src/v1/blkio.rs b/cgroups/src/v1/blkio.rs
@@ -84,7 +84,7 @@ impl Controller for Blkio {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(blkio) = &controller_opt.resources.block_io {
             return Some(blkio);
         }

diff --git a/cgroups/src/v1/controller.rs b/cgroups/src/v1/controller.rs
@@ -19,5 +19,5 @@ pub trait Controller {
     fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()>;
 
     /// Checks if the controller needs to handle this request
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource>;
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource>;
 }
diff --git a/cgroups/src/v1/cpu.rs b/cgroups/src/v1/cpu.rs
@@ -32,7 +32,7 @@ impl Controller for Cpu {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(cpu) = &controller_opt.resources.cpu {
             if cpu.shares.is_some()
                 || cpu.period.is_some()

diff --git a/cgroups/src/v1/cpuacct.rs b/cgroups/src/v1/cpuacct.rs
@@ -27,7 +27,7 @@ impl Controller for CpuAcct {
         Ok(())
     }
 
-    fn needs_to_handle(_controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(_controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         None
     }
 }

diff --git a/cgroups/src/v1/cpuset.rs b/cgroups/src/v1/cpuset.rs
@@ -38,7 +38,7 @@ impl Controller for CpuSet {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(cpuset) = &controller_opt.resources.cpu {
             if cpuset.cpus.is_some() || cpuset.mems.is_some() {
                 return Some(cpuset);

diff --git a/cgroups/src/v1/devices.rs b/cgroups/src/v1/devices.rs
@@ -33,7 +33,7 @@ impl Controller for Devices {
     }
 
     // always needs to be called due to default devices
-    fn needs_to_handle(_controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(_controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         Some(&())
     }
 }

diff --git a/cgroups/src/v1/freezer.rs b/cgroups/src/v1/freezer.rs
@@ -32,8 +32,8 @@ impl Controller for Freezer {
         Ok(())
     }
 
-    fn needs_to_handle(controller: &ControllerOpt) -> Option<&Self::Resource> {
-        if let Some(freezer_state) = &controller.freezer_state {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
+        if let Some(freezer_state) = &controller_opt.freezer_state {
             return Some(freezer_state);
         }
 
@@ -190,9 +190,10 @@ mod tests {
             let state = FreezerState::Thawed;
 
             let controller_opt = ControllerOpt {
-                resources: linux_resources,
+                resources: &linux_resources,
                 freezer_state: Some(state),
-                ..Default::default()
+                oom_score_adj: None,
+                disable_oom_killer: false,
             };
 
             let pid = Pid::from_raw(1000);
@@ -223,9 +224,10 @@ mod tests {
             let state = FreezerState::Frozen;
 
             let controller_opt = ControllerOpt {
-                resources: linux_resources,
+                resources: &linux_resources,
                 freezer_state: Some(state),
-                ..Default::default()
+                oom_score_adj: None,
+                disable_oom_killer: false,
             };
 
             let pid = Pid::from_raw(1001);
@@ -256,9 +258,10 @@ mod tests {
             let state = FreezerState::Undefined;
 
             let controller_opt = ControllerOpt {
-                resources: linux_resources,
+                resources: &linux_resources,
                 freezer_state: Some(state),
-                ..Default::default()
+                oom_score_adj: None,
+                disable_oom_killer: false,
             };
 
             let pid = Pid::from_raw(1002);

diff --git a/cgroups/src/v1/hugetlb.rs b/cgroups/src/v1/hugetlb.rs
@@ -28,7 +28,7 @@ impl Controller for HugeTlb {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(hugepage_limits) = controller_opt.resources.hugepage_limits.as_ref() {
             if !hugepage_limits.is_empty() {
                 return controller_opt.resources.hugepage_limits.as_ref();

diff --git a/cgroups/src/v1/manager.rs b/cgroups/src/v1/manager.rs
@@ -170,9 +170,10 @@ impl CgroupManager for Manager {
 
     fn freeze(&self, state: FreezerState) -> Result<()> {
         let controller_opt = ControllerOpt {
-            resources: Default::default(),
+            resources: &Default::default(),
             freezer_state: Some(state),
-            ..Default::default()
+            oom_score_adj: None,
+            disable_oom_killer: false,
         };
         Freezer::apply(
             &controller_opt,

diff --git a/cgroups/src/v1/memory.rs b/cgroups/src/v1/memory.rs
@@ -101,7 +101,7 @@ impl Controller for Memory {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(memory) = &controller_opt.resources.memory {
             return Some(memory);
         }
@@ -455,9 +455,10 @@ mod tests {
             };
 
         let controller_opt = ControllerOpt {
-            resources: linux_resources,
+            resources: &linux_resources,
             disable_oom_killer,
-            ..Default::default()
+            oom_score_adj: None,
+            freezer_state: None,
         };
 
             let result = <Memory as Controller>::apply(&controller_opt, &tmp);

diff --git a/cgroups/src/v1/network_classifier.rs b/cgroups/src/v1/network_classifier.rs
@@ -22,7 +22,7 @@ impl Controller for NetworkClassifier {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(network) = controller_opt.resources.network.as_ref() {
             return Some(network);
         }

diff --git a/cgroups/src/v1/network_priority.rs b/cgroups/src/v1/network_priority.rs
@@ -22,7 +22,7 @@ impl Controller for NetworkPriority {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(network) = &controller_opt.resources.network {
             return Some(network);
         }

diff --git a/cgroups/src/v1/perf_event.rs b/cgroups/src/v1/perf_event.rs
@@ -12,7 +12,7 @@ impl Controller for PerfEvent {
         Ok(())
     }
     //no need to handle any case
-    fn needs_to_handle(_controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(_controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         None
     }
 }

diff --git a/cgroups/src/v1/pids.rs b/cgroups/src/v1/pids.rs
@@ -27,7 +27,7 @@ impl Controller for Pids {
         Ok(())
     }
 
-    fn needs_to_handle(controller_opt: &ControllerOpt) -> Option<&Self::Resource> {
+    fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> {
         if let Some(pids) = &controller_opt.resources.pids {
             return Some(pids);
         }

diff --git a/cgroups/src/v2/manager.rs b/cgroups/src/v2/manager.rs
@@ -128,9 +128,10 @@ impl CgroupManager for Manager {
 
     fn freeze(&self, state: FreezerState) -> Result<()> {
         let controller_opt = ControllerOpt {
-            resources: Default::default(),
+            resources: &Default::default(),
             freezer_state: Some(state),
-            ..Default::default()
+            oom_score_adj: None,
+            disable_oom_killer: false,
         };
         Freezer::apply(&controller_opt, &self.full_path)
     }

diff --git a/cgroups/src/v2/systemd_manager.rs b/cgroups/src/v2/systemd_manager.rs
@@ -249,9 +249,10 @@ impl CgroupManager for SystemDCGroupManager {
 
     fn freeze(&self, state: FreezerState) -> Result<()> {
         let controller_opt = ControllerOpt {
-            resources: Default::default(),
+            resources: &Default::default(),
             freezer_state: Some(state),
-            ..Default::default()
+            oom_score_adj: None,
+            disable_oom_killer: false,
         };
         Freezer::apply(&controller_opt, &self.full_path)
     }

diff --git a/cgroups/src/v2/unified.rs b/cgroups/src/v2/unified.rs
@@ -77,8 +77,10 @@ mod tests {
         };
 
         let controller_opt = ControllerOpt {
-            resources,
-            ..Default::default()
+            resources: &resources,
+            freezer_state: None,
+            oom_score_adj: None,
+            disable_oom_killer: false,
         };
 
         // act
@@ -112,8 +114,10 @@ mod tests {
         };
 
         let controller_opt = ControllerOpt {
-            resources,
-            ..Default::default()
+            resources: &resources,
+            freezer_state: None,
+            oom_score_adj: None,
+            disable_oom_killer: false,
         };
 
         // act
@@ -143,8 +147,10 @@ mod tests {
         };
 
         let controller_opt = ControllerOpt {
-            resources,
-            ..Default::default()
+            resources: &resources,
+            oom_score_adj: None,
+            disable_oom_killer: false,
+            freezer_state: None,
         };
 
         // act

diff --git a/src/container/builder_impl.rs b/src/container/builder_impl.rs
@@ -150,17 +150,21 @@ impl<'a> ContainerBuilderImpl<'a> {
         log::debug!("init pid is {:?}", init_pid);
 
         if self.rootless.is_none() && linux.resources.is_some() && self.init {
-            let controller_opt = cgroups::common::ControllerOpt {
-                resources: linux.resources.clone().unwrap(),
-                ..Default::default()
-            };
-            cmanager
-                .add_task(init_pid)
-                .context("Failed to add tasks to cgroup manager")?;
+            if let Some(resources) = linux.resources.as_ref() {
+                let controller_opt = cgroups::common::ControllerOpt {
+                    resources,
+                    freezer_state: None,
+                    oom_score_adj: None,
+                    disable_oom_killer: false,
+                };
+                cmanager
+                    .add_task(init_pid)
+                    .context("Failed to add tasks to cgroup manager")?;
 
-            cmanager
-                .apply(&controller_opt)
-                .context("Failed to apply resource limits through cgroup")?;
+                cmanager
+                    .apply(&controller_opt)
+                    .context("Failed to apply resource limits through cgroup")?;
+            }
         }
 
         // if file to write the pid to is specified, write pid of the child