From 62f6128ba1006c1f606c5973a21b30364326f864 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Tue, 19 Mar 2024 12:55:13 +0200 Subject: [PATCH] Kernel: Cleanup NVMe Queue command submission There is techically a race condition on thread sleep and checking done mask. This patch allows read to success even if this race condition is hit, although the full timeout has to be waited. This can be fixed in future with some sort of wait queues that can properly handle this race condition. --- kernel/kernel/Storage/NVMe/Queue.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/kernel/kernel/Storage/NVMe/Queue.cpp b/kernel/kernel/Storage/NVMe/Queue.cpp index 628564e4..68b72b7a 100644 --- a/kernel/kernel/Storage/NVMe/Queue.cpp +++ b/kernel/kernel/Storage/NVMe/Queue.cpp @@ -67,24 +67,20 @@ namespace Kernel } const uint64_t start_time = SystemTimer::get().ms_since_boot(); - while (SystemTimer::get().ms_since_boot() < start_time + s_nvme_command_poll_timeout_ms) - { - if (m_done_mask & cid_mask) - { - uint16_t status = m_status_codes[cid]; - m_used_mask &= ~cid_mask; - return status; - } - } + while (!(m_done_mask & cid_mask) && SystemTimer::get().ms_since_boot() < start_time + s_nvme_command_poll_timeout_ms) + continue; - while (SystemTimer::get().ms_since_boot() < start_time + s_nvme_command_timeout_ms) + // FIXME: Here is a possible race condition if done mask is set before + // scheduler has put the current thread blocking. + // EINTR should also be handled here. + while (!(m_done_mask & cid_mask) && SystemTimer::get().ms_since_boot() < start_time + s_nvme_command_timeout_ms) + Scheduler::get().block_current_thread(&m_semaphore, start_time + s_nvme_command_timeout_ms); + + if (m_done_mask & cid_mask) { - if (m_done_mask & cid_mask) - { - uint16_t status = m_status_codes[cid]; - m_used_mask &= ~cid_mask; - return status; - } + uint16_t status = m_status_codes[cid]; + m_used_mask &= ~cid_mask; + return status; } m_used_mask &= ~cid_mask;