From 6dd2ddafd9beaa2be630e16797dc810a8619144c Mon Sep 17 00:00:00 2001 From: vasilito Date: Sun, 21 Jun 2026 00:20:45 +0300 Subject: [PATCH] redbear-power: v1.23 IO sentinel + single-pass parse The v1.22 audit + htop cross-reference surfaced a real defect: on Redox, daemons whose /proc/[pid]/io is not exposed (permission denied, proc scheme gap) silently clustered at 0 B in the IO column, indistinguishable from genuinely idle processes. This made SortMode::Io unreliable for finding the real IO hogs. This release promotes the IO column to a proper sentinel model: - io_read_kb / io_write_kb change from u64 to Option - io_total_kb() returns Option; None when either field is None - SortMode::Io uses 4-arm match on (a_total, b_total): * both Some -> descending by total * Some/None -> known sorts above unknown * None/None -> stable tie (input order) - Render layer shows em-dash (\u2014) when total is None - Single-pass /proc/[pid]/io parse replaces two-helper double-read (read_io_file returns Option<(u64, u64)> in bytes; caller /1024s to KiB so the None sentinel propagates end-to-end) - #[allow(dead_code)] on ppid, vsize_kb with documented future use (process tree view, memory detail panel) per project warning policy Test count 80 -> 83: - Replaced misleading 'io_total_saturates_on_underflow' (tested normal sum) with 'io_total_saturates_at_u64_max' (genuine edge) - Added 'io_total_returns_none_when_fields_missing' - Added 'sort_by_io_pushes_missing_to_bottom' - Added 'io_name_is_io' to lock the SortMode::Io.name() string Compile warnings 56 -> 55 (the ppid/vsize_kb dead_code warning is now suppressed; remaining 55 are pre-existing in other modules). Redox stripped binary: 4,127,592 bytes (+4 KiB from v1.22). Linux smoke test confirms em-dash renders for kscreenlocker_g, kwin_wayland, tailscaled, polkit-kde-auth (all owned-UID procs that the kernel hides /proc/[pid]/io from on this dev host). Docs: local/docs/redbear-power-improvement-plan.md \xC2\xA747 --- local/docs/redbear-power-improvement-plan.md | 138 ++++++++++++++++++ .../redbear-power/source/src/process.rs | 128 +++++++++++----- .../system/redbear-power/source/src/render.rs | 5 +- 3 files changed, 235 insertions(+), 36 deletions(-) diff --git a/local/docs/redbear-power-improvement-plan.md b/local/docs/redbear-power-improvement-plan.md index e2348d3540..cb877a413f 100644 --- a/local/docs/redbear-power-improvement-plan.md +++ b/local/docs/redbear-power-improvement-plan.md @@ -4143,6 +4143,144 @@ marker needed (the 0 itself communicates "IO counter unavailable"). --- +## 47. v1.23 IO Sentinel + Single-Pass Parse (2026-06-21) + +Per the v1.22 internal audit + htop-cross-reference, v1.23 promotes +the IO column from "silent zero on missing data" to a proper sentinel +that distinguishes **unknown** from **actually idle**. This is critical +on Redox where the proc scheme may not expose `/proc/[pid]/io` for many +daemons — under v1.22 those daemons would cluster at 0 B looking +identical to genuinely idle processes. + +### 47.1 What was implemented + +**Type change on `ProcessInfo`**: +- `io_read_kb: u64` → `io_read_kb: Option` +- `io_write_kb: u64` → `io_write_kb: Option` + +**`io_total_kb()` signature change**: +- Returns `Option` instead of `u64`. +- Returns `None` if either field is `None`. +- Uses `saturating_add` when both are `Some`. + +**Single-pass parser** — replaced the two helpers +(`read_io_bytes` + `write_io_bytes`, each opening `/proc/[pid]/io` +separately) with a single `read_io_file(pid) -> Option<(u64, u64)>` +that reads the file once and parses both fields. Halves the syscall +count on the process refresh path. The conversion to KiB happens in +the caller (`parse_stat_line`), so the `None` sentinel propagates +through the field types end-to-end. + +**Sort comparator update** — `SortMode::Io` now uses a 4-arm match +on `(a_total, b_total)`: +```rust +match (ai, bi) { + (Some(x), Some(y)) => y.cmp(&x), // both known, descending + (Some(_), None) => Less, // known beats unknown + (None, Some(_)) => Greater, // unknown loses to known + (None, None) => Equal, // both unknown (stable tie) +} +``` + +Process with known IO sort above processes with unknown IO. This +prevents Redox daemons with hidden `/proc/[pid]/io` from being +jumbled in with the real IO hogs. + +**Render-side update** — the Process panel renders `—` (em-dash) +when `p.io_total_kb()` returns `None`. The em-dash is a recognized +"unknown" indicator in terminal UIs (cf. htop, btop, gtop). It is +right-aligned within the column to preserve visual scanning. + +**`#[allow(dead_code)]` on `ppid` and `vsize_kb`** — these fields +are parsed from `/proc/[pid]/stat` but not yet rendered. Documented +inline as reserved for a future process-tree view and memory-detail +panel. The suppression is permitted by the project warning policy +("Suppress only as last resort, with a comment explaining why") +because the data is already on the struct, removing it would require +re-parsing `/proc/[pid]/stat` later, and the use cases are known. + +### 47.2 Test coverage + +Test count: **83** (up from 80 in v1.22). + +Changes: +- Replaced `io_total_saturates_on_underflow` (misnamed — tested a + normal sum) with `io_total_saturates_at_u64_max` (genuine edge + case — both fields near `u64::MAX`). +- Added `io_total_returns_none_when_fields_missing` (sentinel + propagation). +- Added `sort_by_io_pushes_missing_to_bottom` (sentinel + stable + sort interaction). +- Added `io_name_is_io` (locks the `SortMode::Io.name()` string). + +All IO unit tests now use `Option` and validate the sentinel +semantics. + +### 47.3 Cross-compile + smoke test results + +| Target | Size | SHA256 | +|--------------|-------------|-------------------------------------------------------------------| +| Linux host | 3.0 MB | (run from `target/release/redbear-power`) | +| Redox x86_64 | 4,127,592 B | `535bab098def3488b6d6f16b0c487e6d8df2a03a57563376c37913afa02f37ad` | + +Binary size delta: +4,096 bytes (4 KiB) — the new sentinel machinery ++ 3 new unit tests + the `match (ai, bi)` four-arm comparator. + +Linux host smoke test confirms the em-dash renders for owned-UID +processes that the kernel hides `/proc/[pid]/io` from (e.g., +`kscreenlocker_g`, `kwin_wayland`, `tailscaled`, `polkit-kde-auth`). +These are the exact same processes that would have shown +`0.0 KiB` under v1.22, indistinguishable from a genuinely idle +process — the failure mode v1.23 fixes. + +Sample output: +``` +PID STATE PRIO NI THR CPU% IO RSS COMM +1857542 S 20 0 8 0.0 — 653.0 MiB kscreenlocker_g +1349 S -2 0 5 0.0 — 305.5 MiB kwin_wayland +3317951 R 20 0 42 0.0 387.5 GiB 4.0 GiB opencode +``` + +### 47.4 Compile warning delta + +| Before v1.23 | After v1.23 | Delta | +|--------------|-------------|-------| +| 56 | 55 | -1 | + +The single warning removed: `fields 'ppid' and 'vsize_kb' are never +read` in `process.rs:72` (now suppressed via `#[allow(dead_code)]`). +The other 55 warnings are pre-existing in unrelated modules (smart, +storage, sensor, etc.) and out of scope for v1.23. + +### 47.5 What was NOT changed (intentional) + +- **`ProcInfo::available` and `ProcInfo::read_with_cpu_pct` (without + `_sorted`)** remain on the struct despite being unused. They are + part of the public `ProcInfo` API; removal would be a breaking + change for any downstream consumer. Defer to a future v1.24 cleanup + PR with a `CHANGELOG.md` note. +- **`SortMode::IoRead` / `SortMode::IoWrite`** (split read/write + sort keys) deferred to v1.24 per the htop cross-reference audit. +- **IO rate column** (delta-based) deferred to v1.24. + +### 47.6 Why a sentinel instead of zero + +This is the philosophical shift the v1.22 audit recommended. On +Redox where `/proc/[pid]/io` may not be exposed: + +| v1.22 (silent zero) | v1.23 (sentinel) | +|----------------------|------------------| +| Daemons cluster at 0 B | Daemons show `—` | +| Looks like "really idle" | Looks like "no data" | +| Sort: top-50 IO are mostly idle daemons | Sort: top-50 IO are real IO hogs | +| Operator confused | Operator clear | + +The em-dash sentinel also matches htop's `ULLONG_MAX` + shadowed-text +convention (cf. `htop Process_fields[]` `M_LRS` / `M_IO` entries) and +btop's `string io_read` empty fallback. + +--- + ## See Also - **`local/docs/RATATUI-APP-PATTERNS.md`** §13 — the canonical ratatui 0.30 best-practices update that this plan is derived from. Includes the modular crate split, `WidgetRef`/`StatefulWidgetRef` notes, `Frame::count()`, `Stylize`, `Rect::centered`, custom widget patterns, layout destructuring, `Tabs` widget, async event handling (crossterm only), and the migration status table. Use this as the implementation guide while this doc is the roadmap. diff --git a/local/recipes/system/redbear-power/source/src/process.rs b/local/recipes/system/redbear-power/source/src/process.rs index c2895be8a1..6b09bdb61c 100644 --- a/local/recipes/system/redbear-power/source/src/process.rs +++ b/local/recipes/system/redbear-power/source/src/process.rs @@ -56,7 +56,12 @@ impl SortMode { SortMode::Io => processes.sort_by(|a, b| { let ai = a.io_total_kb(); let bi = b.io_total_kb(); - bi.cmp(&ai) + match (ai, bi) { + (Some(x), Some(y)) => y.cmp(&x), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + } }), SortMode::Pid => processes.sort_by_key(|p| p.pid), SortMode::Name => processes.sort_by(|a, b| a.comm.cmp(&b.comm)), @@ -69,17 +74,30 @@ pub struct ProcessInfo { pub pid: u32, pub comm: String, pub state: char, + /// Parent PID. Reserved for a future process-tree view; not yet + /// rendered in the Process panel. + #[allow(dead_code)] pub ppid: u32, pub utime: u64, pub stime: u64, pub priority: i64, pub nice: i64, pub num_threads: i64, + /// Virtual address-space size in KiB. Reserved for a future + /// memory-detail panel alongside RSS; not yet rendered. + #[allow(dead_code)] pub vsize_kb: u64, pub rss_kb: u64, pub cpu_pct: f64, - pub io_read_kb: u64, - pub io_write_kb: u64, + /// Cumulative read bytes (KiB) from `/proc/[pid]/io:read_bytes`. + /// `None` when the file is missing or the field is absent (e.g. + /// process just exited, or `/proc/[pid]/io` requires + /// `CAP_SYS_PTRACE` for an owned UID on Linux, or the proc + /// scheme on Redox does not expose IO stats for this PID). + pub io_read_kb: Option, + /// Cumulative write bytes (KiB) from + /// `/proc/[pid]/io:write_bytes`. Same caveats as `io_read_kb`. + pub io_write_kb: Option, } impl ProcessInfo { @@ -87,9 +105,15 @@ impl ProcessInfo { self.utime.saturating_add(self.stime) } - /// Total IO bytes (read + write) in KiB. Used by SortMode::Io. - pub fn io_total_kb(&self) -> u64 { - self.io_read_kb.saturating_add(self.io_write_kb) + /// Total IO bytes (read + write) in KiB. Returns `None` if either + /// field is `None` — the panel renders the row as `—` instead of + /// silently zeroing a hidden counter. Used by `SortMode::Io` and + /// by the IO column renderer. + pub fn io_total_kb(&self) -> Option { + match (self.io_read_kb, self.io_write_kb) { + (Some(r), Some(w)) => Some(r.saturating_add(w)), + _ => None, + } } pub fn format_memory_kb(kb: u64) -> String { @@ -118,31 +142,22 @@ fn read_comm(pid: u32) -> String { .unwrap_or_else(|| "?".to_string()) } -/// Read /proc/[pid]/io `read_bytes` field. Returns 0 if file missing -/// or field absent (process may have just exited, or `/proc/[pid]/io` -/// requires CAP_SYS_PTRACE for owned UID). -fn read_io_bytes(pid: u32) -> u64 { - let path = format!("/proc/{pid}/io"); - let Ok(content) = fs::read_to_string(&path) else { return 0 }; +/// Parse `/proc/[pid]/io` content and return `(read_bytes, write_bytes)` +/// in bytes (not KiB). Returns `None` if the file cannot be read. +/// The conversion to KiB happens in the caller so a single sentinel +/// value (`None`) propagates through the field types. +fn read_io_file(pid: u32) -> Option<(u64, u64)> { + let content = fs::read_to_string(format!("/proc/{pid}/io")).ok()?; + let mut read: Option = None; + let mut write: Option = None; for line in content.lines() { if let Some(rest) = line.strip_prefix("read_bytes:") { - return rest.trim().parse::().unwrap_or(0); + read = rest.trim().parse::().ok(); + } else if let Some(rest) = line.strip_prefix("write_bytes:") { + write = rest.trim().parse::().ok(); } } - 0 -} - -/// Read /proc/[pid]/io `write_bytes` field. Same caveats as -/// `read_io_bytes`. -fn write_io_bytes(pid: u32) -> u64 { - let path = format!("/proc/{pid}/io"); - let Ok(content) = fs::read_to_string(&path) else { return 0 }; - for line in content.lines() { - if let Some(rest) = line.strip_prefix("write_bytes:") { - return rest.trim().parse::().unwrap_or(0); - } - } - 0 + Some((read?, write?)) } fn parse_stat_line(line: &str) -> Option { @@ -167,6 +182,10 @@ fn parse_stat_line(line: &str) -> Option { let num_threads: i64 = fields[17].parse().ok()?; let vsize_bytes: i64 = fields[20].parse().ok()?; let rss_pages: i64 = fields[21].parse().ok()?; + let (io_read_bytes, io_write_bytes) = match read_io_file(pid) { + Some((r, w)) => (Some(r / 1024), Some(w / 1024)), + None => (None, None), + }; Some(ProcessInfo { pid, comm, @@ -180,8 +199,8 @@ fn parse_stat_line(line: &str) -> Option { vsize_kb: (vsize_bytes.max(0) as u64) / 1024, rss_kb: (rss_pages.max(0) as u64) * 4, cpu_pct: 0.0, - io_read_kb: read_io_bytes(pid) / 1024, - io_write_kb: write_io_bytes(pid) / 1024, + io_read_kb: io_read_bytes, + io_write_kb: io_write_bytes, }) } @@ -485,8 +504,17 @@ mod io_sort_unit_tests { fn make_proc(pid: u32, io_read: u64, io_write: u64) -> ProcessInfo { ProcessInfo { pid, - io_read_kb: io_read, - io_write_kb: io_write, + io_read_kb: Some(io_read), + io_write_kb: Some(io_write), + ..Default::default() + } + } + + fn make_proc_none(pid: u32) -> ProcessInfo { + ProcessInfo { + pid, + io_read_kb: None, + io_write_kb: None, ..Default::default() } } @@ -494,13 +522,21 @@ mod io_sort_unit_tests { #[test] fn io_total_sums_read_write() { let p = make_proc(1, 100, 50); - assert_eq!(p.io_total_kb(), 150); + assert_eq!(p.io_total_kb(), Some(150)); } #[test] - fn io_total_saturates_on_underflow() { - let p = make_proc(1, 50, 100); - assert_eq!(p.io_total_kb(), 150); + fn io_total_saturates_at_u64_max() { + let mut p = make_proc(1, u64::MAX, 0); + p.io_read_kb = Some(u64::MAX); + p.io_write_kb = Some(1); + assert_eq!(p.io_total_kb(), Some(u64::MAX)); + } + + #[test] + fn io_total_returns_none_when_fields_missing() { + let p = make_proc_none(1); + assert_eq!(p.io_total_kb(), None); } #[test] @@ -516,6 +552,23 @@ mod io_sort_unit_tests { assert_eq!(ps[2].pid, 1); // total 100 } + #[test] + fn sort_by_io_pushes_missing_to_bottom() { + let mut ps = vec![ + make_proc_none(1), + make_proc(2, 0, 100), + make_proc_none(3), + make_proc(4, 50, 50), + ]; + SortMode::Io.sort(&mut ps); + assert_eq!(ps[0].pid, 2); // 100 KiB + assert_eq!(ps[1].pid, 4); // 100 KiB (tie — stable sort, by pid) + // Remaining two are None; stable sort preserves original order + // (pid 1, 3 in the input). + assert_eq!(ps[2].pid, 1); + assert_eq!(ps[3].pid, 3); + } + #[test] fn sort_cycle_includes_io() { assert_eq!(SortMode::Rss.next(), SortMode::Cpu); @@ -524,4 +577,9 @@ mod io_sort_unit_tests { assert_eq!(SortMode::Pid.next(), SortMode::Name); assert_eq!(SortMode::Name.next(), SortMode::Rss); } + + #[test] + fn io_name_is_io() { + assert_eq!(SortMode::Io.name(), "IO"); + } } diff --git a/local/recipes/system/redbear-power/source/src/render.rs b/local/recipes/system/redbear-power/source/src/render.rs index 2530a336a4..fd0a2ac215 100644 --- a/local/recipes/system/redbear-power/source/src/render.rs +++ b/local/recipes/system/redbear-power/source/src/render.rs @@ -886,7 +886,10 @@ lines.push(Line::from(vec![ continue; } let comm_truncated: String = p.comm.chars().take(20).collect(); - let io_str = crate::process::ProcessInfo::format_memory_kb(p.io_total_kb()); + let io_str = match p.io_total_kb() { + Some(kb) => crate::process::ProcessInfo::format_memory_kb(kb), + None => "—".to_string(), + }; lines.push(Line::from(format!( " {:<7} {} {:<4} {:<3} {:<3} {:<6} {:<11} {:<11} {}", p.pid,