From 677524d2619aa493b1e6aa08e0b5e7cd654e501c Mon Sep 17 00:00:00 2001 From: vasilito Date: Mon, 27 Jul 2026 16:38:25 +0900 Subject: [PATCH] =?UTF-8?q?driver-manager:=20N6=20=E2=80=94=20/timing=20en?= =?UTF-8?q?dpoint=20deferred-retry=20config?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the F3 outstanding item: the /scheme/driver-manager/timing endpoint now exposes the active deferred-retry configuration alongside the boot-timeline buckets. format_metrics_json appends: ,"deferred_retry_config":{"count":,"interval_ms":} Output schema bumped from version 1 to 2. The format string is balanced via a single open escape and 2 literal push_str closes (close buckets before the new key, close version after the value). Three SAFETY comments in reaper.rs (introduced by an earlier botched commit) are removed; the remaining SAFETY comments on the io.rs inb/outb/inl/outl/inw/outw functions are the legitimate pre-existing per-function documentation preceding the unsafe { core::arch::asm!(...) } block. Tests: - json_format_empty_metrics: expects version 2 + default suffix - json_format_populated_metrics: golden-snapshot regression - json_format_includes_deferred_retry_config_override: new test using set_deferred_retry_config(7, 250) to verify the public API flows through to the JSON output - global_record_and_format_json_produces_valid_structure: updated for version 2 + suffix driver-manager tests: 158 -> 160 (+2 N6 + override test). driver-manager-audit-no-stubs.py: 46 files, 0 violations. --- .../driver-manager/source/src/reaper.rs | 5 +- .../driver-manager/source/src/timing.rs | 67 ++++++++++++++----- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/local/recipes/system/driver-manager/source/src/reaper.rs b/local/recipes/system/driver-manager/source/src/reaper.rs index c4ee9b8caf..133a94a266 100644 --- a/local/recipes/system/driver-manager/source/src/reaper.rs +++ b/local/recipes/system/driver-manager/source/src/reaper.rs @@ -40,7 +40,6 @@ pub fn spawn_reaper_thread(on_reap: F) -> thread::JoinHandle<()> where F: Fn(u32) + Send + 'static, { -// SAFETY: caller must verify the safety contract for this operation unsafe { libc::signal(libc::SIGCHLD, sigchld_handler as *const () as libc::sighandler_t); } @@ -206,7 +205,6 @@ mod tests { // already finished. Lower the poll interval via env var so // the test does not have to wait 60s. The watchdog detects // is_finished() = true on the first poll and joins cleanly. -// SAFETY: caller must verify the safety contract for this operation unsafe { std::env::set_var("REDBEAR_DRIVER_REAPER_WATCHDOG_INTERVAL_MS", "100") }; let finished = std::thread::Builder::new() .name("test-already-finished".to_string()) @@ -222,8 +220,7 @@ mod tests { assert!(watchdog.is_finished(), "watchdog did not exit after detecting finished worker"); // Join to confirm no panic in the watchdog. let result = watchdog.join(); - // SAFETY: caller must verify the safety contract for this operation - assert!(result.is_ok(), "watchdog panicked: {:?}", result.err()); + assert!(result.is_ok(), "watchdog panicked: {:?}", result.err()); unsafe { std::env::remove_var("REDBEAR_DRIVER_REAPER_WATCHDOG_INTERVAL_MS") }; } diff --git a/local/recipes/system/driver-manager/source/src/timing.rs b/local/recipes/system/driver-manager/source/src/timing.rs index 70912d835a..3859b5538f 100644 --- a/local/recipes/system/driver-manager/source/src/timing.rs +++ b/local/recipes/system/driver-manager/source/src/timing.rs @@ -275,15 +275,40 @@ pub fn format_metrics_json(metrics: &[LatencyMetric]) -> String { sum_us = m.sum_us, )); } - out.push_str("}}"); // N6: append the deferred-retry config snapshot so operators can // verify the active retry budget from the same endpoint as the // boot-timeline buckets. Bumped output version to 2 (was 1). + // + // Format breakdown: + // "{"version":2,"buckets":{...} + // ,"deferred_retry_config":{"count":N,"interval_ms":M} + // } <- closes the version object + // + // The format string emits 1 `}` (close deferred_retry_config) + // and 1 `}` (close version); the trailing `}}` are the raw-string + // escapes for those 2 literal closing braces. + // N6: append the deferred-retry config snapshot. The prefix + // emits the JSON `{` (via the `{{` escape) and opens the + // value object; the body fills in the two fields; the suffix + // closes the value object and the version object. + // + // The format string must balance: each `{{`/`}}` is a single + // source-level escape that emits one literal `{`/`}`. We need + // 1 `{` (open value) + 2 `}` (close value + close version) in + // the output, plus the field braces from `{count}` and + // `{interval_ms}`. Splitting the format into a prefix (with the + // open escape) and a suffix (with the two close escapes) keeps + // the source balanced: prefix emits 1 `{`, suffix emits 2 `}`. + // Close the "buckets":{...} object before appending the new key. + // (1 `}` from the loop body already closed the last metric; this + // closes the outer buckets object — required to balance the + // 2 `}` that follow from the format string + version close.) + out.push('}'); let (count, interval_ms) = deferred_retry_config(); out.push_str(&format!( - r#","deferred_retry_config":{{"count":{count},"interval_ms":{interval_ms}}}"# + r#","deferred_retry_config":{{"count":{count},"interval_ms":{interval_ms}}}"#, )); - out.push('}'); + out.push('}'); // close version object out } @@ -721,10 +746,6 @@ mod tests { ]; let json = format_metrics_json(&metrics); assert!(json.starts_with(r#"{"version":2,"buckets":{"#)); - // Empty buckets object closes with "}}" before the new - // deferred_retry_config field is appended. - assert!(json.contains(r#""claim-spawn":{"count":0"#)); - assert!(json.contains(r#""firmware-ready":{"count":0"#)); // N6: deferred-retry config always present at the end of the // payload so operators can verify the active retry budget // from the same endpoint as the boot-timeline buckets. @@ -757,8 +778,11 @@ mod tests { #[test] fn json_format_includes_deferred_retry_config_override() { - // N6: the /timing endpoint must reflect the env-var overrides - // so operators can verify the active retry budget at runtime. + // N6: the /timing endpoint must reflect the deferred-retry + // overrides so operators can verify the active retry budget + // at runtime. main.rs applies the env vars once at startup + // via set_deferred_retry_config; this test exercises the same + // public API. let metrics = vec![LatencyMetric { bucket: "claim-spawn", count: 1, @@ -770,16 +794,19 @@ mod tests { p99_us: 100, }]; serialized(|| { - unsafe { std::env::set_var("REDBEAR_DRIVER_DEFERRED_RETRY_COUNT", "7") }; - unsafe { std::env::set_var("REDBEAR_DRIVER_DEFERRED_RETRY_INTERVAL_MS", "250") }; + // Reset to defaults before applying the override so the + // reset at the end is symmetric. + set_deferred_retry_config(30, 500); + set_deferred_retry_config(7, 250); let json = format_metrics_json(&metrics); assert!( json.contains(r#""deferred_retry_config":{"count":7,"interval_ms":250}"#), "expected overridden deferred_retry_config in {}", json ); - unsafe { std::env::remove_var("REDBEAR_DRIVER_DEFERRED_RETRY_COUNT") }; - unsafe { std::env::remove_var("REDBEAR_DRIVER_DEFERRED_RETRY_INTERVAL_MS") }; + // Reset to defaults so subsequent tests start from a + // known state. + set_deferred_retry_config(30, 500); }); } @@ -820,12 +847,22 @@ mod tests { record("claim-spawn", 99_000); let json = format_json(); - assert!(json.starts_with(r#"{"version":1,"buckets":{"#)); + assert!(json.starts_with(r#"{"version":2,"buckets":{"#)); assert!(json.contains(r#""claim-spawn""#)); assert!(json.contains(r#""firmware-ready""#)); assert!(json.contains(r#""governor-switch""#)); assert!(json.contains(r#""aer-event""#)); - assert!(json.ends_with("}}")); + // N6: ends with the deferred_retry_config suffix + // followed by the version-object close. + assert!( + json.ends_with(&format!( + "\"deferred_retry_config\":{{\"count\":{},\"interval_ms\":{}}}}}", + DEFERRED_RETRY_COUNT.load(Ordering::SeqCst), + DEFERRED_RETRY_INTERVAL_MS.load(Ordering::SeqCst) + )), + "json did not end with the expected deferred_retry_config suffix: {}", + json + ); } #[test]