From 3ee883420a4f1b6db29cff2d6cb7273b5b620e68 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Thu, 15 Feb 2024 03:01:45 -0500 Subject: [PATCH] bug: fix uptime calculation for Linux (#1410) * bug: fix uptime for linux Use another calculation to determine the uptime of a process on Linux. * appease clippy * changelog * edit add --- CHANGELOG.md | 1 + src/data_collection/processes.rs | 4 +- src/data_collection/processes/linux.rs | 48 +++++++++++++------ .../processes/linux/process.rs | 16 +++++-- src/widgets/process_table/proc_widget_data.rs | 1 + 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ecab8bc..23b3f516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#1314](https://github.com/ClementTsang/bottom/pull/1314): Fix fat32 mounts not showing up in macOS. - [#1355](https://github.com/ClementTsang/bottom/pull/1355): Reduce chances of non-D0 devices waking up due to temperature checks on Linux. +- [#1410](https://github.com/ClementTsang/bottom/pull/1410): Fix uptime calculation for Linux. ## [0.9.6] - 2023-08-26 diff --git a/src/data_collection/processes.rs b/src/data_collection/processes.rs index 08b48a29..ef96c818 100644 --- a/src/data_collection/processes.rs +++ b/src/data_collection/processes.rs @@ -74,7 +74,7 @@ pub struct ProcessHarvest { /// The current state of the process (e.g. zombie, asleep). pub process_state: (String, char), - /// Cumulative total CPU time used. + /// Cumulative process uptime. pub time: Duration, /// This is the *effective* user ID of the process. This is only used on Unix platforms. @@ -109,7 +109,7 @@ impl ProcessHarvest { self.write_bytes_per_sec += rhs.write_bytes_per_sec; self.total_read_bytes += rhs.total_read_bytes; self.total_write_bytes += rhs.total_write_bytes; - self.time += rhs.time; + self.time = self.time.max(rhs.time); #[cfg(feature = "gpu")] { self.gpu_mem += rhs.gpu_mem; diff --git a/src/data_collection/processes/linux.rs b/src/data_collection/processes/linux.rs index 3533e63e..5d99fbbc 100644 --- a/src/data_collection/processes/linux.rs +++ b/src/data_collection/processes/linux.rs @@ -132,9 +132,7 @@ fn get_linux_cpu_usage( } fn read_proc( - prev_proc: &PrevProcDetails, process: Process, cpu_usage: f64, cpu_fraction: f64, - use_current_cpu_total: bool, time_difference_in_secs: u64, total_memory: u64, - user_table: &mut UserTable, + prev_proc: &PrevProcDetails, process: Process, args: ReadProcArgs, user_table: &mut UserTable, ) -> error::Result<(ProcessHarvest, u64)> { let Process { pid: _, @@ -144,6 +142,15 @@ fn read_proc( cmdline, } = process; + let ReadProcArgs { + use_current_cpu_total, + cpu_usage, + cpu_fraction, + total_memory, + time_difference_in_secs, + uptime, + } = args; + let (command, name) = { let truncated_name = stat.comm.as_str(); if let Ok(cmdline) = cmdline { @@ -231,7 +238,7 @@ fn read_proc( if ticks_per_sec == 0 { Duration::ZERO } else { - Duration::from_secs(stat.utime + stat.stime) / ticks_per_sec + Duration::from_secs(uptime.saturating_sub(stat.start_time / ticks_per_sec as u64)) } } else { Duration::ZERO @@ -279,6 +286,17 @@ fn is_str_numeric(s: &str) -> bool { s.chars().all(|c| c.is_ascii_digit()) } +/// General args to keep around for reading proc data. +#[derive(Copy, Clone)] +pub(crate) struct ReadProcArgs { + pub(crate) use_current_cpu_total: bool, + pub(crate) cpu_usage: f64, + pub(crate) cpu_fraction: f64, + pub(crate) total_memory: u64, + pub(crate) time_difference_in_secs: u64, + pub(crate) uptime: u64, +} + pub(crate) fn linux_process_data( collector: &mut DataCollector, time_difference_in_secs: u64, ) -> error::Result> { @@ -329,6 +347,15 @@ pub(crate) fn linux_process_data( } }); + let args = ReadProcArgs { + use_current_cpu_total, + cpu_usage, + cpu_fraction, + total_memory, + time_difference_in_secs, + uptime: sysinfo::System::uptime(), + }; + let process_vector: Vec = pids .filter_map(|pid_path| { if let Ok(process) = Process::from_path(pid_path) { @@ -336,16 +363,9 @@ pub(crate) fn linux_process_data( let prev_proc_details = pid_mapping.entry(pid).or_default(); #[allow(unused_mut)] - if let Ok((mut process_harvest, new_process_times)) = read_proc( - prev_proc_details, - process, - cpu_usage, - cpu_fraction, - use_current_cpu_total, - time_difference_in_secs, - total_memory, - user_table, - ) { + if let Ok((mut process_harvest, new_process_times)) = + read_proc(prev_proc_details, process, args, user_table) + { #[cfg(feature = "gpu")] if let Some(gpus) = &collector.gpu_pids { gpus.iter().for_each(|gpu| { diff --git a/src/data_collection/processes/linux/process.rs b/src/data_collection/processes/linux/process.rs index 6841b0d5..bb1d07a6 100644 --- a/src/data_collection/processes/linux/process.rs +++ b/src/data_collection/processes/linux/process.rs @@ -26,7 +26,8 @@ fn next_part<'a>(iter: &mut impl Iterator) -> Result<&'a str, io .ok_or_else(|| io::Error::from(io::ErrorKind::InvalidData)) } -/// A wrapper around the data in `/proc//stat`. For documentation, see [here](https://man7.org/linux/man-pages/man5/proc.5.html). +/// A wrapper around the data in `/proc//stat`. For documentation, see +/// [here](https://man7.org/linux/man-pages/man5/proc.5.html). /// /// Note this does not necessarily get all fields, only the ones we use in bottom. pub(crate) struct Stat { @@ -47,6 +48,9 @@ pub(crate) struct Stat { /// The resident set size, or the number of pages the process has in real memory. pub rss: u64, + + /// The start time of the process, represented in clock ticks. + pub start_time: u64, } impl Stat { @@ -76,18 +80,19 @@ impl Stat { .chars() .next() .ok_or_else(|| anyhow!("missing state"))?; - let ppid: Pid = next_part(&mut rest)?.parse()?; // Skip 9 fields until utime (pgrp, session, tty_nr, tpgid, flags, minflt, cminflt, majflt, cmajflt). let mut rest = rest.skip(9); - let utime: u64 = next_part(&mut rest)?.parse()?; let stime: u64 = next_part(&mut rest)?.parse()?; - // Skip 8 fields until rss (cutime, cstime, priority, nice, num_threads, itrealvalue, starttime, vsize). - let mut rest = rest.skip(8); + // Skip 6 fields until starttime (cutime, cstime, priority, nice, num_threads, itrealvalue). + let mut rest = rest.skip(6); + let start_time: u64 = next_part(&mut rest)?.parse()?; + // Skip one field until rss (vsize) + let mut rest = rest.skip(1); let rss: u64 = next_part(&mut rest)?.parse()?; Ok(Stat { @@ -97,6 +102,7 @@ impl Stat { utime, stime, rss, + start_time, }) } diff --git a/src/widgets/process_table/proc_widget_data.rs b/src/widgets/process_table/proc_widget_data.rs index 5f454662..0537a474 100644 --- a/src/widgets/process_table/proc_widget_data.rs +++ b/src/widgets/process_table/proc_widget_data.rs @@ -263,6 +263,7 @@ impl ProcWidgetData { self.wps += other.wps; self.total_read += other.total_read; self.total_write += other.total_write; + self.time = self.time.max(other.time); #[cfg(feature = "gpu")] { self.gpu_mem_usage = match (&self.gpu_mem_usage, &other.gpu_mem_usage) {