redox-driver-core: eliminate concurrent path double bus enumeration
ConcurrentDeviceManager::from_manager called bus.enumerate_devices() a second time inside its constructor — the same buses DeviceManager:: enumerate() had just enumerated to build the remaining list. Renamed to from_devices(mgr, devices) which takes the already-enumerated Vec<DeviceInfo> directly, eliminating the redundant I/O and removing a potential consistency window between the two passes.
This commit is contained in:
@@ -70,62 +70,49 @@ pub struct ConcurrentDeviceManager {
|
||||
}
|
||||
|
||||
impl ConcurrentDeviceManager {
|
||||
/// Build a concurrent wrapper. Eagerly enumerates devices and collects
|
||||
/// every matching driver per device (static match table or dynids),
|
||||
/// in registration order. Workers probe the real driver objects
|
||||
/// through shared refs, so quirks, blacklist, and `exclusive_with`
|
||||
/// gates inside `Driver::probe` apply identically to the serial path.
|
||||
pub fn from_manager(mgr: &DeviceManager) -> Self {
|
||||
/// Build a concurrent wrapper from already-enumerated devices.
|
||||
/// Collects every matching driver per device (static match table or
|
||||
/// dynids), in registration order. Workers probe the real driver
|
||||
/// objects through shared refs, so quirks, blacklist, and
|
||||
/// `exclusive_with` gates inside `Driver::probe` apply identically to
|
||||
/// the serial path.
|
||||
pub fn from_devices(
|
||||
mgr: &DeviceManager,
|
||||
devices: Vec<DeviceInfo>,
|
||||
) -> Self {
|
||||
let drivers = mgr.drivers_snapshot();
|
||||
let mut jobs: Vec<(DeviceId, DeviceInfo, Vec<Arc<dyn Driver>>)> = Vec::new();
|
||||
let mut pre_events: Vec<ProbeEvent> = Vec::new();
|
||||
for bus in mgr.buses_iter() {
|
||||
let bus_name = bus.name().to_string();
|
||||
match bus.enumerate_devices() {
|
||||
Ok(devices) => {
|
||||
let count = devices.len();
|
||||
for info in devices {
|
||||
if let Some(bound_name) = mgr
|
||||
.bound_devices_snapshot()
|
||||
.get(&info.id)
|
||||
.map(|b| b.driver_name.clone())
|
||||
{
|
||||
pre_events.push(ProbeEvent::AlreadyBound {
|
||||
device: info.id.clone(),
|
||||
driver_name: bound_name,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
let candidates: Vec<Arc<dyn Driver>> = drivers
|
||||
|
||||
for info in devices {
|
||||
if let Some(bound_name) = mgr
|
||||
.bound_devices_snapshot()
|
||||
.get(&info.id)
|
||||
.map(|b| b.driver_name.clone())
|
||||
{
|
||||
pre_events.push(ProbeEvent::AlreadyBound {
|
||||
device: info.id.clone(),
|
||||
driver_name: bound_name,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
let candidates: Vec<Arc<dyn Driver>> = drivers
|
||||
.iter()
|
||||
.filter(|driver| {
|
||||
driver.match_table().iter().any(|m| m.matches(&info))
|
||||
|| mgr
|
||||
.list_dynids(driver.name())
|
||||
.iter()
|
||||
.filter(|driver| {
|
||||
driver.match_table().iter().any(|m| m.matches(&info))
|
||||
|| mgr
|
||||
.list_dynids(driver.name())
|
||||
.iter()
|
||||
.any(|m| m.matches(&info))
|
||||
})
|
||||
.map(Arc::clone)
|
||||
.collect();
|
||||
if candidates.is_empty() {
|
||||
pre_events.push(ProbeEvent::NoDriverFound {
|
||||
device: info.id,
|
||||
});
|
||||
} else {
|
||||
jobs.push((info.id.clone(), info, candidates));
|
||||
}
|
||||
}
|
||||
pre_events.push(ProbeEvent::BusEnumerated {
|
||||
bus: bus_name,
|
||||
device_count: count,
|
||||
});
|
||||
}
|
||||
Err(error) => {
|
||||
pre_events.push(ProbeEvent::BusEnumerationFailed {
|
||||
bus: bus_name,
|
||||
error,
|
||||
});
|
||||
}
|
||||
.any(|m| m.matches(&info))
|
||||
})
|
||||
.map(Arc::clone)
|
||||
.collect();
|
||||
if candidates.is_empty() {
|
||||
pre_events.push(ProbeEvent::NoDriverFound {
|
||||
device: info.id,
|
||||
});
|
||||
} else {
|
||||
jobs.push((info.id.clone(), info, candidates));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -277,7 +264,7 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Test driver with an empty match table: `from_manager` finds no
|
||||
/// Test driver with an empty match table: `from_devices` finds no
|
||||
/// candidates for any device, so jobs stay empty regardless of what
|
||||
/// `probe` would return.
|
||||
struct EmptyDriver {
|
||||
@@ -337,7 +324,7 @@ mod tests {
|
||||
#[test]
|
||||
fn concurrent_enumerate_preserves_job_count() {
|
||||
let base = build_manager_with_devices(4);
|
||||
let concurrent = ConcurrentDeviceManager::from_manager(&base);
|
||||
let concurrent = ConcurrentDeviceManager::from_devices(&base, base.buses_iter().next().unwrap().enumerate_devices().unwrap_or_default());
|
||||
assert_eq!(concurrent.pending_jobs(), 0);
|
||||
}
|
||||
|
||||
@@ -350,7 +337,7 @@ mod tests {
|
||||
});
|
||||
mgr.register_bus(Box::new(CountingBus { devices: Vec::new() }));
|
||||
mgr.register_driver(Box::new(EmptyDriver { name: "noop" }));
|
||||
let c = ConcurrentDeviceManager::from_manager(&mgr);
|
||||
let c = ConcurrentDeviceManager::from_devices(&mgr, mgr.buses_iter().next().unwrap().enumerate_devices().unwrap_or_default());
|
||||
assert_eq!(c.pending_jobs(), 0);
|
||||
let events = c.enumerate(4);
|
||||
assert!(events.iter().all(|e| matches!(
|
||||
@@ -385,7 +372,7 @@ mod tests {
|
||||
});
|
||||
mgr.register_bus(Box::new(CountingBus { devices }));
|
||||
mgr.register_driver(Box::new(EmptyDriver { name: "noop" }));
|
||||
let c = ConcurrentDeviceManager::from_manager(&mgr);
|
||||
let c = ConcurrentDeviceManager::from_devices(&mgr, mgr.buses_iter().next().unwrap().enumerate_devices().unwrap_or_default());
|
||||
assert_eq!(c.pending_jobs(), 0);
|
||||
}
|
||||
|
||||
@@ -463,7 +450,7 @@ mod tests {
|
||||
probe_count: Arc::clone(&probe_count),
|
||||
}));
|
||||
|
||||
let concurrent = ConcurrentDeviceManager::from_manager(&mgr);
|
||||
let concurrent = ConcurrentDeviceManager::from_devices(&mgr, mgr.buses_iter().next().unwrap().enumerate_devices().unwrap_or_default());
|
||||
assert_eq!(concurrent.pending_jobs(), 4);
|
||||
let events = concurrent.enumerate(4);
|
||||
|
||||
@@ -511,7 +498,7 @@ mod tests {
|
||||
probe_count: Arc::clone(&binding_count),
|
||||
}));
|
||||
|
||||
let concurrent = ConcurrentDeviceManager::from_manager(&mgr);
|
||||
let concurrent = ConcurrentDeviceManager::from_devices(&mgr, mgr.buses_iter().next().unwrap().enumerate_devices().unwrap_or_default());
|
||||
assert_eq!(concurrent.pending_jobs(), 1);
|
||||
let events = concurrent.enumerate(2);
|
||||
|
||||
@@ -547,7 +534,7 @@ mod tests {
|
||||
probe_count: Arc::new(AtomicUsize::new(0)),
|
||||
}));
|
||||
|
||||
let concurrent = ConcurrentDeviceManager::from_manager(&mgr);
|
||||
let concurrent = ConcurrentDeviceManager::from_devices(&mgr, mgr.buses_iter().next().unwrap().enumerate_devices().unwrap_or_default());
|
||||
assert_eq!(concurrent.pending_jobs(), 1);
|
||||
let events = concurrent.enumerate(2);
|
||||
|
||||
|
||||
@@ -408,8 +408,10 @@ impl DeviceManager {
|
||||
}
|
||||
|
||||
if remaining.len() >= workload_threshold && self.config.max_concurrent_probes > 1 {
|
||||
// Large workload: use the concurrent worker pool.
|
||||
let concurrent = crate::concurrent::ConcurrentDeviceManager::from_manager(self);
|
||||
let concurrent = crate::concurrent::ConcurrentDeviceManager::from_devices(
|
||||
self,
|
||||
remaining,
|
||||
);
|
||||
let concurrent_events = concurrent.enumerate(self.config.max_concurrent_probes);
|
||||
events.extend(concurrent_events);
|
||||
// Sync state back from the concurrent path to the manager.
|
||||
|
||||
Reference in New Issue
Block a user