From c875201c3f3876bb946b87b9ffa2211ca7ca8cf1 Mon Sep 17 00:00:00 2001 From: LI Qing Date: Fri, 26 Apr 2024 11:53:33 +0800 Subject: [PATCH] Fix deadlocks that may arise after converting virtio-blk to async The IRQ part of the driver must NOT share a SpinLock with the normal part of the driver unless the SpinLock is acquired with IRQ disabled in the task context. --- .../src/fs/ext2/indirect_block_cache.rs | 5 +---- kernel/comps/virtio/src/device/block/device.rs | 17 ++++++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/kernel/aster-nix/src/fs/ext2/indirect_block_cache.rs b/kernel/aster-nix/src/fs/ext2/indirect_block_cache.rs index b13e2a7d..dc6b3261 100644 --- a/kernel/aster-nix/src/fs/ext2/indirect_block_cache.rs +++ b/kernel/aster-nix/src/fs/ext2/indirect_block_cache.rs @@ -92,10 +92,7 @@ impl IndirectBlockCache { return Ok(()); } // TODO: How to determine the number of evictions each time? - // - // FIXME: When we set it to `Self::MAX_SIZE / 2` here, - // running the `/regression/ext2.sh` test may cause a deadlock issue. - let evict_num = 1; + let evict_num = Self::MAX_SIZE / 2; self.evict(evict_num) } diff --git a/kernel/comps/virtio/src/device/block/device.rs b/kernel/comps/virtio/src/device/block/device.rs index e37e6872..892ea1af 100644 --- a/kernel/comps/virtio/src/device/block/device.rs +++ b/kernel/comps/virtio/src/device/block/device.rs @@ -152,10 +152,13 @@ impl DeviceInner { /// Handles the irq issued from the device fn handle_irq(&self) { info!("Virtio block device handle irq"); + // When we enter the IRQs handling function, + // IRQs have already been disabled, + // so there is no need to call `lock_irq_disabled`. loop { // Pops the complete request let complete_request = { - let mut queue = self.queue.lock_irq_disabled(); + let mut queue = self.queue.lock(); let Ok((token, _)) = queue.pop_used() else { return; }; @@ -198,7 +201,7 @@ impl DeviceInner { // TODO: Most logic is the same as read and write, there should be a refactor. // TODO: Should return an Err instead of panic if the device fails. fn request_device_id(&self) -> String { - let id = self.id_allocator.lock().alloc().unwrap(); + let id = self.id_allocator.lock_irq_disabled().alloc().unwrap(); let req_slice = { let req_slice = DmaStreamSlice::new(&self.block_requests, id * REQ_SIZE, REQ_SIZE); let req = BlockReq { @@ -241,7 +244,7 @@ impl DeviceInner { queue.pop_used_with_token(token).expect("pop used failed"); resp_slice.sync().unwrap(); - self.id_allocator.lock().free(id); + self.id_allocator.lock_irq_disabled().free(id); let resp: BlockResp = resp_slice.read_val(0).unwrap(); match RespStatus::try_from(resp.status).unwrap() { RespStatus::Ok => {} @@ -266,7 +269,7 @@ impl DeviceInner { fn read(&self, bio_request: BioRequest) { let dma_streams = Self::dma_stream_map(&bio_request); - let id = self.id_allocator.lock().alloc().unwrap(); + let id = self.id_allocator.lock_irq_disabled().alloc().unwrap(); let req_slice = { let req_slice = DmaStreamSlice::new(&self.block_requests, id * REQ_SIZE, REQ_SIZE); let req = BlockReq { @@ -317,7 +320,7 @@ impl DeviceInner { // Records the submitted request let submitted_request = SubmittedRequest::new(id as u16, bio_request, dma_streams); self.submitted_requests - .lock() + .lock_irq_disabled() .insert(token, submitted_request); return; } @@ -327,7 +330,7 @@ impl DeviceInner { fn write(&self, bio_request: BioRequest) { let dma_streams = Self::dma_stream_map(&bio_request); - let id = self.id_allocator.lock().alloc().unwrap(); + let id = self.id_allocator.lock_irq_disabled().alloc().unwrap(); let req_slice = { let req_slice = DmaStreamSlice::new(&self.block_requests, id * REQ_SIZE, REQ_SIZE); let req = BlockReq { @@ -377,7 +380,7 @@ impl DeviceInner { // Records the submitted request let submitted_request = SubmittedRequest::new(id as u16, bio_request, dma_streams); self.submitted_requests - .lock() + .lock_irq_disabled() .insert(token, submitted_request); return; }