diff --git a/Cargo.lock b/Cargo.lock index 8a01d56..c0eec2f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -484,6 +484,12 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "fuchsia-cprng" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" + [[package]] name = "futures" version = "0.3.30" @@ -865,6 +871,7 @@ dependencies = [ "procfs", "ring", "rstest", + "tempdir", "thiserror", "tracing", "tracing-subscriber", @@ -1296,6 +1303,43 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "552840b97013b1a26992c11eac34bdd778e464601a4c2054b5f0bff7c6761293" +dependencies = [ + "fuchsia-cprng", + "libc", + "rand_core 0.3.1", + "rdrand", + "winapi", +] + +[[package]] +name = "rand_core" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a6fdeb83b075e8266dcc8762c22776f6877a63111121f5f8c7411e5be7eed4b" +dependencies = [ + "rand_core 0.4.2", +] + +[[package]] +name = "rand_core" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c33a3c44ca05fa6f1807d8e6743f3824e8509beca625669633be0acbdf509dc" + +[[package]] +name = "rdrand" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "678054eb77286b51581ba43620cc911abf02758c91f93f479767aed0f90458b2" +dependencies = [ + "rand_core 0.3.1", +] + [[package]] name = "redox_syscall" version = "0.4.1" @@ -1340,6 +1384,15 @@ version = "1.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc" +[[package]] +name = "remove_dir_all" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7" +dependencies = [ + "winapi", +] + [[package]] name = "rgb" version = "0.8.37" @@ -1366,9 +1419,9 @@ dependencies = [ [[package]] name = "rstest" -version = "0.18.2" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97eeab2f3c0a199bc4be135c36c924b6590b88c377d416494288c14f2db30199" +checksum = "9d5316d2a1479eeef1ea21e7f9ddc67c191d497abc8fc3ba2467857abbb68330" dependencies = [ "futures", "futures-timer", @@ -1378,9 +1431,9 @@ dependencies = [ [[package]] name = "rstest_macros" -version = "0.18.2" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d428f8247852f894ee1be110b375111b586d4fa431f6c46e64ba5a0dcccbe605" +checksum = "04a9df72cc1f67020b0d63ad9bfe4a323e459ea7eb68e03bd9824db49f9a4c25" dependencies = [ "cfg-if", "glob", @@ -1614,6 +1667,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempdir" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15f2b5fb00ccdf689e0149d1b1b3c03fead81c2b37735d812fa8bddbbf41b6d8" +dependencies = [ + "rand", + "remove_dir_all", +] + [[package]] name = "tempfile" version = "3.9.0" diff --git a/Cargo.toml b/Cargo.toml index 188c2cd..cc1fbf9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,12 +26,13 @@ tracing-subscriber = "0.3.18" chrono = "0.4.37" inferno = "0.11.19" primal = "0.3.2" -nix = { version = "0.28.0", features = ["user"]} +nix = { version = "0.28.0", features = ["user"] } [dev-dependencies] assert_cmd = { version = "2.0.14" } insta = { version = "1.38.0", features = ["yaml"] } -rstest = "0.18.2" +rstest = "0.19.0" +tempdir = "0.3.7" [build-dependencies] bindgen = "0.69.4" diff --git a/src/profiler.rs b/src/profiler.rs index b07f3dc..f8e7cca 100644 --- a/src/profiler.rs +++ b/src/profiler.rs @@ -22,7 +22,7 @@ use crate::collector::*; use crate::object::{BuildId, ObjectFile}; use crate::perf_events::setup_perf_event; use crate::unwind_info::{in_memory_unwind_info, remove_redundant, remove_unnecesary_markers}; -use crate::util::summarize_address_range; +use crate::util::{get_online_cpus, summarize_address_range}; pub enum TracerEvent { ProcessExit(i32), @@ -234,7 +234,9 @@ impl Profiler<'_> { } pub fn run(mut self, collector: Arc>) { - let num_cpus = num_possible_cpus().expect("get possible CPUs") as u64; + // In this case, we only want to calculate maximum sampling buffer sizes based on the + // number of online CPUs, NOT possible CPUs, when they differ - which is often. + let num_cpus = get_online_cpus().expect("get online CPUs").len() as u64; let max_samples_per_session = self.sample_freq as u64 * num_cpus * self.session_duration.as_secs(); if max_samples_per_session >= MAX_AGGREGATED_STACKS_ENTRIES.into() { @@ -465,6 +467,9 @@ impl Profiler<'_> { let value = unsafe { plain::as_bytes(&default) }; let mut values: Vec> = Vec::new(); + // This is a place where you need to know the POSSIBLE, not ONLINE CPUs, because eBPF's + // internals require setting up certain buffers for all possible CPUs, even if the CPUs + // don't all exist. let num_cpus = num_possible_cpus().expect("get possible CPUs") as u64; for _ in 0..num_cpus { values.push(value.to_vec()); @@ -1107,7 +1112,7 @@ impl Profiler<'_> { pub fn setup_perf_events(&mut self) { let mut prog_fds = Vec::new(); - for i in 0..num_possible_cpus().expect("get possible CPUs") { + for i in get_online_cpus().expect("get online CPUs") { let perf_fd = unsafe { setup_perf_event(i.try_into().unwrap(), self.sample_freq as u64) } .expect("setup perf event"); diff --git a/src/util.rs b/src/util.rs index 1d5f01b..5af4fd2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,3 +1,6 @@ +use anyhow::{Context, Error}; +use std::{fs::File, io::Read}; + #[derive(Debug, PartialEq)] pub struct AddressBlockRange { pub addr: u64, @@ -33,9 +36,48 @@ pub fn summarize_address_range(low: u64, high: u64) -> Vec { res } +fn _read_cpu_range(path: &str) -> Result, Error> { + let mut cpus: Vec<_> = vec![]; + let mut fh = File::open(path)?; + let mut cpu_range_str = String::new(); + fh.read_to_string(&mut cpu_range_str)?; + + for cpu_range in cpu_range_str.split(',') { + let rangeop_result = cpu_range.find('-'); + match rangeop_result { + None => cpus.push( + cpu_range + .trim_end() + .parse::() + .with_context(|| "Failed to parse lone CPU".to_string())?, + ), + Some(index) => { + let start = cpu_range[..index] + .trim_end() + .parse::() + .with_context(|| "Failed to parse starting CPU".to_string())?; + let end = cpu_range[index + 1..] + .trim_end() + .parse::() + .with_context(|| "Failed to parse ending CPU".to_string())?; + cpus.extend(start..end + 1); + } + } + } + + Ok(cpus) +} + +pub fn get_online_cpus() -> Result, Error> { + let cpus: Vec = _read_cpu_range("/sys/devices/system/cpu/online")?; + + Ok(cpus) +} + #[cfg(test)] mod tests { use std::mem::size_of; + use tempdir::TempDir; use libbpf_rs::libbpf_sys; use libbpf_rs::MapFlags; @@ -155,4 +197,50 @@ mod tests { assert_eq!(parsed.executable_id, mapping2.executable_id); } } + + #[test] + fn cpu_ranges_to_list() { + use std::io::Seek; + use std::io::Write; + + let tmp_dir = TempDir::new("cpu_devs").unwrap(); + let file_path = tmp_dir.path().join("online"); + let mut tmp_file = File::create(file_path.clone()).unwrap(); + let file_str = file_path.to_str().unwrap(); + + writeln!(tmp_file, "0").unwrap(); + let cpus = _read_cpu_range(file_str).unwrap(); + assert_eq!(cpus, vec![0]); + + tmp_file.rewind().unwrap(); + writeln!(tmp_file, "0-7").unwrap(); + + let cpus = _read_cpu_range(file_str).unwrap(); + assert_eq!(cpus, (0..=7).collect::>()); + + tmp_file.rewind().unwrap(); + writeln!(tmp_file, "0-7,16-23").unwrap(); + + let cpus = _read_cpu_range(file_str).unwrap(); + let expected = (0..=7).chain(16..=23).collect::>(); + + assert_eq!(cpus, expected); + + tmp_file.rewind().unwrap(); + writeln!(tmp_file, "0-1,3,7-9,48,49").unwrap(); + + let cpus = _read_cpu_range(file_str).unwrap(); + assert_eq!( + cpus, + (0..=1) + .chain(3..=3) + .chain(7..=9) + .chain(48..=48) + .chain(49..=49) + .collect::>() + ); + + drop(tmp_file); + tmp_dir.close().expect("tempdir should be destroyed"); + } }