From d3401b5916707633031a7ba210176eff7473dfd8 Mon Sep 17 00:00:00 2001 From: manunicholasjacob Date: Wed, 14 Oct 2026 08:03:56 -0500 Subject: [PATCH] fix(sensors): handle RAPL energy counter wraparound powercap energy_uj counters wrap to 0 after max_energy_range_uj. Read that range for socket, domain and psys zones in generate_topology, and use a counter_delta helper in the power and energy diff functions so a wrap gives the real delta. When the range is unknown, the socket path now returns None instead of reporting 0 W. Fixes #280 Co-Authored-By: Claude Opus 5.5 Signed-off-by: manunicholasjacob --- src/sensors/mod.rs | 182 +++++++++++++++++++++++++++++++---- src/sensors/powercap_rapl.rs | 108 +++++++++++++++++++++ 2 files changed, 270 insertions(+), 20 deletions(-) diff --git a/src/sensors/mod.rs b/src/sensors/mod.rs index 3c033566..72ba936c 100644 --- a/src/sensors/mod.rs +++ b/src/sensors/mod.rs @@ -38,6 +38,30 @@ pub trait RecordReader { fn read_record(&self) -> Result>; } +/// Returns the energy consumed between two readings of a RAPL energy counter. +/// +/// RAPL counters (powercap `energy_uj`) wrap back to zero after reaching +/// `max_energy_range_uj`. If `last < prev` the counter wrapped and the energy +/// consumed is `(max_range - prev) + last`. Returns None when the counter went +/// backwards and the range is unknown (0) or smaller than `prev`. +pub fn counter_delta(prev: u64, last: u64, max_range: u64) -> Option { + if last >= prev { + Some(last - prev) + } else if max_range > 0 && prev <= max_range { + Some(max_range - prev + last) + } else { + None + } +} + +/// Reads the counter range stored by the sensor under "max_energy_range_uj", 0 if unknown. +fn max_energy_range_uj(sensor_data: &HashMap) -> u64 { + sensor_data + .get("max_energy_range_uj") + .and_then(|v| v.trim().parse::().ok()) + .unwrap_or(0) +} + // !!!!!!!!!!!!!!!!! Topology !!!!!!!!!!!!!!!!!!!!!!! /// Topology struct represents the whole CPUSocket architecture, /// from the electricity consumption point of view, @@ -428,16 +452,41 @@ impl Topology { if len > 2 { let last = self.record_buffer.last().unwrap(); let previous = self.record_buffer.get(len - 2).unwrap(); - let last_value = last.value.parse::().unwrap(); - let previous_value = previous.value.parse::().unwrap(); - if previous_value <= last_value { - let diff = last_value - previous_value; + let last_value = last.value.trim().parse::().unwrap(); + let previous_value = previous.value.trim().parse::().unwrap(); + if let Some(diff) = + counter_delta(previous_value, last_value, self.host_energy_max_range_uj()) + { return Some(Record::new(last.timestamp, diff.to_string(), last.unit)); } } None } + /// Returns the wraparound range of the host energy counter, or 0 if unknown. + /// The range is only known when the host record comes from a single + /// counter: psys, or the PKG counter of a single socket without a DRAM domain. + /// A sum of several counters that wrap independently has no single range. + fn host_energy_max_range_uj(&self) -> u64 { + if self._sensor_data.contains_key("psys") { + return max_energy_range_uj(&self._sensor_data); + } + let mut counters = vec![]; + for s in &self.sockets { + counters.push(&s.sensor_data); + for d in &s.domains { + if d.name == "dram" { + counters.push(&d.sensor_data); + } + } + } + if counters.len() == 1 { + max_energy_range_uj(counters[0]) + } else { + 0 + } + } + /// Returns a Record instance containing the power consumed between /// last and previous measurement, in microwatts. pub fn get_records_diff_power_microwatts(&self) -> Option { @@ -447,13 +496,14 @@ impl Topology { .record_buffer .get(self.record_buffer.len() - 2) .unwrap(); - match previous_record.value.trim().parse::() { - Ok(previous_microjoules) => match last_record.value.trim().parse::() { + match previous_record.value.trim().parse::() { + Ok(previous_microjoules) => match last_record.value.trim().parse::() { Ok(last_microjoules) => { - if previous_microjoules > last_microjoules { - return None; - } - let microjoules = last_microjoules - previous_microjoules; + let microjoules = counter_delta( + previous_microjoules, + last_microjoules, + self.host_energy_max_range_uj(), + )?; let time_diff = last_record.timestamp.as_secs_f64() - previous_record.timestamp.as_secs_f64(); let microwatts = microjoules as f64 / time_diff; @@ -1277,15 +1327,17 @@ impl CPUSocket { if let (Ok(last_microjoules), Ok(previous_microjoules)) = (last_rec_val.parse::(), prev_rec_val.parse::()) { - let mut microjoules = 0; - if last_microjoules >= previous_microjoules { - microjoules = last_microjoules - previous_microjoules; - } else { + let Some(microjoules) = counter_delta( + previous_microjoules, + last_microjoules, + max_energy_range_uj(&self.sensor_data), + ) else { debug!( - "socket: previous_microjoules ({}) > last_microjoules ({})", + "socket: previous_microjoules ({}) > last_microjoules ({}), wraparound could not be resolved", previous_microjoules, last_microjoules ); - } + return None; + }; let time_diff = last_record.timestamp.as_secs_f64() - previous_record.timestamp.as_secs_f64(); let microwatts = microjoules as f64 / time_diff; @@ -1459,10 +1511,11 @@ impl Domain { last_record.value.trim().parse::(), previous_record.value.trim().parse::(), ) { - if previous_microjoules > last_microjoules { - return None; - } - let microjoules = last_microjoules - previous_microjoules; + let microjoules = counter_delta( + previous_microjoules, + last_microjoules, + max_energy_range_uj(&self.sensor_data), + )?; let time_diff = last_record.timestamp.as_secs_f64() - previous_record.timestamp.as_secs_f64(); let microwatts = microjoules as f64 / time_diff; @@ -1659,6 +1712,95 @@ mod tests { println!("{:?}", s.read_stats()); } } + + fn uj_record(secs: u64, value: u64) -> Record { + Record::new( + Duration::from_secs(secs), + value.to_string(), + units::Unit::MicroJoule, + ) + } + + fn range_data(max: Option) -> HashMap { + let mut data = HashMap::new(); + if let Some(max) = max { + data.insert(String::from("max_energy_range_uj"), max.to_string()); + } + data + } + + #[test] + fn counter_delta_handles_wraparound() { + // no wrap + assert_eq!(counter_delta(100, 350, 1000), Some(250)); + assert_eq!(counter_delta(100, 100, 0), Some(0)); + // one wrap: 900 -> 1000 (100) then 0 -> 150 (150) + assert_eq!(counter_delta(900, 150, 1000), Some(250)); + // typical Intel package range (61 uJ energy unit) + assert_eq!( + counter_delta(262_143_000_000, 1_000_000, 262_143_328_850), + Some(1_328_850) + ); + // range unknown: cannot tell how much energy was consumed + assert_eq!(counter_delta(900, 150, 0), None); + // inconsistent range + assert_eq!(counter_delta(900, 150, 500), None); + } + + #[test] + fn socket_power_across_wrap() { + let mut socket = + CPUSocket::new(0, vec![], vec![], String::new(), 1, range_data(Some(1000))); + socket.record_buffer = vec![uj_record(10, 900), uj_record(12, 150)]; + let power = socket.get_records_diff_power_microwatts().unwrap(); + assert_eq!(power.value, "125"); + // unknown range: no record instead of a fabricated 0 W + socket.sensor_data = range_data(None); + assert!(socket.get_records_diff_power_microwatts().is_none()); + } + + #[test] + fn domain_power_across_wrap() { + let mut domain = Domain::new( + 0, + String::from("dram"), + String::new(), + 1, + range_data(Some(1000)), + ); + domain.record_buffer = vec![uj_record(10, 900), uj_record(12, 150)]; + let power = domain.get_records_diff_power_microwatts().unwrap(); + assert_eq!(power.value, "125"); + domain.sensor_data = range_data(None); + assert!(domain.get_records_diff_power_microwatts().is_none()); + } + + #[test] + fn topology_power_across_wrap() { + // psys: the host record is a single counter + let mut psys_data = range_data(Some(1000)); + psys_data.insert(String::from("psys"), String::from("/nonexistent")); + let mut topo = Topology::new(psys_data); + topo.record_buffer = vec![uj_record(8, 0), uj_record(10, 900), uj_record(12, 150)]; + assert_eq!( + topo.get_records_diff_power_microwatts().unwrap().value, + "125" + ); + assert_eq!(topo.get_records_diff().unwrap().value, "250"); + + // single socket without dram: the host record is the PKG counter + let mut topo = Topology::new(HashMap::new()); + topo.safe_add_socket(0, vec![], vec![], String::new(), 1, range_data(Some(1000))); + topo.record_buffer = vec![uj_record(10, 900), uj_record(12, 150)]; + assert_eq!( + topo.get_records_diff_power_microwatts().unwrap().value, + "125" + ); + + // PKG + DRAM: a sum of independently wrapping counters has no single range + topo.safe_add_domain_to_socket(0, 0, "dram", "", 1, range_data(Some(1000))); + assert!(topo.get_records_diff_power_microwatts().is_none()); + } } // Copyright 2020 The scaphandre authors. diff --git a/src/sensors/powercap_rapl.rs b/src/sensors/powercap_rapl.rs index 639b7db9..d6d6f5e9 100644 --- a/src/sensors/powercap_rapl.rs +++ b/src/sensors/powercap_rapl.rs @@ -70,6 +70,17 @@ impl PowercapRAPLSensor { } } +/// Stores the zone's max_energy_range_uj (the value at which its energy_uj +/// counter wraps) in sensor_data, if the kernel exposes it. +fn insert_max_energy_range(sensor_data: &mut HashMap, zone_folder: &str) { + if let Ok(max) = fs::read_to_string(format!("{zone_folder}/max_energy_range_uj")) { + sensor_data.insert( + String::from("max_energy_range_uj"), + String::from(max.trim()), + ); + } +} + impl RecordReader for Topology { fn read_record(&self) -> Result> { // if psys is available, return psys @@ -171,6 +182,10 @@ impl Sensor for PowercapRAPLSensor { String::from("source_file"), format!("{}/intel-rapl:{}/energy_uj", self.base_path, socket_id), ); + insert_max_energy_range( + &mut sensor_data_for_socket, + &format!("{}/intel-rapl:{}", self.base_path, socket_id), + ); topo.safe_add_socket( socket_id, vec![], @@ -187,6 +202,7 @@ impl Sensor for PowercapRAPLSensor { self.base_path, socket_id, domain_id ), ); + insert_max_energy_range(&mut sensor_data_for_domain, &folder_name); if let Ok(domain_name) = &fs::read_to_string(format!("{folder_name}/name")) { topo.safe_add_domain_to_socket( socket_id, @@ -250,6 +266,7 @@ impl Sensor for PowercapRAPLSensor { String::from("source_file"), format!("{}/intel-rapl:{}/energy_uj", self.base_path, socket_id), ); + insert_max_energy_range(&mut sensor_data_for_socket, &folder_name); topo.safe_add_socket( socket_id, vec![], @@ -275,6 +292,7 @@ impl Sensor for PowercapRAPLSensor { let domain_name_trimed = domain_name.trim(); if domain_name_trimed == "psys" { debug!("Found PSYS domain RAPL folder."); + insert_max_energy_range(&mut topo._sensor_data, &folder_name); topo._sensor_data.insert(String::from("psys"), folder_name); } } @@ -300,6 +318,7 @@ impl Sensor for PowercapRAPLSensor { #[cfg(test)] mod tests { use super::*; + use crate::sensors::RecordGenerator; use std::any::type_name; fn type_of(_: T) -> &'static str { @@ -314,6 +333,95 @@ mod tests { type_of(topology) ) } + + fn write_zone(base: &std::path::Path, zone: &str, name: &str, energy: u64, max: u64) { + let dir = base.join(zone); + fs::create_dir_all(&dir).unwrap(); + // sysfs values end with a newline + fs::write(dir.join("name"), format!("{name}\n")).unwrap(); + fs::write(dir.join("energy_uj"), format!("{energy}\n")).unwrap(); + fs::write(dir.join("max_energy_range_uj"), format!("{max}\n")).unwrap(); + } + + fn set_energy(base: &std::path::Path, zone: &str, energy: u64) { + fs::write(base.join(zone).join("energy_uj"), format!("{energy}\n")).unwrap(); + } + + #[test] + fn powercap_counters_wraparound() { + let base = env::temp_dir().join(format!("scaphandre-powercap-wrap-{}", std::process::id())); + let _ = fs::remove_dir_all(&base); + write_zone( + &base, + "intel-rapl:0", + "package-0", + 262_143_000_000, + 262_143_328_850, + ); + write_zone( + &base, + "intel-rapl:0:0", + "dram", + 65_712_000_000, + 65_712_999_613, + ); + write_zone( + &base, + "intel-rapl:1", + "psys", + 262_143_200_000, + 262_143_328_850, + ); + // SAFETY: SCAPHANDRE_POWERCAP_PATH is only read in --vm mode, which no other test uses. + unsafe { env::set_var("SCAPHANDRE_POWERCAP_PATH", &base) }; + let sensor = PowercapRAPLSensor::new(1, 1, true); + let mut topo = sensor.generate_topology().unwrap(); + + let socket = &topo.sockets[0]; + assert_eq!(socket.sensor_data["max_energy_range_uj"], "262143328850"); + assert_eq!( + socket.domains[0].sensor_data["max_energy_range_uj"], + "65712999613" + ); + assert_eq!(topo._sensor_data["max_energy_range_uj"], "262143328850"); + + let refresh = |topo: &mut Topology| { + topo.refresh_record(); + for s in topo.get_sockets() { + s.refresh_record(); + for d in s.get_domains() { + d.refresh_record(); + } + } + }; + refresh(&mut topo); + std::thread::sleep(std::time::Duration::from_millis(20)); + // every counter wraps between the two reads + set_energy(&base, "intel-rapl:0", 1_000_000); + set_energy(&base, "intel-rapl:0:0", 2_000_000); + set_energy(&base, "intel-rapl:1", 3_000_000); + refresh(&mut topo); + + let expected = |records: &[Record], uj: u64| { + let secs = records[1].timestamp.as_secs_f64() - records[0].timestamp.as_secs_f64(); + ((uj as f64 / secs) as u64).to_string() + }; + let socket = &topo.sockets[0]; + assert_eq!( + socket.get_records_diff_power_microwatts().unwrap().value, + expected(&socket.record_buffer, 328_850 + 1_000_000) + ); + let dram = &socket.domains[0]; + assert_eq!( + dram.get_records_diff_power_microwatts().unwrap().value, + expected(&dram.record_buffer, 999_613 + 2_000_000) + ); + assert_eq!( + topo.get_records_diff_power_microwatts().unwrap().value, + expected(&topo.record_buffer, 128_850 + 3_000_000) + ); + fs::remove_dir_all(&base).unwrap(); + } } // Copyright 2020 The scaphandre authors.