diff --git a/lib/propolis/src/hw/nvme/queue.rs b/lib/propolis/src/hw/nvme/queue.rs index ebe5b80c3..b977f3427 100644 --- a/lib/propolis/src/hw/nvme/queue.rs +++ b/lib/propolis/src/hw/nvme/queue.rs @@ -113,7 +113,13 @@ struct QueueState { /// a `SubQueueState` for a Submission Queue. inner: Mutex>, - pub acc_mem: MemAccessor, + /// This queue's memory accessor node. + /// + /// Be careful about lock ordering when using this accessor; access_borrow() + /// holds this node's lock. If a user of this queue state requires both + /// `access_borrow()` and `QueueInner`, the protocol is to lock queue + /// state first and this accessor second. + acc_mem: MemAccessor, } impl QueueState { fn new(size: u32, acc_mem: MemAccessor, inner: QS) -> Self { @@ -649,12 +655,15 @@ impl SubQueue { pub fn pop( self: &Arc, ) -> Option<(GuestData, Permit, u16)> { + // Lock the SubQueueState early to conform to lock ordering requirement; + // see docs on QueueState::acc_mem. + let mut state = self.state.lock(); + let Some(mem) = self.state.acc_mem.access_borrow() else { return None }; let mem = mem.view(); // Attempt to reserve an entry on the Completion Queue let permit = self.cq.reserve_entry(&self, &mem)?; - let mut state = self.state.lock(); // Check for last-minute updates to the tail via any configured doorbell // page, prior to attempting the pop itself.