From 71649ffe097acbc2daea2dbdbee53601ba32520f Mon Sep 17 00:00:00 2001 From: Bananymous Date: Mon, 20 Apr 2026 12:45:41 +0300 Subject: [PATCH] Kernel Rework ThreadBlocker I don't know why I though the block chain had to be stored fully in the ThreadBlocker, that did not even fix the problem I was trying to fix when I last rewrote it. Roll back to doubly linked list of block chain and now just check that the node is contained within the ThreadBlocker before removing and after acquiring the ThreadBlocker's lock. Also there is no need to have a separate lock the node's blocker field. We can just perform an atomic reads and writes to it. We can still get a blocker that the node is no longer part of, but this can be resolved with a simple check. This patch reduces ThreadBlocker's size from over 200 bytes to just 12 bytes +4 bytes padding --- kernel/include/kernel/SchedulerQueueNode.h | 5 +- kernel/include/kernel/ThreadBlocker.h | 7 +-- kernel/kernel/Scheduler.cpp | 23 +++----- kernel/kernel/ThreadBlocker.cpp | 68 ++++++++++++---------- 4 files changed, 49 insertions(+), 54 deletions(-) diff --git a/kernel/include/kernel/SchedulerQueueNode.h b/kernel/include/kernel/SchedulerQueueNode.h index e37737fc..0b0f4e03 100644 --- a/kernel/include/kernel/SchedulerQueueNode.h +++ b/kernel/include/kernel/SchedulerQueueNode.h @@ -22,8 +22,9 @@ namespace Kernel uint64_t wake_time_ns { static_cast(-1) }; - SpinLock blocker_lock; - ThreadBlocker* blocker { nullptr }; + BAN::Atomic blocker { nullptr }; + SchedulerQueueNode* block_chain_prev { nullptr }; + SchedulerQueueNode* block_chain_next { nullptr }; ProcessorID processor_id { PROCESSOR_NONE }; bool blocked { false }; diff --git a/kernel/include/kernel/ThreadBlocker.h b/kernel/include/kernel/ThreadBlocker.h index 6a9f18f9..e9fd91b9 100644 --- a/kernel/include/kernel/ThreadBlocker.h +++ b/kernel/include/kernel/ThreadBlocker.h @@ -15,7 +15,6 @@ namespace Kernel void block_with_wake_time_ns(uint64_t wake_time_ns, BaseMutex*); void unblock(); - void block_with_timeout_ms(uint64_t timeout_ms, BaseMutex* mutex) { ASSERT(!BAN::Math::will_multiplication_overflow(timeout_ms, 1'000'000)); @@ -29,14 +28,12 @@ namespace Kernel private: void add_thread_to_block_queue(SchedulerQueue::Node*); - void remove_blocked_thread(SchedulerQueue::Node*); + void remove_thread_from_block_queue(SchedulerQueue::Node*); private: + SchedulerQueue::Node* m_block_chain { nullptr }; SpinLock m_lock; - SchedulerQueue::Node* m_block_chain[32] {}; - size_t m_block_chain_length { 0 }; - friend class Scheduler; }; diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index e0598cb2..590bc97d 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -307,15 +307,14 @@ namespace Kernel void Scheduler::wake_up_sleeping_threads() { + ASSERT(Processor::get_interrupt_state() == InterruptState::Disabled); + const uint64_t current_ns = SystemTimer::get().ns_since_boot(); while (!m_block_queue.empty() && current_ns >= m_block_queue.front()->wake_time_ns) { auto* node = m_block_queue.pop_front(); - { - SpinLockGuard _(node->blocker_lock); - if (node->blocker) - node->blocker->remove_blocked_thread(node); - } + if (auto* blocker = node->blocker.load()) + blocker->remove_thread_from_block_queue(node); node->blocked = false; update_most_loaded_node_queue(node, &m_run_queue); m_run_queue.add_thread_to_back(node); @@ -368,11 +367,8 @@ namespace Kernel return; if (node != m_current) m_block_queue.remove_node(node); - { - SpinLockGuard _(node->blocker_lock); - if (node->blocker) - node->blocker->remove_blocked_thread(node); - } + if (auto* blocker = node->blocker.load()) + blocker->remove_thread_from_block_queue(node); node->blocked = false; if (node != m_current) m_run_queue.add_thread_to_back(node); @@ -665,11 +661,8 @@ namespace Kernel m_current->blocked = true; m_current->wake_time_ns = wake_time_ns; - { - SpinLockGuard _(m_current->blocker_lock); - if (blocker) - blocker->add_thread_to_block_queue(m_current); - } + if (blocker) + blocker->add_thread_to_block_queue(m_current); update_most_loaded_node_queue(m_current, &m_block_queue); diff --git a/kernel/kernel/ThreadBlocker.cpp b/kernel/kernel/ThreadBlocker.cpp index 3faed184..49969339 100644 --- a/kernel/kernel/ThreadBlocker.cpp +++ b/kernel/kernel/ThreadBlocker.cpp @@ -23,60 +23,64 @@ namespace Kernel void ThreadBlocker::unblock() { - decltype(m_block_chain) temp_block_chain; - size_t temp_block_chain_length { 0 }; + SpinLockGuard _(m_lock); + for (auto* node = m_block_chain; node;) { - SpinLockGuard _(m_lock); - for (size_t i = 0; i < m_block_chain_length; i++) - temp_block_chain[i] = m_block_chain[i]; - temp_block_chain_length = m_block_chain_length; - m_block_chain_length = 0; + auto* next = node->block_chain_next; + + ASSERT(node->blocked); + ASSERT(node->blocker == this); + + node->blocker.store(nullptr); + node->block_chain_prev = nullptr; + node->block_chain_next = nullptr; + + Processor::scheduler().unblock_thread(node); + + node = next; } - for (size_t i = 0; i < temp_block_chain_length; i++) - Processor::scheduler().unblock_thread(temp_block_chain[i]); + m_block_chain = nullptr; } void ThreadBlocker::add_thread_to_block_queue(SchedulerQueue::Node* node) { - ASSERT(node->blocker_lock.current_processor_has_lock()); - SpinLockGuard _(m_lock); - ASSERT(m_block_chain_length < sizeof(m_block_chain) / sizeof(m_block_chain[0])); - - ASSERT(node); ASSERT(node->blocked); ASSERT(node->blocker == nullptr); - for (size_t i = 0 ; i < m_block_chain_length; i++) - ASSERT(m_block_chain[i] != node); - m_block_chain[m_block_chain_length++] = node; + node->blocker.store(this); + node->block_chain_prev = nullptr; + node->block_chain_next = m_block_chain; - node->blocker = this; + if (m_block_chain) + m_block_chain->block_chain_prev = node; + m_block_chain = node; } - void ThreadBlocker::remove_blocked_thread(SchedulerQueue::Node* node) + void ThreadBlocker::remove_thread_from_block_queue(SchedulerQueue::Node* node) { - ASSERT(node->blocker_lock.current_processor_has_lock()); - SpinLockGuard _(m_lock); - ASSERT(node); + // NOTE: this is possible if we got here while another + // core was doing an unblock on this blocker + if (node->blocker.load() != this) + return; ASSERT(node->blocked); - ASSERT(node->blocker == this); - for (size_t i = 0 ; i < m_block_chain_length; i++) - { - if (m_block_chain[i] != node) - continue; - for (size_t j = i + 1; j < m_block_chain_length; j++) - m_block_chain[j - 1] = m_block_chain[j]; - m_block_chain_length--; - } + if (node->block_chain_prev) + node->block_chain_prev->block_chain_next = node->block_chain_next; + if (node->block_chain_next) + node->block_chain_next->block_chain_prev = node->block_chain_prev; - node->blocker = nullptr; + if (node == m_block_chain) + m_block_chain = node->block_chain_next; + + node->blocker.store(nullptr); + node->block_chain_prev = nullptr; + node->block_chain_next = nullptr; } }