From 2d7f7880c9e04fd7b8d70756ed5d66823ad542d0 Mon Sep 17 00:00:00 2001 From: vasilito Date: Mon, 27 Jul 2026 12:41:58 +0900 Subject: [PATCH] =?UTF-8?q?redox-driver-core:=20F5=20=E2=80=94=20regressio?= =?UTF-8?q?n=20tests=20for=20single=20enumeration=20per=20probe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two AtomicUsize-counting bus regression tests that verify each registered bus's enumerate_devices() is called exactly once per DeviceManager::enumerate() call. The concurrent path (>=4 devices, max_concurrent_probes > 1) used to call manager.enumerate() recursively (G8 in the v3.0 assessment); the v2.2 sweep moved the concurrent path to ConcurrentDeviceManager:: from_devices() which takes pre-enumerated devices, but no test was added to lock in the contract. Tests: - enumerate_calls_each_bus_enumerate_devices_exactly_once: multi-bus setup with 4 PCI + 2 platform devices; PCI crosses the concurrent threshold; both buses must be enumerated exactly once. - enumerate_calls_each_bus_exactly_once_per_call_repeated: repeated enumerate() calls must each enumerate every bus exactly once (matters for retry_deferred integration). Adds CountingBus struct (test-only) with an Arc call counter; existing MockBus / MockDriver helpers retained. 35 redox-driver-core tests pass (was 33). --- .../redox-driver-core/source/src/manager.rs | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/local/recipes/drivers/redox-driver-core/source/src/manager.rs b/local/recipes/drivers/redox-driver-core/source/src/manager.rs index 2e8725d75f..90435710fd 100644 --- a/local/recipes/drivers/redox-driver-core/source/src/manager.rs +++ b/local/recipes/drivers/redox-driver-core/source/src/manager.rs @@ -652,6 +652,28 @@ mod tests { } } + /// Counting bus variant that records how many times + /// `enumerate_devices` was called. Used by the single-enumeration + /// regression test to verify the concurrent path does NOT + /// re-walk registered buses. + struct CountingBus { + name: &'static str, + devices: Vec, + call_count: std::sync::Arc, + } + + impl Bus for CountingBus { + fn name(&self) -> &str { + self.name + } + + fn enumerate_devices(&self) -> Result, BusError> { + self.call_count + .fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(self.devices.clone()) + } + } + struct MockDriver { name: &'static str, description: &'static str, @@ -768,4 +790,160 @@ mod tests { if bus == "pci" && *device_count == 1 ))); } + + /// Regression test for F5: the concurrent probe path must NOT + /// re-walk registered buses. Each bus's `enumerate_devices` should + /// fire exactly once per `DeviceManager::enumerate()` call, even + /// when the workload crosses the threshold into concurrent mode. + /// + /// Historically this was a bug (G8 in the v3.0 assessment): the + /// concurrent path used to call `manager.enumerate()` recursively, + /// doubling bus-enumeration work on multi-bus systems. + #[test] + fn enumerate_calls_each_bus_enumerate_devices_exactly_once() { + let mut manager = DeviceManager::new(config()); + // The default config() has max_concurrent_probes=4 and the + // workload_threshold is 4 devices. With 4 PCI devices and 2 + // platform devices, the PCI bus crosses the concurrent threshold. + let count_a = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let count_b = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + manager.register_bus(Box::new(CountingBus { + name: "pci", + devices: (0..4) + .map(|i| { + use crate::device::{DeviceId, DeviceInfo}; + DeviceInfo { + id: DeviceId { + bus: "pci".to_string(), + path: format!("0000:00:0{i}.0"), + }, + vendor: Some(0x1234), + device: Some(0x5678), + class: Some(0x02), + subclass: Some(0x00), + prog_if: None, + revision: None, + subsystem_vendor: None, + subsystem_device: None, + raw_path: format!("/scheme/pci/0000:00:0{i}.0"), + description: None, + } + }) + .collect(), + call_count: std::sync::Arc::clone(&count_a), + })); + manager.register_bus(Box::new(CountingBus { + name: "platform", + devices: (0..2) + .map(|i| { + use crate::device::{DeviceId, DeviceInfo}; + DeviceInfo { + id: DeviceId { + bus: "platform".to_string(), + path: format!("plat{i}"), + }, + vendor: None, + device: None, + class: None, + subclass: None, + prog_if: None, + revision: None, + subsystem_vendor: None, + subsystem_device: None, + raw_path: format!("/scheme/platform/plat{i}"), + description: None, + } + }) + .collect(), + call_count: std::sync::Arc::clone(&count_b), + })); + // Register a low-priority driver that matches all of the above + // devices so every device enters the probe path (and forces the + // concurrent threshold to be met for the PCI bus). + manager.register_driver(Box::new(MockDriver { + name: "catchall", + description: "catch-all", + priority: 1, + matches: vec![DriverMatch { + vendor: None, + device: None, + class: None, + subclass: None, + prog_if: None, + subsystem_vendor: None, + subsystem_device: None, + }], + })); + + let _ = manager.enumerate(); + + assert_eq!( + count_a.load(std::sync::atomic::Ordering::SeqCst), + 1, + "pci bus must be enumerated exactly once per manager.enumerate() call" + ); + assert_eq!( + count_b.load(std::sync::atomic::Ordering::SeqCst), + 1, + "platform bus must be enumerated exactly once per manager.enumerate() call" + ); + } + + /// Second call to `enumerate()` should also enumerate each bus + /// exactly once — verifies the regression test does not flake + /// when called multiple times (e.g. during retry_deferred). + #[test] + fn enumerate_calls_each_bus_exactly_once_per_call_repeated() { + let mut manager = DeviceManager::new(config()); + let count = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + manager.register_bus(Box::new(CountingBus { + name: "pci", + devices: (0..4) + .map(|i| { + use crate::device::{DeviceId, DeviceInfo}; + DeviceInfo { + id: DeviceId { + bus: "pci".to_string(), + path: format!("0000:00:0{i}.0"), + }, + vendor: Some(0x1234), + device: Some(0x5678), + class: Some(0x02), + subclass: Some(0x00), + prog_if: None, + revision: None, + subsystem_vendor: None, + subsystem_device: None, + raw_path: format!("/scheme/pci/0000:00:0{i}.0"), + description: None, + } + }) + .collect(), + call_count: std::sync::Arc::clone(&count), + })); + manager.register_driver(Box::new(MockDriver { + name: "catchall", + description: "catch-all", + priority: 1, + matches: vec![DriverMatch { + vendor: None, + device: None, + class: None, + subclass: None, + prog_if: None, + subsystem_vendor: None, + subsystem_device: None, + }], + })); + + for _ in 0..3 { + let _ = manager.enumerate(); + } + + assert_eq!( + count.load(std::sync::atomic::Ordering::SeqCst), + 3, + "each call to manager.enumerate() should enumerate each bus exactly once" + ); + } }