From 405e88bd1b75e618ec1f336fdf8e3cef2de6f277 Mon Sep 17 00:00:00 2001
From: modeco80 <lily.modeco80@protonmail.ch>
Date: Sun, 3 Nov 2024 13:10:08 -0500
Subject: [PATCH] cvmts: Allow specifying cgroup cpu period

probably limited utility wise but it's there now
---
 config.example.toml           | 23 ++++++++++++++++-------
 cvmts/src/util/cgroup.ts      | 10 +++++-----
 cvmts/src/vm/qemu_launcher.ts | 10 +++++++++-
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/config.example.toml b/config.example.toml
index b7d8d48..2506247 100644
--- a/config.example.toml
+++ b/config.example.toml
@@ -57,20 +57,29 @@ qemuArgs = "qemu-system-x86_64"
 vncPort = 5900
 snapshots = true
 
-# Resource limits. Only works on Linux, with `Delegate=yes` set in your .service file. No-op on other platforms.
+# Resource limits. 
+# Only works on Linux, with `Delegate=yes` set in your .service file. No-op on other platforms.
 #
-# cpuUsageMax optionally specifies the max CPU usage as percentage in the common top notation, so 200% means 2 CPUs, 400% is 4 CPUs, 
-# so on.
+# cpuUsageMax is an optional value specifies the max CPU usage as percentage in the common top notation. 
+# 200% means 2 CPUs, 400% is 4 CPUs.
+#
+# A general reccomendation is to set this to 100*[vCPU count].
+# For example, if your QEMU command line contains -smp cores=2, then cpuUsageMax should be 200.
+# For an overcomitted host, you can use lower values, 
+# but it *can* get noticable if you throttle CPU too low.
 #
 # runOnCpus is an optional array that specifies what CPUs the VM is allowed to run on.
-# Systemd user slices can not delegate the cpuset controller, meaning this option will *not* work if you do not use a system service.
-# (effectively, it will be a loud no-op that logs an error on startup)
+# This option will *not* work if you do not use a system service. (effectively, it will be a loud no-op that logs an error on startup).
 #
-# limitProcess is optional (default false) and determines if only qemu vCPU threads are put into the cgroup,
+# periodMs is an optional value in milliseconds that specifies the cgroup's CPU accounting period.
+# The default is 100 ms (which matches the cgroups2 defaults), and should work in pretty much all cases, but
+# it's a knob provided for any addl. tuning purposes.
+#
+# limitProcess is an optional boolean (default false) that determines if only qemu vCPU threads are put into the cgroup,
 # or the entire QEMU process (incl. all its threads). The default behaviour of only limiting vCPU threads
 # is more than likely what you want, so the example configuration omits specifying this key.
 #
-# Commenting this inline table from the configuration disables resource limiting entirely.
+# Commenting or removing this table from the configuration disables resource limiting entirely.
 resourceLimits = { cpuUsageMax = 100, runOnCpus = [ 2, 4 ] }
 
 
diff --git a/cvmts/src/util/cgroup.ts b/cvmts/src/util/cgroup.ts
index 44783ce..7c97e89 100644
--- a/cvmts/src/util/cgroup.ts
+++ b/cvmts/src/util/cgroup.ts
@@ -19,8 +19,8 @@ export class CGroupController {
 	WriteValue(key: string, value: string) {
 		try {
 			writeFileSync(path.join(this.cg.Path(), `${this.controller}.${key}`), value);
-		} catch(e) {
-			logger.error({error: e, controller_key: `${this.controller}.${key}`, value: value }, 'Failed to set CGroup controller value')
+		} catch (e) {
+			logger.error({ error: e, controller_name: this.controller, controller_key: `${this.controller}.${key}`, value: value }, 'Failed to set CGroup controller value');
 		}
 	}
 }
@@ -35,11 +35,11 @@ export class CGroup {
 	InitControllers(wants_cpuset: boolean) {
 		// Configure this "root" cgroup to provide cpu and cpuset controllers to the leaf
 		// QEMU cgroups. A bit iffy but whatever.
-		if(wants_cpuset) {
+		if (wants_cpuset) {
 			try {
 				writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu +cpuset');
-			} catch(err) {
-				logger.error({error: err}, 'Could not provide cpuset controller to subtree');
+			} catch (err) {
+				logger.error({ error: err }, 'Could not provide cpuset controller to subtree. runOnCpus will not function.');
 				// just give up if this fails
 				writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu');
 			}
diff --git a/cvmts/src/vm/qemu_launcher.ts b/cvmts/src/vm/qemu_launcher.ts
index a0ff2d8..54134d2 100644
--- a/cvmts/src/vm/qemu_launcher.ts
+++ b/cvmts/src/vm/qemu_launcher.ts
@@ -7,6 +7,7 @@ import { CGroup } from '../util/cgroup.js';
 export interface CgroupLimits {
 	cpuUsageMax?: number;
 	runOnCpus?: number[];
+	periodMs?: number;
 	limitProcess?: boolean;
 }
 
@@ -19,12 +20,19 @@ interface CGroupValue {
 function MakeValuesFromLimits(limits: CgroupLimits): CGroupValue[] {
 	let option_array = [];
 
+	// The default period is 100 ms, which matches cgroups2 defaults.
+	let periodUs = 100 * 1000;
+
+	// Convert a user-configured period to us, since that's what cgroups2 expects.
+	if(limits.periodMs)
+		periodUs = limits.periodMs * 1000;
+
 	if (limits.cpuUsageMax) {
 		// cpu.max
 		option_array.push({
 			controller: 'cpu',
 			key: 'max',
-			value: `${(limits.cpuUsageMax / 100) * 100000} 100000`
+			value: `${(limits.cpuUsageMax / 100) * periodUs} ${periodUs}`
 		});
 	}