diff --git a/plus/bencher_runner/src/config.rs b/plus/bencher_runner/src/config.rs index fc92156b2..9a445747a 100644 --- a/plus/bencher_runner/src/config.rs +++ b/plus/bencher_runner/src/config.rs @@ -175,8 +175,26 @@ fn default_disk() -> Disk { DEFAULT_DISK } +/// Default guest kernel command line. +/// +/// Shared determinism params on both architectures: `norandmaps` disables +/// guest ASLR (opt-out parity with the host `--aslr` flag via the +/// user-overridable `kernel_cmdline`), `nokaslr` disables guest KASLR +/// (every run boots a fresh VM, so KASLR is per-run variance unique to +/// the sandbox path), and `transparent_hugepage=never` makes guest memory +/// backing deterministic. +/// +/// Arch differences: `reboot=t` (triple fault) is x86-only; aarch64 uses +/// `reboot=k` and needs `keep_bootcon` for console output during boot. fn default_kernel_cmdline() -> String { - "console=ttyS0 reboot=t panic=1 pci=off root=/dev/vda rw init=/init".to_owned() + #[cfg(target_arch = "aarch64")] + { + "keep_bootcon console=ttyS0 reboot=k panic=1 pci=off root=/dev/vda rw init=/init norandmaps nokaslr transparent_hugepage=never".to_owned() + } + #[cfg(not(target_arch = "aarch64"))] + { + "console=ttyS0 reboot=t panic=1 pci=off root=/dev/vda rw init=/init norandmaps nokaslr transparent_hugepage=never".to_owned() + } } const fn default_timeout_secs() -> u64 { @@ -558,4 +576,36 @@ mod tests { fn config_with_empty_token_errors() { Config::new("img").with_token("").unwrap_err(); } + + #[test] + fn default_kernel_cmdline_guest_determinism() { + let config = Config::new("img"); + assert!(config.kernel_cmdline.contains("norandmaps")); + assert!(config.kernel_cmdline.contains("nokaslr")); + assert!(config.kernel_cmdline.contains("transparent_hugepage=never")); + assert!(config.kernel_cmdline.contains("init=/init")); + } + + #[cfg(target_arch = "aarch64")] + #[test] + fn default_kernel_cmdline_aarch64() { + let config = Config::new("img"); + assert!(config.kernel_cmdline.starts_with("keep_bootcon ")); + assert!(config.kernel_cmdline.contains("reboot=k")); + assert!(!config.kernel_cmdline.contains("reboot=t")); + } + + #[cfg(not(target_arch = "aarch64"))] + #[test] + fn default_kernel_cmdline_x86() { + let config = Config::new("img"); + assert!(config.kernel_cmdline.contains("reboot=t")); + assert!(!config.kernel_cmdline.contains("keep_bootcon")); + } + + #[test] + fn kernel_cmdline_override_preserved() { + let config = Config::new("img").with_kernel_cmdline("console=ttyS0 custom=1"); + assert_eq!(config.kernel_cmdline, "console=ttyS0 custom=1"); + } } diff --git a/plus/bencher_runner/src/cpu.rs b/plus/bencher_runner/src/cpu.rs index c0fdbc4cb..7d0697985 100644 --- a/plus/bencher_runner/src/cpu.rs +++ b/plus/bencher_runner/src/cpu.rs @@ -33,21 +33,37 @@ impl CpuLayout { /// Detect available CPUs and create a layout. /// /// Reserves 1-2 cores for housekeeping (depending on total core count) - /// and assigns the rest to benchmarks. + /// and assigns the rest to benchmarks. On Linux this reads the actual + /// online CPU IDs rather than a count: disabling SMT on topologies with + /// interleaved sibling numbering (common on AMD) leaves a non-contiguous + /// online set like `0,2,4,6`, and a count-based layout would pin to + /// offline cores. #[must_use] pub fn detect() -> Self { - let num_cores = Self::available_cores(); - Self::with_core_count(num_cores) + #[cfg(target_os = "linux")] + if let Ok(online) = fs::read_to_string("/sys/devices/system/cpu/online") + && let Some(ids) = parse_cpu_id_list(&online) + && !ids.is_empty() + { + return Self::with_cpu_ids(ids); + } + + Self::with_core_count(Self::available_cores()) } - /// Create a layout for a given number of cores. + /// Create a layout from an explicit list of online CPU IDs. /// - /// - 1 core: housekeeping = [0], benchmark = [0] (shared, no isolation) - /// - 2-7 cores: housekeeping = [0], benchmark = [1..n] - /// - 8+ cores: housekeeping = [0, 1], benchmark = [2..n] + /// IDs need not be contiguous. The lowest 1-2 IDs become housekeeping + /// cores and the rest benchmark cores: + /// - 1 ID: housekeeping and benchmark share it (no isolation) + /// - 2-7 IDs: housekeeping = first, benchmark = rest + /// - 8+ IDs: housekeeping = first two, benchmark = rest #[must_use] - pub fn with_core_count(num_cores: usize) -> Self { - if num_cores == 0 { + pub fn with_cpu_ids(mut ids: Vec) -> Self { + ids.sort_unstable(); + ids.dedup(); + + if ids.is_empty() { // Fallback for edge cases return Self { housekeeping: vec![0], @@ -55,27 +71,33 @@ impl CpuLayout { }; } - if num_cores == 1 { + if ids.len() == 1 { // Single core - no isolation possible return Self { - housekeeping: vec![0], - benchmark: vec![0], + housekeeping: ids.clone(), + benchmark: ids, }; } // Reserve 1 core for housekeeping on small systems, 2 on larger ones - let housekeeping_count = if num_cores >= 8 { 2 } else { 1 }; - - let housekeeping: Vec = (0..housekeeping_count).collect(); - let benchmark: Vec = (housekeeping_count..num_cores).collect(); + let housekeeping_count = if ids.len() >= 8 { 2 } else { 1 }; + let benchmark = ids.split_off(housekeeping_count); Self { - housekeeping, + housekeeping: ids, benchmark, } } + /// Create a layout for a given number of contiguous cores `0..n`. + #[must_use] + pub fn with_core_count(num_cores: usize) -> Self { + Self::with_cpu_ids((0..num_cores).collect()) + } + /// Get the number of available CPU cores. + /// + /// Count-based fallback for when the online CPU ID list is unavailable. #[cfg(target_os = "linux")] #[expect( clippy::cast_possible_truncation, @@ -83,14 +105,6 @@ impl CpuLayout { reason = "sysconf returns c_long; core count always fits in usize" )] fn available_cores() -> usize { - // Try reading from /sys/devices/system/cpu/online first - if let Ok(online) = fs::read_to_string("/sys/devices/system/cpu/online") - && let Some(count) = parse_cpu_list(&online) - { - return count; - } - - // Fallback to nix sysconf match nix::unistd::sysconf(nix::unistd::SysconfVar::_NPROCESSORS_ONLN) { Ok(Some(n)) if n > 0 => n as usize, _ => 1, // Ultimate fallback @@ -128,24 +142,25 @@ impl CpuLayout { } } -/// Parse a CPU list string like "0-3" or "0,2,4" or "0-3,8-11". -/// -/// Returns the total count of CPUs in the list. +/// Parse a kernel CPU list string like "0-3" or "0,2,4" or "0-3,8-11" +/// into the expanded list of CPU IDs. #[cfg(target_os = "linux")] -fn parse_cpu_list(s: &str) -> Option { - let mut count = 0; +fn parse_cpu_id_list(s: &str) -> Option> { + let mut ids = Vec::new(); for part in s.trim().split(',') { let part = part.trim(); if let Some((start, end)) = part.split_once('-') { let start: usize = start.trim().parse().ok()?; let end: usize = end.trim().parse().ok()?; - count += end - start + 1; + if end < start { + return None; + } + ids.extend(start..=end); } else { - let _: usize = part.parse().ok()?; - count += 1; + ids.push(part.parse().ok()?); } } - Some(count) + Some(ids) } /// Format a list of CPU IDs as a cpuset string. @@ -200,11 +215,60 @@ fn format_cpuset(cpus: &[usize]) -> String { result } +/// Format a list of CPU IDs as a kernel hex bitmask string. +/// +/// Produces the comma-separated 32-bit word format used by files like +/// `/proc/irq/default_smp_affinity` and +/// `/sys/devices/virtual/workqueue/cpumask`. Words are ordered most +/// significant first; all words after the first are zero-padded to +/// 8 hex digits (e.g., `[0, 1]` -> `"3"`, `[32]` -> `"1,00000000"`). +#[must_use] +#[expect( + clippy::integer_division, + reason = "dividing CPU IDs into 32-bit mask words" +)] +pub fn format_cpumask(cpus: &[usize]) -> String { + use std::fmt::Write as _; + + let Some(&max) = cpus.iter().max() else { + return "0".to_owned(); + }; + + let mut words = vec![0u32; max / 32 + 1]; + for &cpu in cpus { + if let Some(word) = words.get_mut(cpu / 32) { + *word |= 1u32 << (cpu % 32); + } + } + + let mut result = String::new(); + for (index, word) in words.iter().rev().enumerate() { + if index == 0 { + // write! to String is infallible + _ = write!(result, "{word:x}"); + } else { + _ = write!(result, ",{word:08x}"); + } + } + + result +} + /// Pin the current thread to the specified CPU cores. /// /// Uses `sched_setaffinity` on Linux via the `nix` crate. #[cfg(target_os = "linux")] pub fn pin_current_thread(cpus: &[usize]) -> io::Result<()> { + pin_tid(0, cpus) +} + +/// Pin a thread by its kernel thread ID (TID) to the specified CPU cores. +/// +/// Unlike [`pin_thread`], this works on threads of other processes +/// (e.g., Firecracker vCPU threads found under `/proc//task`). +/// A TID of 0 targets the calling thread. +#[cfg(target_os = "linux")] +pub fn pin_tid(tid: libc::pid_t, cpus: &[usize]) -> io::Result<()> { use nix::sched::{CpuSet, sched_setaffinity}; use nix::unistd::Pid; @@ -218,7 +282,7 @@ pub fn pin_current_thread(cpus: &[usize]) -> io::Result<()> { .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; } - sched_setaffinity(Pid::from_raw(0), &set).map_err(io::Error::other) + sched_setaffinity(Pid::from_raw(tid), &set).map_err(io::Error::other) } /// Pin the current thread to the specified CPU cores. @@ -309,6 +373,48 @@ mod tests { assert!(layout.has_isolation()); } + #[test] + fn layout_interleaved_smt_offline() { + // SMT disabled on an interleaved-sibling topology (e.g., AMD): + // only the even-numbered cores remain online. + let layout = CpuLayout::with_cpu_ids(vec![0, 2, 4, 6]); + assert_eq!(layout.housekeeping, vec![0]); + assert_eq!(layout.benchmark, vec![2, 4, 6]); + assert!(layout.has_isolation()); + assert_eq!(layout.benchmark_cpuset(), "2,4,6"); + } + + #[test] + fn layout_interleaved_eight_ids() { + let layout = CpuLayout::with_cpu_ids(vec![0, 2, 4, 6, 8, 10, 12, 14]); + assert_eq!(layout.housekeeping, vec![0, 2]); + assert_eq!(layout.benchmark, vec![4, 6, 8, 10, 12, 14]); + assert!(layout.has_isolation()); + } + + #[test] + fn layout_ids_unsorted_and_duplicated() { + let layout = CpuLayout::with_cpu_ids(vec![6, 2, 0, 4, 2]); + assert_eq!(layout.housekeeping, vec![0]); + assert_eq!(layout.benchmark, vec![2, 4, 6]); + } + + #[test] + fn layout_single_nonzero_id() { + let layout = CpuLayout::with_cpu_ids(vec![5]); + assert_eq!(layout.housekeeping, vec![5]); + assert_eq!(layout.benchmark, vec![5]); + assert!(!layout.has_isolation()); + } + + #[test] + fn layout_empty_ids() { + let layout = CpuLayout::with_cpu_ids(Vec::new()); + assert_eq!(layout.housekeeping, vec![0]); + assert_eq!(layout.benchmark, vec![0]); + assert!(!layout.has_isolation()); + } + #[test] fn format_cpuset_empty() { assert_eq!(format_cpuset(&[]), ""); @@ -334,6 +440,46 @@ mod tests { assert_eq!(format_cpuset(&[5, 3, 4, 2]), "2-5"); } + #[test] + fn format_cpumask_empty() { + assert_eq!(format_cpumask(&[]), "0"); + } + + #[test] + fn format_cpumask_single() { + assert_eq!(format_cpumask(&[0]), "1"); + } + + #[test] + fn format_cpumask_low_cores() { + assert_eq!(format_cpumask(&[0, 1]), "3"); + } + + #[test] + fn format_cpumask_full_byte() { + assert_eq!(format_cpumask(&[0, 1, 2, 3, 4, 5, 6, 7]), "ff"); + } + + #[test] + fn format_cpumask_high_bit() { + assert_eq!(format_cpumask(&[31]), "80000000"); + } + + #[test] + fn format_cpumask_second_word() { + assert_eq!(format_cpumask(&[32]), "1,00000000"); + } + + #[test] + fn format_cpumask_spanning_words() { + assert_eq!(format_cpumask(&[0, 32, 33]), "3,00000001"); + } + + #[test] + fn format_cpumask_third_word() { + assert_eq!(format_cpumask(&[64, 1]), "1,00000000,00000002"); + } + #[test] fn benchmark_cpuset_string() { let layout = CpuLayout::with_core_count(8); @@ -348,19 +494,29 @@ mod tests { #[cfg(target_os = "linux")] #[test] - fn parse_cpu_list_simple() { - assert_eq!(parse_cpu_list("0-3"), Some(4)); + fn parse_cpu_id_list_simple() { + assert_eq!(parse_cpu_id_list("0-3"), Some(vec![0, 1, 2, 3])); + } + + #[cfg(target_os = "linux")] + #[test] + fn parse_cpu_id_list_complex() { + assert_eq!( + parse_cpu_id_list("0-3,8-11\n"), + Some(vec![0, 1, 2, 3, 8, 9, 10, 11]) + ); } #[cfg(target_os = "linux")] #[test] - fn parse_cpu_list_complex() { - assert_eq!(parse_cpu_list("0-3,8-11"), Some(8)); + fn parse_cpu_id_list_individual() { + assert_eq!(parse_cpu_id_list("0,2,4"), Some(vec![0, 2, 4])); } #[cfg(target_os = "linux")] #[test] - fn parse_cpu_list_individual() { - assert_eq!(parse_cpu_list("0,2,4"), Some(3)); + fn parse_cpu_id_list_invalid() { + assert_eq!(parse_cpu_id_list("abc"), None); + assert_eq!(parse_cpu_id_list("3-1"), None); } } diff --git a/plus/bencher_runner/src/error.rs b/plus/bencher_runner/src/error.rs index 0862a007d..94072dc32 100644 --- a/plus/bencher_runner/src/error.rs +++ b/plus/bencher_runner/src/error.rs @@ -55,6 +55,9 @@ pub enum JailError { path: Utf8PathBuf, source: std::io::Error, }, + + #[error("Cpuset partition mode '{mode}' rejected by the kernel: {state}")] + PartitionInvalid { mode: String, state: String }, } #[derive(Debug, Error)] diff --git a/plus/bencher_runner/src/firecracker/mod.rs b/plus/bencher_runner/src/firecracker/mod.rs index 0044233a7..fdbdee111 100644 --- a/plus/bencher_runner/src/firecracker/mod.rs +++ b/plus/bencher_runner/src/firecracker/mod.rs @@ -14,6 +14,7 @@ mod client; pub mod config; pub mod error; +mod pin; mod process; mod vsock; @@ -121,6 +122,10 @@ pub fn run_firecracker( layout.benchmark_cpuset() ); } + // Keep VM memory resident: swap adds run-to-run variance + if let Err(e) = cg.disable_swap() { + eprintln!("Warning: failed to disable swap for VM cgroup: {e}"); + } Some(cg) }, Err(e) => { @@ -195,6 +200,15 @@ pub fn run_firecracker( action_type: ActionType::InstanceStart, })?; + // Pin vCPU threads (spawned during InstanceStart) to dedicated + // benchmark cores. Required companion to the isolated cpuset + // partition, which has no load balancing between cores. + if let Some(layout) = &config.cpu_layout + && layout.has_isolation() + { + pin::pin_vcpu_threads(fc_process.pid(), layout, config.vcpus); + } + // Step 5: Collect results via vsock let timeout = if config.timeout_secs > 0 { Duration::from_secs(config.timeout_secs) @@ -218,7 +232,9 @@ pub fn run_firecracker( wall_clock_ms: u64::try_from(elapsed.as_millis()).unwrap_or(u64::MAX), timed_out: matches!(e, FirecrackerError::Timeout(_)), transport: "vsock".to_owned(), - cgroup: None, + cgroup: cgroup + .as_ref() + .and_then(|cg| metrics::read_cgroup_metrics(cg.path())), }; if let Some(line) = metrics::format_metrics(&run_metrics) { eprintln!("{line}"); @@ -234,7 +250,9 @@ pub fn run_firecracker( wall_clock_ms: u64::try_from(elapsed.as_millis()).unwrap_or(u64::MAX), timed_out: false, transport: "vsock".to_owned(), - cgroup: None, + cgroup: cgroup + .as_ref() + .and_then(|cg| metrics::read_cgroup_metrics(cg.path())), }; if let Some(line) = metrics::format_metrics(&run_metrics) { eprintln!("{line}"); diff --git a/plus/bencher_runner/src/firecracker/pin.rs b/plus/bencher_runner/src/firecracker/pin.rs new file mode 100644 index 000000000..10911f6df --- /dev/null +++ b/plus/bencher_runner/src/firecracker/pin.rs @@ -0,0 +1,193 @@ +//! Pin Firecracker threads to dedicated benchmark cores. +//! +//! Inside an isolated cpuset partition there is no load balancing, and +//! even without one, vCPU threads migrating between benchmark cores adds +//! run-to-run variance (cold caches, TLB refills). Firecracker names its +//! vCPU threads `fc_vcpu {index}`; each is pinned to its own benchmark +//! core, and the remaining VMM/API threads are pinned to the last +//! benchmark core (housekeeping cores are outside the VM cgroup cpuset, +//! so pinning there would fail with EINVAL). All of this is best-effort +//! with warnings. + +use std::time::{Duration, Instant}; + +use camino::{Utf8Path, Utf8PathBuf}; + +use crate::cpu::{CpuLayout, pin_tid}; + +/// How long to wait for all vCPU threads to appear after `InstanceStart`. +const DISCOVERY_TIMEOUT: Duration = Duration::from_millis(500); + +/// Poll interval while waiting for vCPU threads. +const POLL_INTERVAL: Duration = Duration::from_millis(20); + +/// Pin the Firecracker process's threads to dedicated benchmark cores. +/// +/// Polls `/proc//task/*/comm` until `vcpu_count` vCPU threads appear +/// (they are spawned during `InstanceStart`) or the discovery timeout +/// elapses, then pins whatever was found. +/// +/// vCPU threads necessarily run for a moment before being pinned (they +/// do not exist until `InstanceStart`). The cgroup cpuset is the hard +/// confinement to benchmark cores; per-thread pinning is a refinement +/// on top of it that stops migration between those cores. +pub(super) fn pin_vcpu_threads(fc_pid: u32, layout: &CpuLayout, vcpu_count: u8) { + let task_dir = Utf8PathBuf::from(format!("/proc/{fc_pid}/task")); + let deadline = Instant::now() + DISCOVERY_TIMEOUT; + + let tasks = loop { + let tasks = read_tasks(&task_dir); + let vcpus_found = tasks + .iter() + .filter(|(_, comm)| parse_vcpu_comm(comm).is_some()) + .count(); + if vcpus_found >= usize::from(vcpu_count) || Instant::now() >= deadline { + break tasks; + } + std::thread::sleep(POLL_INTERVAL); + }; + + if tasks.is_empty() { + eprintln!("Warning: found no Firecracker threads to pin under {task_dir}"); + return; + } + + let mut pinned_vcpus = 0usize; + for (tid, comm) in &tasks { + let Some(core) = assign_core(comm, &layout.benchmark) else { + continue; + }; + match pin_tid(*tid, &[core]) { + Ok(()) => { + if parse_vcpu_comm(comm).is_some() { + pinned_vcpus += 1; + } + }, + Err(e) => { + eprintln!("Warning: failed to pin thread '{comm}' (tid {tid}) to core {core}: {e}"); + }, + } + } + + println!( + "CPU isolation: pinned {pinned_vcpus} of {vcpu_count} vCPU threads to dedicated cores" + ); +} + +/// Read all (tid, comm) pairs under a `/proc//task` directory. +/// +/// Threads that exit mid-scan are silently skipped. +fn read_tasks(task_dir: &Utf8Path) -> Vec<(libc::pid_t, String)> { + let Ok(entries) = std::fs::read_dir(task_dir.as_std_path()) else { + return Vec::new(); + }; + + let mut tasks = Vec::new(); + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(tid) = name.to_str().and_then(|name| name.parse().ok()) else { + continue; + }; + let Ok(comm) = std::fs::read_to_string(entry.path().join("comm")) else { + continue; + }; + tasks.push((tid, comm.trim().to_owned())); + } + + tasks +} + +/// Parse a Firecracker vCPU thread name (`fc_vcpu {index}`) into its index. +fn parse_vcpu_comm(comm: &str) -> Option { + comm.trim().strip_prefix("fc_vcpu ")?.parse().ok() +} + +/// Choose the benchmark core for a Firecracker thread. +/// +/// vCPU `N` gets its own core (`benchmark[N % len]`); all other threads +/// (VMM, API server) share the last benchmark core, keeping them off the +/// cores running the earlier vCPUs. When the vCPU count equals the +/// benchmark core count, the highest-index vCPU therefore shares its +/// core with the VMM threads and sees more scheduling noise than its +/// peers; the lower-index vCPUs keep dedicated cores. +fn assign_core(comm: &str, benchmark: &[usize]) -> Option { + let &last = benchmark.last()?; + match parse_vcpu_comm(comm) { + Some(index) => benchmark.get(index % benchmark.len()).copied(), + None => Some(last), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_vcpu_comm() { + assert_eq!(parse_vcpu_comm("fc_vcpu 0"), Some(0)); + assert_eq!(parse_vcpu_comm("fc_vcpu 12"), Some(12)); + assert_eq!(parse_vcpu_comm("fc_vcpu 3\n"), Some(3)); + } + + #[test] + fn rejects_non_vcpu_comm() { + assert_eq!(parse_vcpu_comm("firecracker"), None); + assert_eq!(parse_vcpu_comm("fc_api"), None); + assert_eq!(parse_vcpu_comm("fc_vcpu"), None); + assert_eq!(parse_vcpu_comm("fc_vcpu x"), None); + assert_eq!(parse_vcpu_comm(""), None); + } + + #[test] + fn assigns_vcpus_to_dedicated_cores() { + let benchmark = vec![2, 3, 4, 5]; + assert_eq!(assign_core("fc_vcpu 0", &benchmark), Some(2)); + assert_eq!(assign_core("fc_vcpu 1", &benchmark), Some(3)); + assert_eq!(assign_core("fc_vcpu 3", &benchmark), Some(5)); + } + + #[test] + fn wraps_vcpus_beyond_core_count() { + let benchmark = vec![2, 3]; + assert_eq!(assign_core("fc_vcpu 2", &benchmark), Some(2)); + assert_eq!(assign_core("fc_vcpu 3", &benchmark), Some(3)); + } + + #[test] + fn assigns_vmm_threads_to_last_core() { + let benchmark = vec![2, 3, 4, 5]; + assert_eq!(assign_core("firecracker", &benchmark), Some(5)); + assert_eq!(assign_core("fc_api", &benchmark), Some(5)); + } + + #[test] + fn no_benchmark_cores_assigns_nothing() { + assert_eq!(assign_core("fc_vcpu 0", &[]), None); + assert_eq!(assign_core("firecracker", &[]), None); + } + + #[test] + fn reads_tasks_from_fake_proc_tree() { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + std::fs::create_dir_all(root.join("100")).unwrap(); + std::fs::create_dir_all(root.join("101")).unwrap(); + std::fs::write(root.join("100/comm"), "firecracker\n").unwrap(); + std::fs::write(root.join("101/comm"), "fc_vcpu 0\n").unwrap(); + + let mut tasks = read_tasks(&root); + tasks.sort_unstable(); + assert_eq!( + tasks, + vec![ + (100, "firecracker".to_owned()), + (101, "fc_vcpu 0".to_owned()) + ] + ); + } + + #[test] + fn read_tasks_missing_dir_is_empty() { + assert!(read_tasks(Utf8Path::new("/nonexistent/task")).is_empty()); + } +} diff --git a/plus/bencher_runner/src/jail/cgroup.rs b/plus/bencher_runner/src/jail/cgroup.rs index 25221d2ef..650130a18 100644 --- a/plus/bencher_runner/src/jail/cgroup.rs +++ b/plus/bencher_runner/src/jail/cgroup.rs @@ -37,11 +37,13 @@ impl CgroupManager { path: parent.clone(), source: e, })?; - - // Enable controllers in parent - Self::enable_controllers(&parent)?; } + // Enable controllers in the parent. Always attempted (idempotent): + // the parent may have been created without controllers, e.g. by + // BencherPartition at startup. + Self::enable_controllers(&parent)?; + // Create this run's cgroup if !cgroup_path.exists() { fs::create_dir_all(&cgroup_path).map_err(|e| JailError::CreateCgroup { @@ -59,36 +61,33 @@ impl CgroupManager { /// Enable controllers in a cgroup. /// /// Enables cpu, memory, and pids controllers (required), and io/cpuset controllers - /// (optional, for I/O throttling and CPU pinning). Returns an error if required - /// controllers cannot be enabled. + /// (optional, for I/O throttling and CPU pinning). The verification read is the + /// real gate: write failures are tolerated when the required controllers are + /// already enabled (e.g., pre-configured by an admin for an unprivileged runner). fn enable_controllers(path: &Utf8Path) -> Result<(), RunnerError> { let subtree_control = path.join("cgroup.subtree_control"); - // Try to enable all controllers at once (most efficient) - if fs::write(&subtree_control, "+cpu +memory +pids +io +cpuset").is_err() { - // Fall back to enabling without cpuset - if fs::write(&subtree_control, "+cpu +memory +pids +io").is_err() { - // Fall back to enabling required controllers without io or cpuset - fs::write(&subtree_control, "+cpu +memory +pids").map_err(|e| { - JailError::EnableControllers { - path: subtree_control.clone(), - source: e, - } - })?; - } - } + // Try to enable all controllers at once, falling back to smaller sets + let write_result = fs::write(&subtree_control, "+cpu +memory +pids +io +cpuset") + .or_else(|_| fs::write(&subtree_control, "+cpu +memory +pids +io")) + .or_else(|_| fs::write(&subtree_control, "+cpu +memory +pids")); // Verify that required controllers are enabled let enabled = fs::read_to_string(&subtree_control).unwrap_or_default(); - for required in ["cpu", "memory", "pids"] { - if !enabled.contains(required) { - return Err(JailError::MissingController { - controller: required.to_owned(), - path: subtree_control.clone(), - enabled: enabled.clone(), + if let Some(missing) = missing_required_controller(&enabled) { + return Err(match write_result { + Err(e) => JailError::EnableControllers { + path: subtree_control, + source: e, } - .into()); - } + .into(), + Ok(()) => JailError::MissingController { + controller: missing.to_owned(), + path: subtree_control, + enabled, + } + .into(), + }); } Ok(()) @@ -108,7 +107,7 @@ impl CgroupManager { // Disable swap to ensure benchmark memory measurements are accurate // and to prevent swap thrashing from affecting benchmark results. - drop(self.write_file("memory.swap.max", "0")); + drop(self.disable_swap()); } // OOM group kill: when the cgroup hits its memory limit, kill ALL processes @@ -234,6 +233,14 @@ impl CgroupManager { devices } + /// Disable swap for this cgroup. + /// + /// Keeps benchmark memory resident: swap thrashing adds run-to-run + /// variance and distorts memory measurements. + pub fn disable_swap(&self) -> Result<(), RunnerError> { + self.write_file("memory.swap.max", "0") + } + /// Add the current process to this cgroup. pub fn add_self(&self) -> Result<(), RunnerError> { let pid = std::process::id(); @@ -278,6 +285,227 @@ impl Drop for CgroupManager { } } +/// Manages the parent `bencher` cgroup as a cpuset scheduler partition. +/// +/// Turning `/sys/fs/cgroup/bencher` into an `isolated` cpuset partition +/// removes the benchmark cores from the root scheduling domain: the kernel +/// load balancer can no longer pull other tasks onto them. This is the +/// runtime equivalent of the `isolcpus=` boot argument. Per-run cgroups +/// created under the partition inherit it automatically. +/// +/// The partition must be a child of a valid partition root; the cgroup +/// root always qualifies, which is why this lives on the parent `bencher` +/// cgroup and not on a per-run child. +pub struct BencherPartition { + /// The parent `bencher` cgroup path. + path: Utf8PathBuf, + /// The cgroup v2 mount point. + root: Utf8PathBuf, +} + +/// Achieved cpuset partition level, in decreasing order of isolation. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PartitionLevel { + /// Scheduler domain isolation: no load balancing onto benchmark cores. + Isolated, + /// A separate scheduler domain with load balancing inside it. + Root, + /// A plain cgroup (no partition); cpuset pinning still applies. + Member, +} + +impl BencherPartition { + /// Create a partition manager for the given cgroup v2 mount point + /// (`/sys/fs/cgroup` in production; tests pass a tempdir tree). + #[must_use] + pub fn new(cgroup_root: &Utf8Path) -> Self { + Self { + path: cgroup_root.join(BENCHER_CGROUP_BASE), + root: cgroup_root.to_owned(), + } + } + + /// Turn the `bencher` cgroup into a cpuset partition on the benchmark + /// cores, verifying the kernel accepted it. + /// + /// Falls back `isolated` -> `root` -> `member` when the kernel reports + /// the partition as invalid (read-back verification). All writes are + /// recorded on the guard and restored in reverse order on drop. + /// Best-effort: any failure degrades to [`PartitionLevel::Member`], + /// which matches the behavior before partitions were introduced. + pub fn apply( + &self, + layout: &CpuLayout, + guard: &mut crate::tuning::TuningGuard, + ) -> PartitionLevel { + // The bencher cgroup only gets cpuset files once the cpuset + // controller is enabled in the root subtree. Additive and + // idempotent, so it is not saved for restore. + if let Err(e) = fs::write(self.root.join("cgroup.subtree_control"), "+cpuset") { + eprintln!("Warning: failed to enable cpuset controller in cgroup root: {e}"); + return PartitionLevel::Member; + } + + if !self.path.exists() + && let Err(e) = fs::create_dir_all(&self.path) + { + eprintln!("Warning: failed to create cgroup {}: {e}", self.path); + return PartitionLevel::Member; + } + + // A partition needs explicit cpus and mems. + if !save_and_write( + guard, + &self.path.join("cpuset.cpus"), + &layout.benchmark_cpuset(), + "bencher cpuset.cpus", + ) { + return PartitionLevel::Member; + } + if !save_and_write( + guard, + &self.path.join("cpuset.mems"), + "0", + "bencher cpuset.mems", + ) { + return PartitionLevel::Member; + } + + let partition_path = self.path.join("cpuset.cpus.partition"); + let original = match fs::read_to_string(&partition_path) { + Ok(value) => partition_mode_token(&value).to_owned(), + Err(e) => { + // Missing on kernels without cpuset partition support. + eprintln!("Warning: cpuset partitions unavailable ({partition_path}: {e})"); + return PartitionLevel::Member; + }, + }; + + // Save the restore entry before attempting any mode write: a write + // can succeed at the syscall level while the kernel rejects the + // partition in the read-back, and the file must still revert on + // drop. Restoring an unchanged value is a harmless no-op. + guard.save_restore( + partition_path.clone(), + original, + "bencher cpuset partition".to_owned(), + ); + + for (mode, level) in [ + ("isolated", PartitionLevel::Isolated), + ("root", PartitionLevel::Root), + ] { + match try_partition_mode(&partition_path, mode) { + Ok(()) => return level, + Err(e) => { + eprintln!("Warning: cpuset partition mode '{mode}' not achieved: {e}"); + }, + } + } + + PartitionLevel::Member + } +} + +impl std::fmt::Display for PartitionLevel { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Isolated => write!(f, "isolated"), + Self::Root => write!(f, "root"), + Self::Member => write!(f, "member"), + } + } +} + +/// Write a partition mode and verify the kernel accepted it. +/// +/// The kernel reports a rejected partition in the read-back value +/// (e.g., `isolated invalid (Cpu list in cpuset.cpus not exclusive)`), +/// so a successful write is not enough. +fn try_partition_mode(path: &Utf8Path, mode: &str) -> Result<(), JailError> { + fs::write(path, mode).map_err(|e| JailError::WriteCgroup { + path: path.to_owned(), + source: e, + })?; + + verify_partition_state(path, mode) +} + +/// Extract the mode token from a `cpuset.cpus.partition` read-back. +/// +/// A healthy partition reads back as just the mode (`member`, `root`, +/// `isolated`); a rejected one as ` invalid ()`. Only the +/// mode token is a valid value to write back on restore. An empty or +/// unreadable value falls back to the kernel default `member`. +fn partition_mode_token(state: &str) -> &str { + state.split_whitespace().next().unwrap_or("member") +} + +/// Read back a partition file and check the kernel reports exactly `mode`. +fn verify_partition_state(path: &Utf8Path, mode: &str) -> Result<(), JailError> { + let state = fs::read_to_string(path).map_err(|e| JailError::WriteCgroup { + path: path.to_owned(), + source: e, + })?; + let state = state.trim(); + + if state == mode { + Ok(()) + } else { + Err(JailError::PartitionInvalid { + mode: mode.to_owned(), + state: state.to_owned(), + }) + } +} + +/// Save the current value of a cgroup file on the guard, then write `value`. +/// +/// An empty current value is saved as a newline so the restore write is +/// not a zero-byte no-op (clearing `cpuset.cpus` requires writing `"\n"`). +/// Returns false (with a warning) when the file cannot be read or written. +fn save_and_write( + guard: &mut crate::tuning::TuningGuard, + path: &Utf8Path, + value: &str, + label: &str, +) -> bool { + let current = match fs::read_to_string(path) { + Ok(current) => current.trim().to_owned(), + Err(e) => { + eprintln!("Warning: failed to read {path}: {e}"); + return false; + }, + }; + + if current == value { + return true; + } + + if let Err(e) = fs::write(path, value) { + eprintln!("Warning: failed to write {path}: {e}"); + return false; + } + + let restore_value = if current.is_empty() { + "\n".to_owned() + } else { + current + }; + guard.save_restore(path.to_owned(), restore_value, label.to_owned()); + true +} + +/// Return the first required controller missing from a +/// `cgroup.subtree_control` listing, or `None` when all are enabled. +/// +/// Matches whole tokens: `cpuset` alone must not satisfy `cpu`. +fn missing_required_controller(enabled: &str) -> Option<&'static str> { + ["cpu", "memory", "pids"] + .into_iter() + .find(|required| !enabled.split_whitespace().any(|token| token == *required)) +} + /// Check if cgroup v2 is available. #[expect(dead_code, reason = "utility for future cgroup v2 feature detection")] #[must_use] @@ -286,3 +514,189 @@ pub fn is_cgroup_v2_available() -> bool { .join("cgroup.controllers") .exists() } + +#[cfg(test)] +mod tests { + use camino::Utf8PathBuf; + + use crate::cpu::CpuLayout; + use crate::tuning::{TuningConfig, TuningGuard}; + + use super::*; + + fn empty_guard() -> TuningGuard { + crate::tuning::apply(&TuningConfig::disabled()) + } + + /// A fake cgroup v2 tree mirroring what the kernel exposes. + fn fake_cgroup_root() -> (tempfile::TempDir, Utf8PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + + fs::write(root.join("cgroup.subtree_control"), "").unwrap(); + fs::create_dir_all(root.join("bencher")).unwrap(); + fs::write(root.join("bencher/cpuset.cpus"), "").unwrap(); + fs::write(root.join("bencher/cpuset.mems"), "").unwrap(); + fs::write(root.join("bencher/cpuset.cpus.partition"), "member\n").unwrap(); + + (dir, root) + } + + #[test] + fn partition_applies_isolated() { + let (_dir, root) = fake_cgroup_root(); + let layout = CpuLayout::with_core_count(8); + let mut guard = empty_guard(); + + let level = BencherPartition::new(&root).apply(&layout, &mut guard); + + assert_eq!(level, PartitionLevel::Isolated); + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.cpus")).unwrap(), + "2-7" + ); + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.mems")).unwrap(), + "0" + ); + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.cpus.partition")).unwrap(), + "isolated" + ); + } + + #[test] + fn partition_restores_on_guard_drop() { + let (_dir, root) = fake_cgroup_root(); + let layout = CpuLayout::with_core_count(8); + + { + let mut guard = empty_guard(); + let level = BencherPartition::new(&root).apply(&layout, &mut guard); + assert_eq!(level, PartitionLevel::Isolated); + } + + // Partition demoted back, cpus and mems cleared (newline write). + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.cpus.partition")).unwrap(), + "member" + ); + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.cpus")).unwrap(), + "\n" + ); + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.mems")).unwrap(), + "\n" + ); + } + + #[test] + fn partition_falls_back_to_member_when_unwritable() { + let (_dir, root) = fake_cgroup_root(); + // A directory in place of the partition file forces every mode + // write to fail, exercising the full fallback chain. + fs::remove_file(root.join("bencher/cpuset.cpus.partition")).unwrap(); + fs::create_dir_all(root.join("bencher/cpuset.cpus.partition")).unwrap(); + let layout = CpuLayout::with_core_count(8); + let mut guard = empty_guard(); + + let level = BencherPartition::new(&root).apply(&layout, &mut guard); + + assert_eq!(level, PartitionLevel::Member); + } + + #[test] + fn partition_member_without_cgroup_v2() { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + let layout = CpuLayout::with_core_count(8); + let mut guard = empty_guard(); + + let level = BencherPartition::new(&root).apply(&layout, &mut guard); + + assert_eq!(level, PartitionLevel::Member); + } + + #[test] + fn partition_restore_entry_saved_before_mode_writes() { + let (_dir, root) = fake_cgroup_root(); + // A previous run (or crash) left the partition file dirty: the + // kernel reports rejected partitions inline in the read-back. + fs::write( + root.join("bencher/cpuset.cpus.partition"), + "isolated invalid (Cpu list not exclusive)\n", + ) + .unwrap(); + let layout = CpuLayout::with_core_count(8); + + { + let mut guard = empty_guard(); + let level = BencherPartition::new(&root).apply(&layout, &mut guard); + assert_eq!(level, PartitionLevel::Isolated); + } + + // The restore writes back the normalized mode token, never the + // full dirty state (which is not a valid value to write). + assert_eq!( + fs::read_to_string(root.join("bencher/cpuset.cpus.partition")).unwrap(), + "isolated" + ); + } + + #[test] + fn partition_mode_token_normalizes_states() { + assert_eq!(partition_mode_token("member\n"), "member"); + assert_eq!(partition_mode_token("isolated\n"), "isolated"); + assert_eq!( + partition_mode_token("isolated invalid (Cpu list not exclusive)\n"), + "isolated" + ); + assert_eq!(partition_mode_token(""), "member"); + } + + #[test] + fn missing_required_controller_matches_whole_tokens() { + assert_eq!(missing_required_controller("cpu memory pids"), None); + assert_eq!( + missing_required_controller("cpuset cpu memory pids io"), + None + ); + assert_eq!(missing_required_controller(""), Some("cpu")); + // "cpuset" alone must not satisfy the "cpu" controller + assert_eq!( + missing_required_controller("cpuset memory pids"), + Some("cpu") + ); + assert_eq!(missing_required_controller("cpu memory"), Some("pids")); + } + + #[test] + fn verify_partition_state_accepts_exact_mode() { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + let path = root.join("cpuset.cpus.partition"); + fs::write(&path, "isolated\n").unwrap(); + + verify_partition_state(&path, "isolated").unwrap(); + } + + #[test] + fn verify_partition_state_rejects_invalid() { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + let path = root.join("cpuset.cpus.partition"); + // The kernel reports a rejected partition in the read-back value. + fs::write(&path, "isolated invalid (Cpu list not exclusive)\n").unwrap(); + + let err = verify_partition_state(&path, "isolated").unwrap_err(); + assert!(err.to_string().contains("invalid")); + } + + #[test] + fn partition_level_display() { + assert_eq!(PartitionLevel::Isolated.to_string(), "isolated"); + assert_eq!(PartitionLevel::Root.to_string(), "root"); + assert_eq!(PartitionLevel::Member.to_string(), "member"); + } +} diff --git a/plus/bencher_runner/src/jail/mod.rs b/plus/bencher_runner/src/jail/mod.rs index 0d29491f1..5e06ef811 100644 --- a/plus/bencher_runner/src/jail/mod.rs +++ b/plus/bencher_runner/src/jail/mod.rs @@ -7,7 +7,7 @@ mod cgroup; #[cfg(target_os = "linux")] -pub use cgroup::CgroupManager; +pub use cgroup::{BencherPartition, CgroupManager, PartitionLevel}; use serde::{Deserialize, Serialize}; diff --git a/plus/bencher_runner/src/lib.rs b/plus/bencher_runner/src/lib.rs index fd365c706..c9a95a2c5 100644 --- a/plus/bencher_runner/src/lib.rs +++ b/plus/bencher_runner/src/lib.rs @@ -37,6 +37,8 @@ pub mod kernel; #[cfg(feature = "plus")] mod local; #[cfg(feature = "plus")] +mod local_isolation; +#[cfg(feature = "plus")] mod log_level; #[cfg(feature = "plus")] pub mod metrics; @@ -64,4 +66,4 @@ pub use log_level::SandboxLogLevel; #[cfg(feature = "plus")] pub use run::{RunArgs, RunOutput, execute, resolve_oci_image, run_with_args}; #[cfg(feature = "plus")] -pub use tuning::{PerfEventParanoid, Swappiness, TuningConfig}; +pub use tuning::{ParseThpModeError, PerfEventParanoid, Swappiness, ThpMode, TuningConfig}; diff --git a/plus/bencher_runner/src/local.rs b/plus/bencher_runner/src/local.rs index 87e8eeda0..0c79a9418 100644 --- a/plus/bencher_runner/src/local.rs +++ b/plus/bencher_runner/src/local.rs @@ -26,6 +26,8 @@ use std::time::{Duration, Instant}; use camino::{Utf8Path, Utf8PathBuf}; use crate::error::RunnerError; +use crate::local_isolation::LocalIsolation; +use crate::metrics::{self, RunMetrics}; use crate::run::{RunOutput, prepare_oci_workspace}; /// Execute a single benchmark run locally on the host system. @@ -92,6 +94,15 @@ pub fn local_execute( }, }; + // Isolate the benchmark on the benchmark cores (cgroup cpuset with a + // CPU affinity fallback), mirroring the Firecracker path. Best-effort: + // failures degrade to no isolation with a warning. + let isolation = LocalIsolation::prepare(config.cpu_layout.as_ref()); + isolation.configure_command(&mut cmd); + + let start = Instant::now(); + // No metrics are emitted on spawn failure: nothing ran, so there is + // no wall clock or cgroup usage to report. let child = cmd .spawn() .map_err(|e| crate::error::ConfigError::BinaryNotFound { @@ -99,7 +110,18 @@ pub fn local_execute( hint: format!("Failed to spawn process: {e}"), })?; - let output = wait_with_timeout(child, config.timeout_secs, cancel_flag)?; + let output = match wait_with_timeout(child, config.timeout_secs, cancel_flag) { + Ok(output) => output, + Err(e) => { + let timed_out = matches!( + e, + RunnerError::Execution(crate::error::ExecutionError::Timeout(_)) + ); + emit_run_metrics(start.elapsed(), timed_out, &isolation); + return Err(e); + }, + }; + emit_run_metrics(start.elapsed(), false, &isolation); // Collect output files from the unpacked rootfs let output_files = collect_output_files( @@ -116,6 +138,22 @@ pub fn local_execute( }) } +/// Output run metrics to stderr in the standard marker format. +/// +/// Reads cgroup metrics before the isolation (and with it the cgroup) +/// is dropped. +fn emit_run_metrics(elapsed: Duration, timed_out: bool, isolation: &LocalIsolation) { + let run_metrics = RunMetrics { + wall_clock_ms: u64::try_from(elapsed.as_millis()).unwrap_or(u64::MAX), + timed_out, + transport: "local".to_owned(), + cgroup: isolation.read_metrics(), + }; + if let Some(line) = metrics::format_metrics(&run_metrics) { + eprintln!("{line}"); + } +} + /// Output from waiting on a child process. struct WaitOutput { exit_code: i32, diff --git a/plus/bencher_runner/src/local_isolation.rs b/plus/bencher_runner/src/local_isolation.rs new file mode 100644 index 000000000..6b04d13eb --- /dev/null +++ b/plus/bencher_runner/src/local_isolation.rs @@ -0,0 +1,247 @@ +//! CPU isolation for non-sandboxed (host) execution. +//! +//! Mirrors the Firecracker path: the benchmark child process is placed in +//! a per-run cgroup with a cpuset restricted to the benchmark cores, so it +//! does not share cores with the runner's own threads. Attachment happens +//! race-free in the child between `fork` and `exec` (via `pre_exec`), so +//! the benchmark never runs a single instruction on the wrong cores. +//! +//! Everything here is best-effort: without root or cgroup v2 the cgroup +//! steps are skipped with a warning and only CPU affinity is applied. + +#![cfg_attr( + target_os = "linux", + expect( + clippy::print_stdout, + clippy::print_stderr, + reason = "isolation setup prints status and best-effort warnings" + ) +)] + +use std::process::Command; + +use crate::cpu::CpuLayout; +use crate::metrics::CgroupMetrics; + +// --------------------------------------------------------------------------- +// Linux implementation +// --------------------------------------------------------------------------- + +/// CPU isolation state for a single non-sandboxed run. +#[cfg(target_os = "linux")] +pub(crate) struct LocalIsolation { + /// Per-run cgroup; `None` when cgroup setup failed or was skipped. + /// Dropping this removes the cgroup. + cgroup: Option, + /// Pre-opened `cgroup.procs` file, inherited by the child so it can + /// move itself into the cgroup before exec. + procs: Option, + /// Benchmark cores for the `sched_setaffinity` fallback. + benchmark: Vec, +} + +#[cfg(target_os = "linux")] +impl LocalIsolation { + /// Prepare CPU isolation for a non-sandboxed run. + /// + /// Never fails: on any setup error (non-root, no cgroup v2) it warns + /// and falls back to affinity-only isolation, or no isolation at all + /// when the layout has none. + pub(crate) fn prepare(layout: Option<&CpuLayout>) -> Self { + let Some(layout) = layout.filter(|layout| layout.has_isolation()) else { + return Self { + cgroup: None, + procs: None, + benchmark: Vec::new(), + }; + }; + + let benchmark = layout.benchmark.clone(); + let run_id = format!("local-{}", uuid::Uuid::new_v4()); + + let cgroup = match crate::jail::CgroupManager::new(&run_id) { + Ok(cgroup) => { + if let Err(e) = cgroup.apply_cpuset(layout) { + eprintln!("Warning: failed to apply cpuset for local run: {e}"); + } + // Keep benchmark memory resident, mirroring the Firecracker path + if let Err(e) = cgroup.disable_swap() { + eprintln!("Warning: failed to disable swap for local run: {e}"); + } + Some(cgroup) + }, + Err(e) => { + eprintln!( + "Warning: failed to create cgroup for local run (falling back to CPU affinity only): {e}" + ); + None + }, + }; + + let procs = cgroup.as_ref().and_then(|cgroup| { + match std::fs::OpenOptions::new() + .write(true) + .open(cgroup.path().join("cgroup.procs")) + { + Ok(file) => Some(file), + Err(e) => { + eprintln!("Warning: failed to open cgroup.procs for local run: {e}"); + None + }, + } + }); + + if procs.is_some() { + println!( + "CPU isolation: benchmark pinned to cores {}", + layout.benchmark_cpuset() + ); + } + + Self { + cgroup, + procs, + benchmark, + } + } + + /// Configure the benchmark command to attach itself to the cgroup (or + /// pin its CPU affinity) in the child, between `fork` and `exec`. + /// + /// Attaching in the child instead of after `spawn` avoids the race + /// where the benchmark runs startup work on the wrong cores. + pub(crate) fn configure_command(&self, cmd: &mut Command) { + use std::os::fd::AsRawFd; + use std::os::unix::process::CommandExt as _; + + let procs_fd = self.procs.as_ref().map(AsRawFd::as_raw_fd); + let benchmark = self.benchmark.clone(); + if procs_fd.is_none() && benchmark.is_empty() { + return; + } + + #[expect( + unsafe_code, + clippy::multiple_unsafe_ops_per_block, + reason = "pre_exec and the async-signal-safe syscalls in it require unsafe" + )] + // SAFETY: The pre_exec closure runs in the forked child before exec, + // so it may only use async-signal-safe operations. It performs no + // allocation: `benchmark` was moved in before the fork, and the + // closure only issues `write` and `sched_setaffinity` syscalls plus + // stack memory manipulation (CPU_ZERO / CPU_SET). `procs_fd` stays + // valid because `self.procs` outlives the spawn (the fd is inherited + // across fork). All failures are ignored: isolation is best-effort + // and must never prevent the benchmark from running. + unsafe { + cmd.pre_exec(move || { + if let Some(fd) = procs_fd { + // Writing "0" to cgroup.procs moves the writing task. + _ = libc::write(fd, b"0".as_ptr().cast(), 1); + } + if !benchmark.is_empty() { + let mut set: libc::cpu_set_t = std::mem::zeroed(); + libc::CPU_ZERO(&mut set); + for &cpu in &benchmark { + if cpu < libc::CPU_SETSIZE as usize { + libc::CPU_SET(cpu, &mut set); + } + } + _ = libc::sched_setaffinity(0, size_of::(), &raw const set); + } + Ok(()) + }); + } + } + + /// Read cgroup resource metrics for this run, if a cgroup was created. + /// + /// Must be called before the isolation is dropped (dropping removes + /// the cgroup). + pub(crate) fn read_metrics(&self) -> Option { + self.cgroup + .as_ref() + .and_then(|cgroup| crate::metrics::read_cgroup_metrics(cgroup.path())) + } +} + +// --------------------------------------------------------------------------- +// Non-Linux stub +// --------------------------------------------------------------------------- + +/// No-op isolation for non-Linux platforms. +#[cfg(not(target_os = "linux"))] +pub(crate) struct LocalIsolation; + +#[cfg(not(target_os = "linux"))] +#[expect( + clippy::unused_self, + reason = "non-Linux stub mirrors the Linux method signatures" +)] +impl LocalIsolation { + pub(crate) fn prepare(_layout: Option<&CpuLayout>) -> Self { + Self + } + + pub(crate) fn configure_command(&self, _cmd: &mut Command) {} + + pub(crate) fn read_metrics(&self) -> Option { + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn no_layout_is_noop() { + let isolation = LocalIsolation::prepare(None); + assert!(isolation.read_metrics().is_none()); + #[cfg(target_os = "linux")] + { + assert!(isolation.cgroup.is_none()); + assert!(isolation.procs.is_none()); + assert!(isolation.benchmark.is_empty()); + } + } + + #[test] + fn layout_without_isolation_is_noop() { + let layout = CpuLayout::with_core_count(1); + let isolation = LocalIsolation::prepare(Some(&layout)); + assert!(isolation.read_metrics().is_none()); + #[cfg(target_os = "linux")] + { + assert!(isolation.cgroup.is_none()); + assert!(isolation.benchmark.is_empty()); + } + } + + #[test] + fn noop_isolation_leaves_command_spawnable() { + let isolation = LocalIsolation::prepare(None); + let mut cmd = Command::new("echo"); + isolation.configure_command(&mut cmd); + let status = cmd + .stdout(std::process::Stdio::null()) + .status() + .expect("echo must spawn"); + assert!(status.success()); + } + + #[cfg(target_os = "linux")] + #[test] + fn configured_command_spawns_best_effort() { + // Cgroup creation may succeed (root CI) or fall back (unprivileged); + // either way the configured command must still spawn and succeed. + let layout = CpuLayout::with_core_count(2); + let isolation = LocalIsolation::prepare(Some(&layout)); + assert_eq!(isolation.benchmark, vec![1]); + + let mut cmd = Command::new("true"); + isolation.configure_command(&mut cmd); + let status = cmd.status().expect("true must spawn"); + assert!(status.success()); + } +} diff --git a/plus/bencher_runner/src/run.rs b/plus/bencher_runner/src/run.rs index 2f11d9898..08cd223e5 100644 --- a/plus/bencher_runner/src/run.rs +++ b/plus/bencher_runner/src/run.rs @@ -13,7 +13,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use bencher_json::Iteration; use crate::error::RunnerError; -use crate::tuning::TuningConfig; +use crate::tuning::{TuningConfig, preflight}; /// Output from a benchmark run. #[derive(Debug)] @@ -128,11 +128,43 @@ fn build_config_from_run_args(args: &RunArgs) -> Result Result<(), RunnerError> { - // Apply host tuning — guard restores settings on drop (no-op on non-Linux) - let _tuning_guard = crate::tuning::apply(&args.tuning); - - let config = build_config_from_run_args(args)?; + // Warn about host conditions that limit benchmark accuracy (Linux only) + preflight::print_host_warnings(); + + // Apply host tuning - guard restores settings on drop (no-op on non-Linux) + let mut tuning_guard = crate::tuning::apply(&args.tuning); + + let mut config = build_config_from_run_args(args)?; + + // Detect the CPU layout after tuning (disabling SMT changes the core + // count), steer kernel work off the benchmark cores, and pin the run + // to them. Mirrors the `runner up` path. Core pinning is core runner + // behavior, deliberately independent of --no-tuning: the tuning config + // only gates the tuning knobs (including the partition and steering). + #[cfg(target_os = "linux")] + { + let cpu_layout = crate::cpu::CpuLayout::detect(); + crate::tuning::apply_cpu_scoped(&args.tuning, &cpu_layout, &mut tuning_guard); + if cpu_layout.has_isolation() { + println!( + " CPU isolation: housekeeping={}, benchmark={}", + cpu_layout.housekeeping_cpuset(), + cpu_layout.benchmark_cpuset() + ); + config = config.with_cpu_layout(cpu_layout); + } else { + println!(" CPU isolation: disabled (insufficient cores)"); + } + } let iter_count = args.iter.as_usize(); for iteration in 0..iter_count { diff --git a/plus/bencher_runner/src/tuning/dma_latency.rs b/plus/bencher_runner/src/tuning/dma_latency.rs new file mode 100644 index 000000000..80814867a --- /dev/null +++ b/plus/bencher_runner/src/tuning/dma_latency.rs @@ -0,0 +1,97 @@ +//! C-state control via the PM `QoS` interface. +//! +//! Writing a latency value (in microseconds) to `/dev/cpu_dma_latency` and +//! holding the file descriptor open asks the kernel to keep CPU exit latency +//! at or below that value. A value of 0 keeps all CPUs out of deep C-states, +//! removing wakeup latency jitter from benchmark runs. The constraint is +//! released automatically when the fd closes, so a crashed runner cannot +//! leave the host stuck in polling mode. This works on both x86 and ARM, +//! unlike per-cpu `cpuidle/state*/disable` writes. + +use super::TuningGuard; + +/// PM `QoS` device node. +pub(super) const CPU_DMA_LATENCY: &str = "/dev/cpu_dma_latency"; + +/// The kernel expects a native-endian i32 latency in microseconds. +/// Only the value 0 is supported here, whose byte representation is +/// endianness-independent; any future non-zero latency must be encoded +/// with `i32::to_ne_bytes` instead of a literal array. +const ZERO_LATENCY: [u8; 4] = [0; 4]; + +/// Request a maximum CPU exit latency of 0 us and hold the constraint. +/// +/// The opened file is pushed onto the guard so the constraint stays active +/// until the guard drops. Missing device nodes (e.g., kernels without +/// `CONFIG_CPU_IDLE`) are skipped with an informational message. +pub(super) fn hold_dma_latency(guard: &mut TuningGuard, path: &str) { + use std::io::Write as _; + + if !std::path::Path::new(path).exists() { + println!(" Tuning: C-states - skipped (path not found)"); + return; + } + + let mut file = match std::fs::OpenOptions::new().write(true).open(path) { + Ok(file) => file, + Err(e) => { + println!(" Tuning: C-states - skipped (open failed: {e})"); + return; + }, + }; + + if let Err(e) = file.write_all(&ZERO_LATENCY) { + println!(" Tuning: C-states - skipped (write failed: {e})"); + return; + } + + println!(" Tuning: C-states - max exit latency held at 0 us (released on exit)"); + guard.held_fds.push(file); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn empty_guard() -> TuningGuard { + TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + } + } + + #[test] + fn holds_fd_and_writes_zero_latency() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cpu_dma_latency"); + std::fs::write(&path, [0xffu8; 4]).unwrap(); + + let mut guard = empty_guard(); + hold_dma_latency(&mut guard, path.to_str().unwrap()); + + assert_eq!(guard.held_fds.len(), 1); + // The write starts at offset 0 and covers all 4 original bytes. + let contents = std::fs::read(&path).unwrap(); + assert_eq!(contents, ZERO_LATENCY); + } + + #[test] + fn skips_missing_path() { + let mut guard = empty_guard(); + hold_dma_latency(&mut guard, "/nonexistent/cpu_dma_latency"); + assert!(guard.held_fds.is_empty()); + } + + #[test] + fn guard_drop_releases_fd() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cpu_dma_latency"); + std::fs::write(&path, []).unwrap(); + + let mut guard = empty_guard(); + hold_dma_latency(&mut guard, path.to_str().unwrap()); + assert_eq!(guard.held_fds.len(), 1); + // Dropping the guard must not panic and releases the fd. + drop(guard); + } +} diff --git a/plus/bencher_runner/src/tuning/kernel_work.rs b/plus/bencher_runner/src/tuning/kernel_work.rs new file mode 100644 index 000000000..e7ec819a4 --- /dev/null +++ b/plus/bencher_runner/src/tuning/kernel_work.rs @@ -0,0 +1,238 @@ +//! Steer device IRQs and unbound kernel workqueues to housekeeping cores. +//! +//! Device interrupts and unbound workqueue workers landing on benchmark +//! cores steal cycles mid-measurement. This module points the IRQ default +//! affinity, every movable IRQ, and the unbound workqueue cpumask at the +//! housekeeping cores instead. Some IRQs are not movable (the kernel +//! returns EIO); those are skipped and summarized in a single line. + +use camino::Utf8Path; + +use super::{TuningGuard, write_sysctl}; +use crate::cpu::{CpuLayout, format_cpumask}; + +/// Steer kernel work (IRQs and unbound workqueues) to housekeeping cores. +/// +/// `root` is the filesystem root (`/` in production); tests pass a +/// tempdir tree containing `proc/` and `sys/` subtrees. All writes are +/// saved on the guard and restored in reverse order on drop. +pub(super) fn steer_kernel_work(guard: &mut TuningGuard, layout: &CpuLayout, root: &Utf8Path) { + let housekeeping_mask = format_cpumask(&layout.housekeeping); + + // New IRQs default to housekeeping cores. + write_sysctl( + guard, + root.join("proc/irq/default_smp_affinity").as_str(), + &housekeeping_mask, + "default IRQ affinity", + ); + + // Unbound workqueue workers run on housekeeping cores. + write_sysctl( + guard, + root.join("sys/devices/virtual/workqueue/cpumask").as_str(), + &housekeeping_mask, + "workqueue cpumask", + ); + + steer_existing_irqs(guard, layout, root); +} + +/// Move every movable IRQ to the housekeeping cores. +/// +/// Iterates `proc/irq//smp_affinity_list`, skipping per-IRQ failures +/// (unmovable IRQs fail with EIO), and prints one summary line. +fn steer_existing_irqs(guard: &mut TuningGuard, layout: &CpuLayout, root: &Utf8Path) { + let irq_dir = root.join("proc/irq"); + let Ok(entries) = std::fs::read_dir(irq_dir.as_std_path()) else { + println!(" Tuning: IRQ steering - skipped (cannot read {irq_dir})"); + return; + }; + + let housekeeping_list = layout.housekeeping_cpuset(); + let mut total = 0usize; + let mut moved = 0usize; + + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(name_str) = name.to_str() else { + continue; + }; + // Only numeric directories are IRQs (skips default_smp_affinity etc.) + if name_str.is_empty() || !name_str.chars().all(|c| c.is_ascii_digit()) { + continue; + } + + let affinity_path = irq_dir.join(name_str).join("smp_affinity_list"); + let Ok(current) = std::fs::read_to_string(affinity_path.as_std_path()) else { + continue; + }; + let current = current.trim().to_owned(); + total += 1; + + if current == housekeeping_list { + moved += 1; + continue; + } + + if std::fs::write(affinity_path.as_std_path(), &housekeeping_list).is_err() { + // Unmovable IRQ (EIO) or insufficient permissions - skip. + continue; + } + + moved += 1; + guard.saved.push(super::SavedSetting { + path: affinity_path, + value: current, + label: format!("IRQ {name_str} affinity"), + }); + } + + println!( + " Tuning: IRQ steering - moved {moved} of {total} IRQs to housekeeping cores ({housekeeping_list})" + ); +} + +#[cfg(test)] +mod tests { + use std::fs; + + use camino::Utf8PathBuf; + + use super::*; + + fn empty_guard() -> TuningGuard { + TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + } + } + + /// Build a fake `/proc` + `/sys` tree with two movable IRQs. + fn fake_root() -> (tempfile::TempDir, Utf8PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + + fs::create_dir_all(root.join("proc/irq/10")).unwrap(); + fs::create_dir_all(root.join("proc/irq/11")).unwrap(); + fs::create_dir_all(root.join("sys/devices/virtual/workqueue")).unwrap(); + + fs::write(root.join("proc/irq/default_smp_affinity"), "ff").unwrap(); + fs::write(root.join("proc/irq/10/smp_affinity_list"), "0-7\n").unwrap(); + fs::write(root.join("proc/irq/11/smp_affinity_list"), "0-7\n").unwrap(); + fs::write(root.join("sys/devices/virtual/workqueue/cpumask"), "ff").unwrap(); + + (dir, root) + } + + #[test] + fn steers_irqs_and_workqueues_to_housekeeping() { + let (_dir, root) = fake_root(); + let layout = CpuLayout::with_core_count(8); + + let mut guard = empty_guard(); + steer_kernel_work(&mut guard, &layout, &root); + + assert_eq!( + fs::read_to_string(root.join("proc/irq/default_smp_affinity")).unwrap(), + "3" + ); + assert_eq!( + fs::read_to_string(root.join("sys/devices/virtual/workqueue/cpumask")).unwrap(), + "3" + ); + assert_eq!( + fs::read_to_string(root.join("proc/irq/10/smp_affinity_list")).unwrap(), + "0-1" + ); + assert_eq!( + fs::read_to_string(root.join("proc/irq/11/smp_affinity_list")).unwrap(), + "0-1" + ); + // default affinity + workqueue + 2 IRQs + assert_eq!(guard.saved.len(), 4); + } + + #[test] + fn guard_drop_restores_originals() { + let (_dir, root) = fake_root(); + let layout = CpuLayout::with_core_count(8); + + { + let mut guard = empty_guard(); + steer_kernel_work(&mut guard, &layout, &root); + } + + assert_eq!( + fs::read_to_string(root.join("proc/irq/default_smp_affinity")).unwrap(), + "ff" + ); + assert_eq!( + fs::read_to_string(root.join("sys/devices/virtual/workqueue/cpumask")).unwrap(), + "ff" + ); + assert_eq!( + fs::read_to_string(root.join("proc/irq/10/smp_affinity_list")).unwrap(), + "0-7" + ); + } + + #[test] + fn skips_unmovable_irq() { + let (_dir, root) = fake_root(); + // A directory in place of the affinity file forces the write to + // fail, mimicking an unmovable IRQ (EIO) even when running as root. + fs::create_dir_all(root.join("proc/irq/12/smp_affinity_list")).unwrap(); + let layout = CpuLayout::with_core_count(8); + + let mut guard = empty_guard(); + steer_kernel_work(&mut guard, &layout, &root); + + // The unmovable IRQ is skipped; the movable ones are still steered. + assert_eq!( + fs::read_to_string(root.join("proc/irq/10/smp_affinity_list")).unwrap(), + "0-1" + ); + assert_eq!(guard.saved.len(), 4); + } + + #[test] + fn already_steered_irq_not_saved() { + let (_dir, root) = fake_root(); + fs::write(root.join("proc/irq/10/smp_affinity_list"), "0-1").unwrap(); + let layout = CpuLayout::with_core_count(8); + + let mut guard = empty_guard(); + steer_kernel_work(&mut guard, &layout, &root); + + // IRQ 10 was already on housekeeping cores: not saved for restore. + assert_eq!(guard.saved.len(), 3); + } + + #[test] + fn skips_missing_tree() { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + let layout = CpuLayout::with_core_count(8); + + let mut guard = empty_guard(); + steer_kernel_work(&mut guard, &layout, &root); + assert!(guard.saved.is_empty()); + } + + #[test] + fn ignores_non_numeric_irq_dirs() { + let (_dir, root) = fake_root(); + fs::create_dir_all(root.join("proc/irq/not-an-irq")).unwrap(); + fs::write(root.join("proc/irq/not-an-irq/smp_affinity_list"), "0-7\n").unwrap(); + let layout = CpuLayout::with_core_count(8); + + let mut guard = empty_guard(); + steer_kernel_work(&mut guard, &layout, &root); + + assert_eq!( + fs::read_to_string(root.join("proc/irq/not-an-irq/smp_affinity_list")).unwrap(), + "0-7\n" + ); + } +} diff --git a/plus/bencher_runner/src/tuning/mod.rs b/plus/bencher_runner/src/tuning/mod.rs index afcae7ba6..b7a8bc843 100644 --- a/plus/bencher_runner/src/tuning/mod.rs +++ b/plus/bencher_runner/src/tuning/mod.rs @@ -3,19 +3,44 @@ //! Applies system-level optimizations (ASLR, CPU governor, SMT, etc.) //! to reduce benchmark noise. All settings are restored on drop via //! a RAII guard. Missing sysfs/procfs files are skipped with an -//! informational message — this handles ARM and other platforms +//! informational message - this handles ARM and other platforms //! where certain controls do not exist. +//! +//! # Crash safety +//! +//! Restoration only happens on clean shutdown (guard drop). On SIGKILL +//! or panic-abort, sysctl writes, IRQ affinities, THP mode, and the +//! cpuset partition stay applied. Worse, the next run then reads the +//! already-tuned value as "current", saves nothing, and a later clean +//! exit cannot restore the true pre-tuning value; a reboot (or manual +//! reset) recovers it. Two mechanisms self-heal by construction: the +//! `/dev/cpu_dma_latency` fd releases its PM `QoS` constraint when the +//! process dies, and per-run cgroups are removed by their own cleanup. +//! +//! Tuning also assumes a single runner process per host: the sysctls, +//! IRQ affinities, THP mode, and cpuset partition are host-global, so a +//! second concurrent runner's shutdown restores them out from under the +//! first. #![cfg_attr( target_os = "linux", expect(clippy::print_stdout, reason = "tuning prints applied settings") )] +#[cfg(target_os = "linux")] +mod dma_latency; +#[cfg(target_os = "linux")] +mod kernel_work; mod perf_event_paranoid; +pub mod preflight; mod swappiness; +mod thp; pub use perf_event_paranoid::PerfEventParanoid; pub use swappiness::Swappiness; +pub use thp::{ParseThpModeError, ThpMode}; + +use crate::cpu::CpuLayout; #[cfg(target_os = "linux")] use camino::{Utf8Path, Utf8PathBuf}; @@ -25,7 +50,7 @@ const INTEL_NO_TURBO: &str = "/sys/devices/system/cpu/intel_pstate/no_turbo"; #[cfg(target_os = "linux")] const CPUFREQ_BOOST: &str = "/sys/devices/system/cpu/cpufreq/boost"; -/// Host tuning configuration — all defaults optimize for benchmark accuracy. +/// Host tuning configuration - all defaults optimize for benchmark accuracy. #[expect( clippy::struct_excessive_bools, reason = "each bool maps to an independent system knob" @@ -46,6 +71,26 @@ pub struct TuningConfig { pub disable_smt: bool, /// Disable turboboost (default: true). pub disable_turbo: bool, + /// Disable automatic NUMA balancing page migration (default: true). + pub disable_numa_balancing: bool, + /// Disable timer migration between CPUs (default: true). + pub disable_timer_migration: bool, + /// Disable the soft lockup watchdog (default: true). + pub disable_soft_watchdog: bool, + /// Disable kernel samepage merging scanning (default: true). + pub disable_ksm: bool, + /// Hold CPUs out of deep C-states via `/dev/cpu_dma_latency` (default: true). + pub disable_cstates: bool, + /// Steer device IRQs and unbound workqueues to housekeeping cores + /// (default: true; only applied when the CPU layout has isolation). + pub steer_kernel_work: bool, + /// Detach benchmark cores from the root scheduling domain via an + /// isolated cpuset partition, the runtime equivalent of `isolcpus=` + /// (default: true; only applied when the CPU layout has isolation). + pub cpuset_partition: bool, + /// Host transparent hugepage mode (default: `never` for deterministic + /// memory backing; `leave` preserves the host configuration). + pub thp: ThpMode, } impl Default for TuningConfig { @@ -58,12 +103,20 @@ impl Default for TuningConfig { governor: Some("performance".to_owned()), disable_smt: true, disable_turbo: true, + disable_numa_balancing: true, + disable_timer_migration: true, + disable_soft_watchdog: true, + disable_ksm: true, + disable_cstates: true, + steer_kernel_work: true, + cpuset_partition: true, + thp: ThpMode::Never, } } } impl TuningConfig { - /// A config with all tuning disabled — no changes will be made. + /// A config with all tuning disabled - no changes will be made. pub fn disabled() -> Self { Self { disable_aslr: false, @@ -73,6 +126,14 @@ impl TuningConfig { governor: None, disable_smt: false, disable_turbo: false, + disable_numa_balancing: false, + disable_timer_migration: false, + disable_soft_watchdog: false, + disable_ksm: false, + disable_cstates: false, + steer_kernel_work: false, + cpuset_partition: false, + thp: ThpMode::Leave, } } } @@ -92,6 +153,22 @@ struct SavedSetting { #[cfg(target_os = "linux")] pub struct TuningGuard { saved: Vec, + /// File descriptors held open for the lifetime of the guard + /// (e.g., the PM `QoS` constraint on `/dev/cpu_dma_latency`). + /// Dropped after the saved settings are restored. + held_fds: Vec, +} + +#[cfg(target_os = "linux")] +impl TuningGuard { + /// Record a file to restore to `value` when the guard drops. + /// + /// Restoration happens in reverse recording order, so dependent writes + /// (e.g., a cpuset partition mode that must be reverted before its + /// cpuset can shrink) are undone correctly. + pub(crate) fn save_restore(&mut self, path: Utf8PathBuf, value: String, label: String) { + self.saved.push(SavedSetting { path, value, label }); + } } #[cfg(target_os = "linux")] @@ -106,7 +183,10 @@ impl Drop for TuningGuard { /// Apply host tuning. Returns a guard that restores settings on drop. #[cfg(target_os = "linux")] pub fn apply(config: &TuningConfig) -> TuningGuard { - let mut guard = TuningGuard { saved: Vec::new() }; + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; if config.disable_aslr { write_sysctl( @@ -156,38 +236,154 @@ pub fn apply(config: &TuningConfig) -> TuningGuard { set_turbo(&mut guard); } + if config.disable_numa_balancing { + write_sysctl( + &mut guard, + "/proc/sys/kernel/numa_balancing", + "0", + "NUMA balancing", + ); + } + + if config.disable_timer_migration { + write_sysctl( + &mut guard, + "/proc/sys/kernel/timer_migration", + "0", + "timer migration", + ); + } + + if config.disable_soft_watchdog { + write_sysctl( + &mut guard, + "/proc/sys/kernel/soft_watchdog", + "0", + "soft watchdog", + ); + } + + if config.disable_ksm { + write_sysctl(&mut guard, "/sys/kernel/mm/ksm/run", "0", "KSM"); + } + + if config.disable_cstates { + dma_latency::hold_dma_latency(&mut guard, dma_latency::CPU_DMA_LATENCY); + } + + if let Some(value) = config.thp.sysfs_value() { + write_bracketed_sysctl( + &mut guard, + "/sys/kernel/mm/transparent_hugepage/enabled", + value, + "THP enabled", + ); + write_bracketed_sysctl( + &mut guard, + "/sys/kernel/mm/transparent_hugepage/defrag", + value, + "THP defrag", + ); + } + guard } +/// Apply CPU-layout-scoped tuning (IRQ and workqueue steering). +/// +/// Must be called after [`apply`] and after [`CpuLayout::detect`], because +/// [`apply`] may disable SMT and change the core count. Settings are saved +/// on the same guard and restored on drop. +#[cfg(target_os = "linux")] +pub fn apply_cpu_scoped(config: &TuningConfig, layout: &CpuLayout, guard: &mut TuningGuard) { + if config.cpuset_partition && layout.has_isolation() { + let partition = crate::jail::BencherPartition::new(Utf8Path::new("/sys/fs/cgroup")); + let level = partition.apply(layout, guard); + println!(" Tuning: cpuset partition - achieved level '{level}'"); + } + + if config.steer_kernel_work && layout.has_isolation() { + kernel_work::steer_kernel_work(guard, layout, Utf8Path::new("/")); + } +} + /// Read current value, write new value, and push restore entry onto the guard. #[cfg(target_os = "linux")] fn write_sysctl(guard: &mut TuningGuard, path: &str, value: &str, label: &str) { let path = Utf8PathBuf::from(path); if !path.exists() { - println!(" Tuning: {label} — skipped (path not found)"); + println!(" Tuning: {label} - skipped (path not found)"); return; } let current = match std::fs::read_to_string(&path) { Ok(v) => v.trim().to_owned(), Err(e) => { - println!(" Tuning: {label} — skipped (read failed: {e})"); + println!(" Tuning: {label} - skipped (read failed: {e})"); + return; + }, + }; + + if current == value { + println!(" Tuning: {label} - already {value}"); + return; + } + + if let Err(e) = std::fs::write(path.as_str(), value) { + println!(" Tuning: {label} - skipped (write failed: {e})"); + return; + } + + println!(" Tuning: {label} - set to {value} (was {current})"); + guard.saved.push(SavedSetting { + path, + value: current, + label: label.to_owned(), + }); +} + +/// Like [`write_sysctl`], but for files using the bracketed selection +/// format (e.g., `always [madvise] never` under +/// `/sys/kernel/mm/transparent_hugepage/`). The current value is the +/// bracketed token; writes take the plain token, so save and restore +/// work with the parsed value. +#[cfg(target_os = "linux")] +fn write_bracketed_sysctl(guard: &mut TuningGuard, path: &str, value: &str, label: &str) { + let path = Utf8PathBuf::from(path); + + if !path.exists() { + println!(" Tuning: {label} - skipped (path not found)"); + return; + } + + let content = match std::fs::read_to_string(&path) { + Ok(v) => v, + Err(e) => { + println!(" Tuning: {label} - skipped (read failed: {e})"); return; }, }; + let Some(current) = parse_bracketed_value(&content) else { + println!( + " Tuning: {label} - skipped (unrecognized format: {})", + content.trim() + ); + return; + }; + let current = current.to_owned(); if current == value { - println!(" Tuning: {label} — already {value}"); + println!(" Tuning: {label} - already {value}"); return; } if let Err(e) = std::fs::write(path.as_str(), value) { - println!(" Tuning: {label} — skipped (write failed: {e})"); + println!(" Tuning: {label} - skipped (write failed: {e})"); return; } - println!(" Tuning: {label} — set to {value} (was {current})"); + println!(" Tuning: {label} - set to {value} (was {current})"); guard.saved.push(SavedSetting { path, value: current, @@ -195,12 +391,21 @@ fn write_sysctl(guard: &mut TuningGuard, path: &str, value: &str, label: &str) { }); } +/// Extract the selected token from a bracketed sysfs value like +/// `always [madvise] never`. +#[cfg(target_os = "linux")] +fn parse_bracketed_value(content: &str) -> Option<&str> { + content + .split_whitespace() + .find_map(|token| token.strip_prefix('[')?.strip_suffix(']')) +} + /// Set the CPU scaling governor on all CPUs. #[cfg(target_os = "linux")] fn set_cpu_governor(guard: &mut TuningGuard, target: &str) { let base = Utf8Path::new("/sys/devices/system/cpu"); let Ok(entries) = std::fs::read_dir(base) else { - println!(" Tuning: CPU governor — skipped (cannot read {base})"); + println!(" Tuning: CPU governor - skipped (cannot read {base})"); return; }; @@ -233,11 +438,11 @@ fn set_cpu_governor(guard: &mut TuningGuard, target: &str) { } if let Err(e) = std::fs::write(&gov_path, target) { - println!(" Tuning: CPU governor ({name_str}) — skipped (write failed: {e})"); + println!(" Tuning: CPU governor ({name_str}) - skipped (write failed: {e})"); continue; } - println!(" Tuning: CPU governor ({name_str}) — set to {target} (was {current})"); + println!(" Tuning: CPU governor ({name_str}) - set to {target} (was {current})"); let gov_utf8_path = Utf8PathBuf::try_from(gov_path) .unwrap_or_else(|p| Utf8PathBuf::from(p.into_path_buf().to_string_lossy().as_ref())); guard.saved.push(SavedSetting { @@ -254,29 +459,29 @@ fn set_smt(guard: &mut TuningGuard) { let path = Utf8PathBuf::from("/sys/devices/system/cpu/smt/control"); if !path.exists() { - println!(" Tuning: SMT — skipped (not available on this platform)"); + println!(" Tuning: SMT - skipped (not available on this platform)"); return; } let current = match std::fs::read_to_string(&path) { Ok(v) => v.trim().to_owned(), Err(e) => { - println!(" Tuning: SMT — skipped (read failed: {e})"); + println!(" Tuning: SMT - skipped (read failed: {e})"); return; }, }; if current == "off" || current == "notsupported" || current == "notimplemented" { - println!(" Tuning: SMT — already {current}"); + println!(" Tuning: SMT - already {current}"); return; } if let Err(e) = std::fs::write(path.as_str(), "off") { - println!(" Tuning: SMT — skipped (write failed: {e})"); + println!(" Tuning: SMT - skipped (write failed: {e})"); return; } - println!(" Tuning: SMT — disabled (was {current})"); + println!(" Tuning: SMT - disabled (was {current})"); guard.saved.push(SavedSetting { path, value: current, @@ -292,7 +497,7 @@ fn set_turbo(guard: &mut TuningGuard) { } else if Utf8Path::new(CPUFREQ_BOOST).exists() { write_sysctl(guard, CPUFREQ_BOOST, "0", "turboboost (generic)"); } else { - println!(" Tuning: turboboost — skipped (not available on this platform)"); + println!(" Tuning: turboboost - skipped (not available on this platform)"); } } @@ -300,8 +505,8 @@ fn set_turbo(guard: &mut TuningGuard) { #[cfg(target_os = "linux")] fn restore(path: &Utf8Path, value: &str, label: &str) { match std::fs::write(path.as_str(), value) { - Ok(()) => println!(" Tuning: {label} — restored to {value}"), - Err(e) => println!(" Tuning: {label} — restore failed: {e}"), + Ok(()) => println!(" Tuning: {label} - restored to {value}"), + Err(e) => println!(" Tuning: {label} - restore failed: {e}"), } } @@ -313,12 +518,16 @@ fn restore(path: &Utf8Path, value: &str, label: &str) { #[cfg(not(target_os = "linux"))] pub struct TuningGuard; -/// No-op on non-Linux — returns a stub guard. +/// No-op on non-Linux - returns a stub guard. #[cfg(not(target_os = "linux"))] pub fn apply(_config: &TuningConfig) -> TuningGuard { TuningGuard } +/// No-op on non-Linux. +#[cfg(not(target_os = "linux"))] +pub fn apply_cpu_scoped(_config: &TuningConfig, _layout: &CpuLayout, _guard: &mut TuningGuard) {} + #[cfg(test)] mod tests { use super::*; @@ -336,6 +545,14 @@ mod tests { assert_eq!(config.governor.as_deref(), Some("performance")); assert!(config.disable_smt); assert!(config.disable_turbo); + assert!(config.disable_numa_balancing); + assert!(config.disable_timer_migration); + assert!(config.disable_soft_watchdog); + assert!(config.disable_ksm); + assert!(config.disable_cstates); + assert!(config.steer_kernel_work); + assert!(config.cpuset_partition); + assert_eq!(config.thp, ThpMode::Never); } #[test] @@ -348,6 +565,36 @@ mod tests { assert_eq!(config.governor, None); assert!(!config.disable_smt); assert!(!config.disable_turbo); + assert!(!config.disable_numa_balancing); + assert!(!config.disable_timer_migration); + assert!(!config.disable_soft_watchdog); + assert!(!config.disable_ksm); + assert!(!config.disable_cstates); + assert!(!config.steer_kernel_work); + assert!(!config.cpuset_partition); + assert_eq!(config.thp, ThpMode::Leave); + } + + #[test] + fn apply_cpu_scoped_disabled_saves_nothing() { + let config = TuningConfig::disabled(); + let layout = CpuLayout::with_core_count(8); + let mut guard = apply(&config); + apply_cpu_scoped(&config, &layout, &mut guard); + #[cfg(target_os = "linux")] + assert!(guard.saved.is_empty()); + } + + #[test] + fn apply_cpu_scoped_no_isolation_saves_nothing() { + let config = TuningConfig::default(); + let layout = CpuLayout::with_core_count(1); + // Empty guard from a disabled config; the single-core layout means + // apply_cpu_scoped must not touch anything. + let mut guard = apply(&TuningConfig::disabled()); + apply_cpu_scoped(&config, &layout, &mut guard); + #[cfg(target_os = "linux")] + assert!(guard.saved.is_empty()); } #[test] @@ -383,7 +630,10 @@ mod tests { fs::write(&file_path, "original").unwrap(); { - let mut guard = TuningGuard { saved: Vec::new() }; + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; guard.saved.push(SavedSetting { path: file_path.clone(), value: "original".to_owned(), @@ -393,7 +643,7 @@ mod tests { fs::write(&file_path, "changed").unwrap(); assert_eq!(fs::read_to_string(&file_path).unwrap(), "changed"); } - // Guard dropped — should restore + // Guard dropped - should restore assert_eq!(fs::read_to_string(&file_path).unwrap(), "original"); } @@ -410,7 +660,10 @@ mod tests { fs::write(&path2, "b").unwrap(); { - let mut guard = TuningGuard { saved: Vec::new() }; + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; guard.saved.push(SavedSetting { path: path1.clone(), value: "a_orig".to_owned(), @@ -430,7 +683,10 @@ mod tests { #[cfg(target_os = "linux")] #[test] fn write_sysctl_skips_missing_path() { - let mut guard = TuningGuard { saved: Vec::new() }; + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; write_sysctl(&mut guard, "/nonexistent/path/value", "0", "test"); assert!( guard.saved.is_empty(), @@ -447,7 +703,10 @@ mod tests { let path = dir.path().join("value"); fs::write(&path, "0").unwrap(); - let mut guard = TuningGuard { saved: Vec::new() }; + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; write_sysctl(&mut guard, path.to_str().unwrap(), "0", "test"); assert!( guard.saved.is_empty(), @@ -455,6 +714,87 @@ mod tests { ); } + #[cfg(target_os = "linux")] + #[test] + fn parse_bracketed_value_variants() { + assert_eq!( + parse_bracketed_value("always [madvise] never\n"), + Some("madvise") + ); + assert_eq!( + parse_bracketed_value("[always] madvise never"), + Some("always") + ); + assert_eq!( + parse_bracketed_value("always defer defer+madvise madvise [never]\n"), + Some("never") + ); + assert_eq!(parse_bracketed_value("no brackets here"), None); + assert_eq!(parse_bracketed_value(""), None); + } + + #[cfg(target_os = "linux")] + #[test] + fn write_bracketed_sysctl_saves_and_writes() { + use std::fs; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("enabled"); + fs::write(&path, "always [madvise] never\n").unwrap(); + + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; + write_bracketed_sysctl(&mut guard, path.to_str().unwrap(), "never", "test"); + + assert_eq!(guard.saved.len(), 1); + assert_eq!(guard.saved[0].value, "madvise"); + assert_eq!(fs::read_to_string(&path).unwrap(), "never"); + } + + #[cfg(target_os = "linux")] + #[test] + fn write_bracketed_sysctl_skips_if_already_set() { + use std::fs; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("enabled"); + fs::write(&path, "always madvise [never]\n").unwrap(); + + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; + write_bracketed_sysctl(&mut guard, path.to_str().unwrap(), "never", "test"); + + assert!(guard.saved.is_empty()); + // The file is untouched (still in the kernel's bracketed format) + assert_eq!( + fs::read_to_string(&path).unwrap(), + "always madvise [never]\n" + ); + } + + #[cfg(target_os = "linux")] + #[test] + fn write_bracketed_sysctl_skips_unrecognized_format() { + use std::fs; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("enabled"); + fs::write(&path, "garbage\n").unwrap(); + + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; + write_bracketed_sysctl(&mut guard, path.to_str().unwrap(), "never", "test"); + + assert!(guard.saved.is_empty()); + assert_eq!(fs::read_to_string(&path).unwrap(), "garbage\n"); + } + #[cfg(target_os = "linux")] #[test] fn write_sysctl_saves_and_writes() { @@ -464,7 +804,10 @@ mod tests { let path = dir.path().join("value"); fs::write(&path, "60").unwrap(); - let mut guard = TuningGuard { saved: Vec::new() }; + let mut guard = TuningGuard { + saved: Vec::new(), + held_fds: Vec::new(), + }; write_sysctl(&mut guard, path.to_str().unwrap(), "10", "test"); assert_eq!(guard.saved.len(), 1); diff --git a/plus/bencher_runner/src/tuning/preflight.rs b/plus/bencher_runner/src/tuning/preflight.rs new file mode 100644 index 000000000..e0ddd535f --- /dev/null +++ b/plus/bencher_runner/src/tuning/preflight.rs @@ -0,0 +1,295 @@ +//! Static preflight checks for host conditions that limit benchmark accuracy. +//! +//! These checks detect conditions the runner cannot fix at runtime +//! (virtualization, missing cgroup v2) or that would fight the runner's +//! own tuning (irqbalance). They are informational only: the runner +//! proceeds either way, falling back to its runtime isolation measures. + +use camino::Utf8Path; + +/// A host condition that may increase benchmark variance. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PreflightWarning { + /// The irqbalance daemon is running and will fight IRQ steering. + IrqBalanceRunning, + /// cgroup v2 is not available, so cpuset-based CPU isolation is not possible. + NoCgroupV2, + /// The host appears to be virtualized; hypervisor steal time adds variance. + VirtualizedHost, + /// The kernel booted without CPU isolation boot args; the runtime cpuset + /// partition is used as a fallback. + NoIsolationBootArgs, +} + +impl std::fmt::Display for PreflightWarning { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::IrqBalanceRunning => write!( + f, + "irqbalance is running and will fight IRQ steering; consider `systemctl disable --now irqbalance`" + ), + Self::NoCgroupV2 => write!( + f, + "cgroup v2 is not available; cpuset-based CPU isolation will be skipped" + ), + Self::VirtualizedHost => write!( + f, + "host appears to be virtualized; hypervisor steal time adds variance, prefer bare metal" + ), + Self::NoIsolationBootArgs => write!( + f, + "kernel booted without `isolcpus=`/`nohz_full=`; using a runtime cpuset partition as fallback (for full isolation add boot args `isolcpus= nohz_full= rcu_nocbs=` listing the benchmark cores)" + ), + } + } +} + +/// Detect preflight warnings under the given filesystem root. +/// +/// `root` is `/` in production; tests pass a tempdir tree containing +/// `proc/` and `sys/` subtrees. +#[must_use] +pub fn detect(root: &Utf8Path) -> Vec { + let mut warnings = Vec::new(); + + if irqbalance_running(root) { + warnings.push(PreflightWarning::IrqBalanceRunning); + } + + if !root.join("sys/fs/cgroup/cgroup.controllers").exists() { + warnings.push(PreflightWarning::NoCgroupV2); + } + + if virtualized_host(root) { + warnings.push(PreflightWarning::VirtualizedHost); + } + + // Only warn when the cmdline is readable and lacks the isolation args; + // an unreadable cmdline gives no signal either way. + if let Ok(cmdline) = std::fs::read_to_string(root.join("proc/cmdline").as_std_path()) + && !cmdline.contains("isolcpus=") + && !cmdline.contains("nohz_full=") + { + warnings.push(PreflightWarning::NoIsolationBootArgs); + } + + warnings +} + +/// Detect and print preflight warnings for the live host (Linux only). +pub fn print_host_warnings() { + #[cfg(target_os = "linux")] + for warning in detect(Utf8Path::new("/")) { + println!(" Preflight: {warning}"); + } +} + +/// Check whether an irqbalance process is running by scanning `proc/*/comm`. +fn irqbalance_running(root: &Utf8Path) -> bool { + let Ok(entries) = std::fs::read_dir(root.join("proc").as_std_path()) else { + return false; + }; + + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(name_str) = name.to_str() else { + continue; + }; + if name_str.is_empty() || !name_str.chars().all(|c| c.is_ascii_digit()) { + continue; + } + + if let Ok(comm) = std::fs::read_to_string(entry.path().join("comm")) + && comm.trim() == "irqbalance" + { + return true; + } + } + + false +} + +/// Hypervisor strings found in DMI system vendor or product name. +const HYPERVISOR_MARKERS: &[&str] = &["QEMU", "KVM", "VMware", "Xen", "innotek", "Virtual Machine"]; + +/// Heuristic virtualization detection. +/// +/// Checks the x86 `hypervisor` cpuinfo flag and, as a cross-arch fallback, +/// well-known hypervisor strings in the DMI system vendor and product name. +fn virtualized_host(root: &Utf8Path) -> bool { + if let Ok(cpuinfo) = std::fs::read_to_string(root.join("proc/cpuinfo").as_std_path()) + && cpuinfo.lines().any(|line| { + line.starts_with("flags") && line.split_whitespace().any(|flag| flag == "hypervisor") + }) + { + return true; + } + + for dmi_file in ["sys_vendor", "product_name"] { + if let Ok(value) = std::fs::read_to_string( + root.join("sys/devices/virtual/dmi/id") + .join(dmi_file) + .as_std_path(), + ) && HYPERVISOR_MARKERS + .iter() + .any(|marker| value.contains(marker)) + { + return true; + } + } + + false +} + +#[cfg(test)] +mod tests { + use std::fs; + + use camino::Utf8PathBuf; + + use super::*; + + /// A root that raises none of the warnings. + fn quiet_root() -> (tempfile::TempDir, Utf8PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::try_from(dir.path().to_path_buf()).unwrap(); + + fs::create_dir_all(root.join("proc")).unwrap(); + fs::create_dir_all(root.join("sys/fs/cgroup")).unwrap(); + fs::write(root.join("sys/fs/cgroup/cgroup.controllers"), "cpuset cpu").unwrap(); + fs::write(root.join("proc/cmdline"), "isolcpus=2-7 nohz_full=2-7\n").unwrap(); + + (dir, root) + } + + #[test] + fn quiet_host_no_warnings() { + let (_dir, root) = quiet_root(); + assert_eq!(detect(&root), Vec::new()); + } + + #[test] + fn detects_irqbalance() { + let (_dir, root) = quiet_root(); + fs::create_dir_all(root.join("proc/123")).unwrap(); + fs::write(root.join("proc/123/comm"), "irqbalance\n").unwrap(); + + assert!(detect(&root).contains(&PreflightWarning::IrqBalanceRunning)); + } + + #[test] + fn other_processes_not_flagged() { + let (_dir, root) = quiet_root(); + fs::create_dir_all(root.join("proc/123")).unwrap(); + fs::write(root.join("proc/123/comm"), "bash\n").unwrap(); + + assert!(!detect(&root).contains(&PreflightWarning::IrqBalanceRunning)); + } + + #[test] + fn detects_missing_cgroup_v2() { + let (_dir, root) = quiet_root(); + fs::remove_file(root.join("sys/fs/cgroup/cgroup.controllers")).unwrap(); + + assert!(detect(&root).contains(&PreflightWarning::NoCgroupV2)); + } + + #[test] + fn detects_hypervisor_cpuinfo_flag() { + let (_dir, root) = quiet_root(); + fs::write( + root.join("proc/cpuinfo"), + "processor : 0\nflags : fpu vme hypervisor sse2\n", + ) + .unwrap(); + + assert!(detect(&root).contains(&PreflightWarning::VirtualizedHost)); + } + + #[test] + fn bare_metal_cpuinfo_not_flagged() { + let (_dir, root) = quiet_root(); + fs::write( + root.join("proc/cpuinfo"), + "processor : 0\nflags : fpu vme sse2\n", + ) + .unwrap(); + + assert!(!detect(&root).contains(&PreflightWarning::VirtualizedHost)); + } + + #[test] + fn detects_dmi_vendor() { + let (_dir, root) = quiet_root(); + fs::create_dir_all(root.join("sys/devices/virtual/dmi/id")).unwrap(); + fs::write(root.join("sys/devices/virtual/dmi/id/sys_vendor"), "QEMU\n").unwrap(); + + assert!(detect(&root).contains(&PreflightWarning::VirtualizedHost)); + } + + #[test] + fn detects_dmi_product_name() { + let (_dir, root) = quiet_root(); + fs::create_dir_all(root.join("sys/devices/virtual/dmi/id")).unwrap(); + fs::write( + root.join("sys/devices/virtual/dmi/id/sys_vendor"), + "Microsoft Corporation\n", + ) + .unwrap(); + fs::write( + root.join("sys/devices/virtual/dmi/id/product_name"), + "Virtual Machine\n", + ) + .unwrap(); + + assert!(detect(&root).contains(&PreflightWarning::VirtualizedHost)); + } + + #[test] + fn detects_missing_isolation_boot_args() { + let (_dir, root) = quiet_root(); + fs::write(root.join("proc/cmdline"), "root=/dev/sda1 quiet\n").unwrap(); + + assert!(detect(&root).contains(&PreflightWarning::NoIsolationBootArgs)); + } + + #[test] + fn nohz_full_alone_counts_as_isolation() { + let (_dir, root) = quiet_root(); + fs::write(root.join("proc/cmdline"), "nohz_full=2-7\n").unwrap(); + + assert!(!detect(&root).contains(&PreflightWarning::NoIsolationBootArgs)); + } + + #[test] + fn unreadable_cmdline_gives_no_boot_arg_warning() { + let (_dir, root) = quiet_root(); + fs::remove_file(root.join("proc/cmdline")).unwrap(); + + assert!(!detect(&root).contains(&PreflightWarning::NoIsolationBootArgs)); + } + + #[test] + fn warning_display_is_human_readable() { + assert!( + PreflightWarning::IrqBalanceRunning + .to_string() + .contains("irqbalance") + ); + assert!( + PreflightWarning::NoCgroupV2 + .to_string() + .contains("cgroup v2") + ); + assert!( + PreflightWarning::VirtualizedHost + .to_string() + .contains("virtualized") + ); + assert!( + PreflightWarning::NoIsolationBootArgs + .to_string() + .contains("isolcpus") + ); + } +} diff --git a/plus/bencher_runner/src/tuning/thp.rs b/plus/bencher_runner/src/tuning/thp.rs new file mode 100644 index 000000000..701736816 --- /dev/null +++ b/plus/bencher_runner/src/tuning/thp.rs @@ -0,0 +1,101 @@ +//! Transparent hugepage (THP) mode for the host. +//! +//! THP collapses regular pages into huge pages in the background, so the +//! memory backing of a benchmark process differs nondeterministically +//! between runs. Pinning the mode to `never` makes backing deterministic; +//! `leave` preserves whatever the host has configured. + +/// Error type for invalid THP mode strings. +#[derive(Debug, thiserror::Error)] +#[error("Invalid transparent hugepage mode: {0} (expected: never, madvise, always, leave)")] +pub struct ParseThpModeError(pub String); + +/// Host transparent hugepage mode applied to +/// `/sys/kernel/mm/transparent_hugepage/{enabled,defrag}`. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum ThpMode { + /// Disable THP for deterministic memory backing (the default). + #[default] + Never, + /// THP only for regions that request it via `madvise`. + Madvise, + /// THP for all anonymous memory. + Always, + /// Do not touch the host THP configuration. + Leave, +} + +impl ThpMode { + /// The value to write to the sysfs files, or `None` for [`Self::Leave`]. + #[must_use] + pub fn sysfs_value(self) -> Option<&'static str> { + match self { + Self::Never => Some("never"), + Self::Madvise => Some("madvise"), + Self::Always => Some("always"), + Self::Leave => None, + } + } +} + +impl std::fmt::Display for ThpMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.sysfs_value().unwrap_or("leave")) + } +} + +impl std::str::FromStr for ThpMode { + type Err = ParseThpModeError; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "never" => Ok(Self::Never), + "madvise" => Ok(Self::Madvise), + "always" => Ok(Self::Always), + "leave" => Ok(Self::Leave), + _ => Err(ParseThpModeError(s.to_owned())), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_is_never() { + assert_eq!(ThpMode::default(), ThpMode::Never); + } + + #[test] + fn from_str_round_trip() { + for mode in [ + ThpMode::Never, + ThpMode::Madvise, + ThpMode::Always, + ThpMode::Leave, + ] { + assert_eq!(mode.to_string().parse::().unwrap(), mode); + } + } + + #[test] + fn from_str_case_insensitive() { + assert_eq!("NEVER".parse::().unwrap(), ThpMode::Never); + assert_eq!("Leave".parse::().unwrap(), ThpMode::Leave); + } + + #[test] + fn from_str_invalid() { + let err = "sometimes".parse::().unwrap_err(); + assert!(err.to_string().contains("sometimes")); + } + + #[test] + fn sysfs_values() { + assert_eq!(ThpMode::Never.sysfs_value(), Some("never")); + assert_eq!(ThpMode::Madvise.sysfs_value(), Some("madvise")); + assert_eq!(ThpMode::Always.sysfs_value(), Some("always")); + assert_eq!(ThpMode::Leave.sysfs_value(), None); + } +} diff --git a/plus/bencher_runner/src/up/mod.rs b/plus/bencher_runner/src/up/mod.rs index 9301e625d..35b199430 100644 --- a/plus/bencher_runner/src/up/mod.rs +++ b/plus/bencher_runner/src/up/mod.rs @@ -5,7 +5,7 @@ use url::Url; use crate::cpu::CpuLayout; use crate::log_level::SandboxLogLevel; -use crate::tuning::TuningConfig; +use crate::tuning::{TuningConfig, preflight}; mod api_client; mod error; @@ -79,7 +79,11 @@ impl Up { #[expect(clippy::print_stdout, reason = "runner CLI startup output")] #[cfg_attr( not(target_os = "linux"), - expect(unused_mut, reason = "mut needed on Linux for CPU layout detection") + expect( + unused_mut, + unused_variables, + reason = "mut and tuning guard needed on Linux for CPU layout detection" + ) )] pub fn run(mut self) -> Result<(), UpError> { install_signal_handlers(); @@ -95,10 +99,13 @@ impl Up { println!(" Non-sandboxed execution: allowed"); } - // Apply host tuning — guard restores settings on drop (no-op on non-Linux). + // Warn about host conditions that limit benchmark accuracy (Linux only) + preflight::print_host_warnings(); + + // Apply host tuning - guard restores settings on drop (no-op on non-Linux). // This must happen before CPU layout detection so that SMT changes // are reflected in the core count. - let _tuning_guard = crate::tuning::apply(&self.config.tuning); + let mut tuning_guard = crate::tuning::apply(&self.config.tuning); // Re-detect CPU layout after tuning (SMT may have changed core count). // Linux-only: CpuLayout::detect() reads /sys/devices which only exists on Linux. @@ -106,6 +113,7 @@ impl Up { { self.config.cpu_layout = Some(CpuLayout::detect()); if let Some(cpu_layout) = &self.config.cpu_layout { + crate::tuning::apply_cpu_scoped(&self.config.tuning, cpu_layout, &mut tuning_guard); if cpu_layout.has_isolation() { println!( " CPU isolation: housekeeping={}, benchmark={}", diff --git a/services/runner/src/parser/tuning.rs b/services/runner/src/parser/tuning.rs index 62d41595d..e3305f42e 100644 --- a/services/runner/src/parser/tuning.rs +++ b/services/runner/src/parser/tuning.rs @@ -1,7 +1,14 @@ -use bencher_runner::{PerfEventParanoid, Swappiness, TuningConfig}; +use bencher_runner::{PerfEventParanoid, Swappiness, ThpMode, TuningConfig}; use clap::Args; /// Host tuning flags shared by `run` and `up` subcommands. +/// +/// Flag polarity: hardware features that tuning disables use positive +/// keep-enabled flags naming the feature (`--smt`, `--turbo`, +/// `--cstates`, `--ksm`); actions the runner itself performs use `--no-` +/// flags naming the skipped action (`--no-irq-steering`, +/// `--no-cpuset-partition`), since a bare `--irq-steering` would read +/// as enabling something that is already on by default. #[expect( clippy::struct_excessive_bools, reason = "CLI flags map to independent tuning knobs" @@ -28,6 +35,35 @@ pub struct CliTuning { #[arg(long)] pub turbo: bool, + /// Keep automatic NUMA balancing enabled (default: disabled for benchmarks). + #[arg(long)] + pub numa_balancing: bool, + + /// Keep timer migration enabled (default: disabled for benchmarks). + #[arg(long)] + pub timer_migration: bool, + + /// Keep the soft lockup watchdog enabled (default: disabled for benchmarks). + #[arg(long)] + pub soft_watchdog: bool, + + /// Keep kernel samepage merging (KSM) enabled (default: disabled for benchmarks). + #[arg(long)] + pub ksm: bool, + + /// Keep deep C-states enabled (default: disabled for benchmarks). + #[arg(long)] + pub cstates: bool, + + /// Do not steer device IRQs and kernel workqueues to housekeeping cores. + #[arg(long)] + pub no_irq_steering: bool, + + /// Do not detach benchmark cores from the scheduler via an isolated + /// cpuset partition. + #[arg(long)] + pub no_cpuset_partition: bool, + /// Set swappiness value (default: 10). #[arg(long)] pub swappiness: Option, @@ -39,6 +75,11 @@ pub struct CliTuning { /// Set `perf_event_paranoid` value (default: -1). #[arg(long, allow_hyphen_values = true)] pub perf_event_paranoid: Option, + + /// Set host transparent hugepage mode (default: never). + /// Use "leave" to preserve the host configuration. + #[arg(long)] + pub thp: Option, } impl TryFrom for TuningConfig { @@ -64,6 +105,108 @@ impl TryFrom for TuningConfig { governor: cli.governor.or_else(|| Some("performance".to_owned())), disable_smt: !cli.smt, disable_turbo: !cli.turbo, + disable_numa_balancing: !cli.numa_balancing, + disable_timer_migration: !cli.timer_migration, + disable_soft_watchdog: !cli.soft_watchdog, + disable_ksm: !cli.ksm, + disable_cstates: !cli.cstates, + steer_kernel_work: !cli.no_irq_steering, + cpuset_partition: !cli.no_cpuset_partition, + thp: cli.thp.unwrap_or_default(), }) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn default_cli() -> CliTuning { + CliTuning { + no_tuning: false, + aslr: false, + nmi_watchdog: false, + smt: false, + turbo: false, + numa_balancing: false, + timer_migration: false, + soft_watchdog: false, + ksm: false, + cstates: false, + no_irq_steering: false, + no_cpuset_partition: false, + swappiness: None, + governor: None, + perf_event_paranoid: None, + thp: None, + } + } + + #[test] + fn default_flags_enable_all_tuning() { + let config = TuningConfig::try_from(default_cli()).unwrap(); + assert!(config.disable_aslr); + assert!(config.disable_nmi_watchdog); + assert!(config.disable_smt); + assert!(config.disable_turbo); + assert!(config.disable_numa_balancing); + assert!(config.disable_timer_migration); + assert!(config.disable_soft_watchdog); + assert!(config.disable_ksm); + assert!(config.disable_cstates); + assert!(config.steer_kernel_work); + assert!(config.cpuset_partition); + assert_eq!(config.thp, ThpMode::Never); + assert_eq!(config.swappiness, Some(Swappiness::DEFAULT)); + assert_eq!(config.perf_event_paranoid, Some(PerfEventParanoid::DEFAULT)); + assert_eq!(config.governor.as_deref(), Some("performance")); + } + + #[test] + fn no_tuning_disables_everything() { + let cli = CliTuning { + no_tuning: true, + ..default_cli() + }; + let config = TuningConfig::try_from(cli).unwrap(); + assert!(!config.disable_aslr); + assert!(!config.disable_numa_balancing); + assert!(!config.disable_cstates); + assert!(!config.steer_kernel_work); + assert!(!config.cpuset_partition); + assert_eq!(config.thp, ThpMode::Leave); + assert_eq!(config.swappiness, None); + assert_eq!(config.governor, None); + } + + #[test] + fn thp_override_passed_through() { + let cli = CliTuning { + thp: Some(ThpMode::Leave), + ..default_cli() + }; + let config = TuningConfig::try_from(cli).unwrap(); + assert_eq!(config.thp, ThpMode::Leave); + } + + #[test] + fn individual_keep_flags_disable_single_knobs() { + let cli = CliTuning { + numa_balancing: true, + cstates: true, + no_irq_steering: true, + no_cpuset_partition: true, + ..default_cli() + }; + let config = TuningConfig::try_from(cli).unwrap(); + assert!(!config.disable_numa_balancing); + assert!(!config.disable_cstates); + assert!(!config.steer_kernel_work); + assert!(!config.cpuset_partition); + // Everything else stays enabled + assert!(config.disable_aslr); + assert!(config.disable_timer_migration); + assert!(config.disable_soft_watchdog); + assert!(config.disable_ksm); + } +} diff --git a/tasks/test_runner/src/task/scenarios.rs b/tasks/test_runner/src/task/scenarios.rs index 7de4e3388..3ee044963 100644 --- a/tasks/test_runner/src/task/scenarios.rs +++ b/tasks/test_runner/src/task/scenarios.rs @@ -2140,6 +2140,40 @@ CMD ["sh", "-c", "echo $MY_VAR"]"#, } }, }, + Scenario { + name: "nosandbox_metrics", + description: "Non-sandboxed: run metrics on stderr with local transport", + // Verifies the local path emits ---BENCHER_METRICS:{json}--- with + // transport "local" (it previously emitted no metrics at all). + dockerfile: r#"FROM busybox:musl +CMD ["echo", "local_metrics_test"]"#, + cancel_after_secs: None, + sandboxed: false, + extra_args: &["--timeout", "60"], + validate: |output| { + let metrics_line = output + .stderr + .lines() + .find(|l| l.contains("---BENCHER_METRICS:")); + let Some(line) = metrics_line else { + bail!( + "No BENCHER_METRICS line found in stderr.\nstderr: {}", + output.stderr + ) + }; + let json_str = extract_json_substr(line); + if let Ok(json) = serde_json::from_str::(json_str) + && let Some(transport) = + json.get("transport").and_then(serde_json::Value::as_str) + { + if transport == "local" { + return Ok(()); + } + bail!("Unexpected transport type: {transport}") + } + bail!("Could not find transport in metrics: {json_str}") + }, + }, Scenario { name: "nosandbox_exit_code", description: "Non-sandboxed: non-zero exit code propagation",