Merge pull request raspberrypi#33 from dschatzberg/fbprocfs

scx: atropos: Use fb-procfs crate

Author: htejun

tools/sched_ext/scx_atropos/Cargo.toml

@@ -11,6 +11,7 @@ anyhow = "1.0.65"
 bitvec = { version = "1.0", features = ["serde"] }
 clap = { version = "4.1", features = ["derive", "env", "unicode", "wrap_help"] }
 ctrlc = { version = "3.1", features = ["termination"] }
+fb_procfs = "0.7.0"
 hex = "0.4.3"
 libbpf-rs = "0.21.0"
 libbpf-sys = { version = "1.2.0", features = ["novendor", "static"] }

tools/sched_ext/scx_atropos/src/main.rs

@@ -19,6 +19,7 @@ use std::sync::Arc;
 use std::time::Duration;
 use std::time::Instant;
 
+use ::fb_procfs as procfs;
 use anyhow::anyhow;
 use anyhow::bail;
 use anyhow::Context;
@@ -157,77 +158,61 @@ fn format_cpumask(cpumask: &[u64], nr_cpus: usize) -> String {
         .fold(String::new(), |acc, x| format!("{} {:016X}", acc, x))
 }
 
-// Neither procfs or fb_procfs can determine per-CPU utilization reliably
-// with CPU hot[un]plugs. Roll our own.
-//
-// https://github.com/eminence/procfs/issues/274
-// https://github.com/facebookincubator/below/issues/8190
-#[derive(Clone, Debug, Default)]
-struct MyCpuStat {
-    user: u64,
-    nice: u64,
-    system: u64,
-    idle: u64,
-    iowait: u64,
-    irq: u64,
-    softirq: u64,
-    steal: u64,
+fn read_total_cpu(reader: &procfs::ProcReader) -> Result<procfs::CpuStat> {
+    Ok(reader
+        .read_stat()
+        .context("Failed to read procfs")?
+        .total_cpu
+        .ok_or_else(|| anyhow!("Could not read total cpu stat in proc"))?)
 }
 
-impl MyCpuStat {
-    fn busy_and_total(&self) -> (u64, u64) {
-        let busy = self.user + self.system + self.nice + self.irq + self.softirq + self.steal;
-        (busy, self.idle + busy + self.iowait)
-    }
-
-    fn calc_util(&self, prev: &MyCpuStat) -> f64 {
-        let (curr_busy, curr_total) = self.busy_and_total();
-        let (prev_busy, prev_total) = prev.busy_and_total();
-        let busy = curr_busy - prev_busy;
-        let total = curr_total - prev_total;
-        if total > 0 {
-            ((busy as f64) / (total as f64)).clamp(0.0, 1.0)
-        } else {
-            1.0
-        }
-    }
-}
-
-#[derive(Clone, Debug, Default)]
-struct MyProcStat {
-    total: MyCpuStat,
-    cpus: BTreeMap<usize, MyCpuStat>,
-}
-
-impl MyProcStat {
-    fn read() -> Result<Self> {
-        let mut result: MyProcStat = Default::default();
-        for line in std::fs::read_to_string("/proc/stat")?.lines() {
-            let mut toks = line.split_whitespace();
-
-            let key = toks.next().ok_or(anyhow!("no key"))?;
-            if !key.starts_with("cpu") {
-                break;
-            }
-
-            let cputime = MyCpuStat {
-                user: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                nice: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                system: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                idle: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                iowait: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                irq: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                softirq: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-                steal: toks.next().ok_or(anyhow!("missing"))?.parse::<u64>()?,
-            };
-
-            if key.len() == 3 {
-                result.total = cputime;
+fn calc_util(curr: &procfs::CpuStat, prev: &procfs::CpuStat) -> Result<f64> {
+    match (curr, prev) {
+        (
+            procfs::CpuStat {
+                user_usec: Some(prev_user),
+                nice_usec: Some(prev_nice),
+                system_usec: Some(prev_system),
+                idle_usec: Some(prev_idle),
+                iowait_usec: Some(prev_iowait),
+                irq_usec: Some(prev_irq),
+                softirq_usec: Some(prev_softirq),
+                stolen_usec: Some(prev_stolen),
+                ..
+            },
+            procfs::CpuStat {
+                user_usec: Some(curr_user),
+                nice_usec: Some(curr_nice),
+                system_usec: Some(curr_system),
+                idle_usec: Some(curr_idle),
+                iowait_usec: Some(curr_iowait),
+                irq_usec: Some(curr_irq),
+                softirq_usec: Some(curr_softirq),
+                stolen_usec: Some(curr_stolen),
+                ..
+            },
+        ) => {
+            let idle_usec = curr_idle - prev_idle;
+            let iowait_usec = curr_iowait - prev_iowait;
+            let user_usec = curr_user - prev_user;
+            let system_usec = curr_system - prev_system;
+            let nice_usec = curr_nice - prev_nice;
+            let irq_usec = curr_irq - prev_irq;
+            let softirq_usec = curr_softirq - prev_softirq;
+            let stolen_usec = curr_stolen - prev_stolen;
+
+            let busy_usec =
+                user_usec + system_usec + nice_usec + irq_usec + softirq_usec + stolen_usec;
+            let total_usec = idle_usec + busy_usec + iowait_usec;
+            if total_usec > 0 {
+                return Ok(((busy_usec as f64) / (total_usec as f64)).clamp(0.0, 1.0));
             } else {
-                result.cpus.insert(key[3..].parse::<usize>()?, cputime);
+                return Ok(1.0);
             }
         }
-        Ok(result)
+        _ => {
+            bail!("Missing stats in cpustat");
+        }
     }
 }
 
@@ -394,38 +379,50 @@ struct Tuner {
     top: Arc<Topology>,
     direct_greedy_under: f64,
     kick_greedy_under: f64,
-    prev_cpu_stats: BTreeMap<usize, MyCpuStat>,
+    proc_reader: procfs::ProcReader,
+    prev_cpu_stats: BTreeMap<u32, procfs::CpuStat>,
     dom_utils: Vec<f64>,
 }
 
 impl Tuner {
     fn new(top: Arc<Topology>, opts: &Opts) -> Result<Self> {
+        let proc_reader = procfs::ProcReader::new();
+        let prev_cpu_stats = proc_reader
+            .read_stat()?
+            .cpus_map
+            .ok_or_else(|| anyhow!("Expected cpus_map to exist"))?;
         Ok(Self {
             direct_greedy_under: opts.direct_greedy_under / 100.0,
             kick_greedy_under: opts.kick_greedy_under / 100.0,
-            prev_cpu_stats: MyProcStat::read()?.cpus,
+            proc_reader,
+            prev_cpu_stats,
             dom_utils: vec![0.0; top.nr_doms],
             top,
         })
     }
 
     fn step(&mut self, skel: &mut AtroposSkel) -> Result<()> {
-        let curr_cpu_stats = MyProcStat::read()?.cpus;
+        let curr_cpu_stats = self
+            .proc_reader
+            .read_stat()?
+            .cpus_map
+            .ok_or_else(|| anyhow!("Expected cpus_map to exist"))?;
         let ti = &mut skel.bss().tune_input;
         let mut dom_nr_cpus = vec![0; self.top.nr_doms];
         let mut dom_util_sum = vec![0.0; self.top.nr_doms];
 
         for cpu in 0..self.top.nr_cpus {
+            let cpu32 = cpu as u32;
             // None domain indicates the CPU was offline during
-            // initialization and None MyCpuStat indicates the CPU has gone
+            // initialization and None CpuStat indicates the CPU has gone
             // down since then. Ignore both.
             if let (Some(dom), Some(curr), Some(prev)) = (
                 self.top.cpu_dom[cpu],
-                curr_cpu_stats.get(&cpu),
-                self.prev_cpu_stats.get(&cpu),
+                curr_cpu_stats.get(&cpu32),
+                self.prev_cpu_stats.get(&cpu32),
             ) {
                 dom_nr_cpus[dom] += 1;
-                dom_util_sum[dom] += curr.calc_util(prev);
+                dom_util_sum[dom] += calc_util(curr, prev)?;
             }
         }
 
@@ -843,9 +840,10 @@ struct Scheduler<'a> {
     balanced_kworkers: bool,
 
     top: Arc<Topology>,
+    proc_reader: procfs::ProcReader,
 
     prev_at: Instant,
-    prev_total_cpu: MyCpuStat,
+    prev_total_cpu: procfs::CpuStat,
     task_loads: BTreeMap<i32, TaskLoad>,
 
     nr_lb_data_errors: u64,
@@ -914,7 +912,8 @@ impl<'a> Scheduler<'a> {
         info!("Atropos Scheduler Attached");
 
         // Other stuff.
-        let prev_total_cpu = MyProcStat::read()?.total;
+        let proc_reader = procfs::ProcReader::new();
+        let prev_total_cpu = read_total_cpu(&proc_reader)?;
 
         Ok(Self {
             skel,
@@ -927,6 +926,7 @@ impl<'a> Scheduler<'a> {
             balanced_kworkers: opts.balanced_kworkers,
 
             top: top.clone(),
+            proc_reader,
 
             prev_at: Instant::now(),
             prev_total_cpu,
@@ -939,8 +939,53 @@ impl<'a> Scheduler<'a> {
     }
 
     fn get_cpu_busy(&mut self) -> Result<f64> {
-        let total_cpu = MyProcStat::read()?.total;
-        let busy = total_cpu.calc_util(&self.prev_total_cpu);
+        let total_cpu = read_total_cpu(&mut self.proc_reader)?;
+        let busy = match (&self.prev_total_cpu, &total_cpu) {
+            (
+                procfs::CpuStat {
+                    user_usec: Some(prev_user),
+                    nice_usec: Some(prev_nice),
+                    system_usec: Some(prev_system),
+                    idle_usec: Some(prev_idle),
+                    iowait_usec: Some(prev_iowait),
+                    irq_usec: Some(prev_irq),
+                    softirq_usec: Some(prev_softirq),
+                    stolen_usec: Some(prev_stolen),
+                    guest_usec: _,
+                    guest_nice_usec: _,
+                },
+                procfs::CpuStat {
+                    user_usec: Some(curr_user),
+                    nice_usec: Some(curr_nice),
+                    system_usec: Some(curr_system),
+                    idle_usec: Some(curr_idle),
+                    iowait_usec: Some(curr_iowait),
+                    irq_usec: Some(curr_irq),
+                    softirq_usec: Some(curr_softirq),
+                    stolen_usec: Some(curr_stolen),
+                    guest_usec: _,
+                    guest_nice_usec: _,
+                },
+            ) => {
+                let idle_usec = curr_idle - prev_idle;
+                let iowait_usec = curr_iowait - prev_iowait;
+                let user_usec = curr_user - prev_user;
+                let system_usec = curr_system - prev_system;
+                let nice_usec = curr_nice - prev_nice;
+                let irq_usec = curr_irq - prev_irq;
+                let softirq_usec = curr_softirq - prev_softirq;
+                let stolen_usec = curr_stolen - prev_stolen;
+
+                let busy_usec =
+                    user_usec + system_usec + nice_usec + irq_usec + softirq_usec + stolen_usec;
+                let total_usec = idle_usec + busy_usec + iowait_usec;
+                busy_usec as f64 / total_usec as f64
+            }
+            _ => {
+                bail!("Some procfs stats are not populated!");
+            }
+        };
+
         self.prev_total_cpu = total_cpu;
         Ok(busy)
     }