usbhubd: judge debounce stability on connection state, not change bit
The debounce's inner clear of C_PORT_CONNECTION could hang or be ignored by the hub, and our control transfers have no timeout — so a literal port of Linux's clear-inside-the-loop design deadlocked hub enumeration: the debounce never converged and the reset/attach path was never reached (observed on QEMU as usbhubd logging the port's first status with connection_changed=true and then going silent forever). Judge the 100ms stable window on the connection STATE alone (the same stability property hub_port_debounce_be_connected protects): a stuck or hung change bit can no longer deadlock enumeration — it only re-triggers a poll, and is cleared once, best-effort, after a successful attach (matching Linux port_event's C_PORT_CONNECTION clear). New regression test debounce_stuck_change_bit_still_converges locks the deadlock scenario; 15/15 tests pass.
This commit is contained in:
Red Bear OS
@@ -543,12 +543,14 @@ fn main() {
|
||||
}
|
||||
|
||||
// Linux 7.1 hub_port_debounce_be_connected(): on a
|
||||
// connection-change event, require the connection bit to stay
|
||||
// stable for 100ms (25ms steps, 2s budget) before enumerating.
|
||||
// connection-change event, require the connection state to
|
||||
// stay stable for 100ms (25ms steps, 2s budget) before
|
||||
// enumerating. The change bit is cleared after attach below,
|
||||
// not inside the debounce — a hung clear must not deadlock
|
||||
// enumeration.
|
||||
if snap.connection_changed && !state.attached {
|
||||
match port_ops::debounce_until_connected(
|
||||
| | fetch_status(&handle, port, usb_3),
|
||||
| | clear_feature(&handle, port, usb::HubPortFeature::CPortConnection),
|
||||
|ms| thread::sleep(time::Duration::from_millis(ms)),
|
||||
) {
|
||||
Ok(s) => {
|
||||
@@ -595,6 +597,15 @@ fn main() {
|
||||
|
||||
let was_attached = state.attached;
|
||||
state.ensure_attached(true);
|
||||
if !was_attached && state.attached {
|
||||
// The connection-change bit was ignored during the debounce
|
||||
// (stuck/hung clears must not deadlock enumeration); clear it
|
||||
// now that the device is attached, best-effort. Linux clears
|
||||
// C_PORT_CONNECTION in port_event on the same path.
|
||||
if snap.connection_changed {
|
||||
let _ = clear_feature(&handle, port, usb::HubPortFeature::CPortConnection);
|
||||
}
|
||||
}
|
||||
// New SuperSpeed child: deliver the accumulated hub delay so the
|
||||
// device can compensate isochronous scheduling (Linux
|
||||
// usb_set_isoch_delay after enumeration, hub.c:5126-5131).
|
||||
|
||||
@@ -115,17 +115,21 @@ impl<E: fmt::Debug> fmt::Display for WaitError<E> {
|
||||
|
||||
impl<E: fmt::Debug> std::error::Error for WaitError<E> {}
|
||||
|
||||
/// Linux 7.1 `hub_port_debounce_be_connected()` (hub.c:4696-4737).
|
||||
/// Linux 7.1 `hub_port_debounce_be_connected()` (hub.c:4696-4737), adapted
|
||||
/// for an environment with no control-transfer timeouts.
|
||||
///
|
||||
/// Poll the port until the connection bit stays continuously set for
|
||||
/// Poll the port until the CONNECTION STATE stays continuously set for
|
||||
/// `HUB_DEBOUNCE_STABLE_MS`, sampling every `HUB_DEBOUNCE_STEP_MS` with a
|
||||
/// total budget of `HUB_DEBOUNCE_TIMEOUT_MS`. Any connection-change event
|
||||
/// or state flip restarts the stable window. The change bit is cleared
|
||||
/// during the loop so subsequent reads reflect hardware, not the
|
||||
/// original event. Returns the final stable status.
|
||||
/// total budget of `HUB_DEBOUNCE_TIMEOUT_MS`. Stability is judged on the
|
||||
/// connection STATE alone, not on the change bit: Linux clears
|
||||
/// C_PORT_CONNECTION inside its loop so later reads are clean, but a hub
|
||||
/// that hangs or ignores the clear would deadlock a literal port of that
|
||||
/// design (our control transfers have no timeout). A stuck change bit is
|
||||
/// harmless — it only re-triggers a poll — so it is ignored here and
|
||||
/// cleared once after a successful attach instead. Returns the final
|
||||
/// stable status.
|
||||
pub fn debounce_until_connected<E>(
|
||||
mut fetch: impl FnMut() -> Result<PortStatusSnapshot, E>,
|
||||
mut clear_connection_change: impl FnMut() -> Result<(), E>,
|
||||
mut sleep: impl FnMut(u64),
|
||||
) -> Result<PortStatusSnapshot, WaitError<E>> {
|
||||
let mut stable_ms = 0u64;
|
||||
@@ -137,7 +141,7 @@ pub fn debounce_until_connected<E>(
|
||||
loop {
|
||||
let cur = fetch().map_err(WaitError::Io)?;
|
||||
|
||||
if !cur.connection_changed && connection == Some(cur.connected) {
|
||||
if connection == Some(cur.connected) {
|
||||
stable_ms += HUB_DEBOUNCE_STEP_MS;
|
||||
if stable_ms >= HUB_DEBOUNCE_STABLE_MS {
|
||||
return if cur.connected {
|
||||
@@ -151,10 +155,6 @@ pub fn debounce_until_connected<E>(
|
||||
connection = Some(cur.connected);
|
||||
}
|
||||
|
||||
if cur.connection_changed {
|
||||
clear_connection_change().map_err(WaitError::Io)?;
|
||||
}
|
||||
|
||||
if total_ms >= HUB_DEBOUNCE_TIMEOUT_MS {
|
||||
return Err(WaitError::Timeout);
|
||||
}
|
||||
@@ -303,14 +303,12 @@ mod tests {
|
||||
let rig = RefCell::new(Rig::new([connected(), connected(), connected(), connected()]));
|
||||
let out = debounce_until_connected(
|
||||
|| rig.borrow_mut().fetch(),
|
||||
|| rig.borrow_mut().clear(),
|
||||
|ms| rig.borrow_mut().sleep(ms),
|
||||
);
|
||||
let rig = rig.into_inner();
|
||||
assert_eq!(out, Ok(connected()));
|
||||
// First read seeds the window; four stable steps of 25 ms reach 100 ms.
|
||||
assert_eq!(rig.sleeps, vec![25, 25, 25, 25]);
|
||||
assert_eq!(rig.clears, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -325,7 +323,6 @@ mod tests {
|
||||
]));
|
||||
let out = debounce_until_connected(
|
||||
|| rig.borrow_mut().fetch(),
|
||||
|| rig.borrow_mut().clear(),
|
||||
|ms| rig.borrow_mut().sleep(ms),
|
||||
);
|
||||
let rig = rig.into_inner();
|
||||
@@ -339,29 +336,44 @@ mod tests {
|
||||
let rig = RefCell::new(Rig::new([snap(), snap(), snap(), snap()]));
|
||||
let out = debounce_until_connected(
|
||||
|| rig.borrow_mut().fetch(),
|
||||
|| rig.borrow_mut().clear(),
|
||||
|ms| rig.borrow_mut().sleep(ms),
|
||||
);
|
||||
assert_eq!(out, Err(WaitError::Disconnected));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debounce_change_bit_cleared_and_timeout() {
|
||||
let flapping = PortStatusSnapshot {
|
||||
fn debounce_stuck_change_bit_still_converges() {
|
||||
// The hub keeps C_PORT_CONNECTION set (clear hung or ignored).
|
||||
// Stability is judged on the connection STATE, so a stuck change
|
||||
// bit must not deadlock the debounce — this was the QEMU
|
||||
// enumeration hang caught by test-usb-hub-qemu.sh.
|
||||
let stuck = PortStatusSnapshot {
|
||||
connection_changed: true,
|
||||
..connected()
|
||||
};
|
||||
let rig = RefCell::new(Rig::new([stuck, stuck, stuck, stuck, stuck]));
|
||||
let out = debounce_until_connected(
|
||||
|| rig.borrow_mut().fetch(),
|
||||
|ms| rig.borrow_mut().sleep(ms),
|
||||
);
|
||||
let rig = rig.into_inner();
|
||||
assert_eq!(out, Ok(stuck));
|
||||
assert_eq!(rig.sleeps, vec![25, 25, 25, 25]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debounce_timeout_when_never_stable() {
|
||||
// Connection flaps between connected and disconnected every poll —
|
||||
// the window never fills, so the total budget expires.
|
||||
let rig = RefCell::new(Rig::new(
|
||||
(0..200).map(|i| if i % 2 == 0 { flapping } else { connected() }),
|
||||
(0..200).map(|i| if i % 2 == 0 { connected() } else { snap() }),
|
||||
));
|
||||
let out = debounce_until_connected(
|
||||
|| rig.borrow_mut().fetch(),
|
||||
|| rig.borrow_mut().clear(),
|
||||
|ms| rig.borrow_mut().sleep(ms),
|
||||
);
|
||||
let rig = rig.into_inner();
|
||||
assert_eq!(out, Err(WaitError::Timeout));
|
||||
assert!(rig.clears > 0);
|
||||
assert_eq!(rig.sleeps.iter().sum::<u64>(), HUB_DEBOUNCE_TIMEOUT_MS);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user