From f067730f6be9bd10f299d295adfd49175f4e91d7 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 1 Dec 2019 11:57:20 +0100 Subject: [PATCH] Kernel: Add a WaitQueue for Thread queueing/waking and use it for Lock The kernel's Lock class now uses a proper wait queue internally instead of just having everyone wake up regularly to try to acquire the lock. We also keep the donation mechanism, so that whenever someone tries to take the lock and fails, that thread donates the remainder of its timeslice to the current lock holder. After unlocking a Lock, the unlocking thread calls WaitQueue::wake_one, which unblocks the next thread in queue. --- Kernel/Lock.cpp | 10 ++++++++-- Kernel/Lock.h | 5 +++-- Kernel/Makefile | 1 + Kernel/Scheduler.cpp | 18 ++++++++++++++++-- Kernel/Thread.cpp | 8 ++++++++ Kernel/Thread.h | 34 +++++++++++++++++++++++++++++++--- Kernel/WaitQueue.cpp | 34 ++++++++++++++++++++++++++++++++++ Kernel/WaitQueue.h | 18 ++++++++++++++++++ 8 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 Kernel/WaitQueue.cpp create mode 100644 Kernel/WaitQueue.h diff --git a/Kernel/Lock.cpp b/Kernel/Lock.cpp index 511d4b3e72..7ff87359c1 100644 --- a/Kernel/Lock.cpp +++ b/Kernel/Lock.cpp @@ -1,4 +1,5 @@ #include +#include void Lock::lock() { @@ -18,8 +19,8 @@ void Lock::lock() return; } m_lock.store(false, AK::memory_order_release); + (void)current->donate_remaining_timeslice_and_block(m_holder, m_name, m_queue); } - Scheduler::donate_to(m_holder, m_name); } } @@ -36,10 +37,12 @@ void Lock::unlock() return; } m_holder = nullptr; + m_queue.wake_one(); m_lock.store(false, AK::memory_order_release); return; } - Scheduler::donate_to(m_holder, m_name); + // I don't know *who* is using "m_lock", so just yield. + Scheduler::yield(); } } @@ -64,7 +67,10 @@ bool Lock::unlock_if_locked() } m_holder = nullptr; m_lock.store(false, AK::memory_order_release); + m_queue.wake_one(); return true; } + // I don't know *who* is using "m_lock", so just yield. + Scheduler::yield(); } } diff --git a/Kernel/Lock.h b/Kernel/Lock.h index 77893f2819..8aa7aca5b1 100644 --- a/Kernel/Lock.h +++ b/Kernel/Lock.h @@ -1,11 +1,12 @@ #pragma once #include -#include #include +#include #include #include #include +#include class Thread; extern Thread* current; @@ -29,6 +30,7 @@ private: u32 m_level { 0 }; Thread* m_holder { nullptr }; const char* m_name { nullptr }; + WaitQueue m_queue; }; class Locker { @@ -46,7 +48,6 @@ private: Lock& m_lock; }; - #define LOCKER(lock) Locker locker(lock) template diff --git a/Kernel/Makefile b/Kernel/Makefile index 6c8544cbc1..bcc8b014a0 100644 --- a/Kernel/Makefile +++ b/Kernel/Makefile @@ -96,6 +96,7 @@ CXX_OBJS = \ VM/RangeAllocator.o \ VM/Region.o \ VM/VMObject.o \ + WaitQueue.o \ init.o \ kprintf.o diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 816abd5fed..f64c0e3647 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -82,9 +82,22 @@ bool Thread::JoinBlocker::should_unblock(Thread& joiner, time_t, long) return !joiner.m_joinee; } +Thread::WaitQueueBlocker::WaitQueueBlocker(WaitQueue& queue) + : m_queue(queue) +{ + m_queue.enqueue(*current); +} + +bool Thread::WaitQueueBlocker::should_unblock(Thread&, time_t, long) +{ + // Someone else will have to unblock us by calling wake_one() or wake_all() on the queue. + return false; +} + Thread::FileDescriptionBlocker::FileDescriptionBlocker(const FileDescription& description) : m_blocked_description(description) -{} +{ +} const FileDescription& Thread::FileDescriptionBlocker::blocked_description() const { @@ -236,7 +249,8 @@ bool Thread::WaitBlocker::should_unblock(Thread& thread, time_t, long) Thread::SemiPermanentBlocker::SemiPermanentBlocker(Reason reason) : m_reason(reason) -{} +{ +} bool Thread::SemiPermanentBlocker::should_unblock(Thread&, time_t, long) { diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index b792e2e97e..eacc56d951 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -180,6 +180,14 @@ void Thread::yield_without_holding_big_lock() process().big_lock().lock(); } +void Thread::donate_and_yield_without_holding_big_lock(Thread* beneficiary, const char* reason) +{ + bool did_unlock = process().big_lock().unlock_if_locked(); + Scheduler::donate_to(beneficiary, reason); + if (did_unlock) + process().big_lock().lock(); +} + u64 Thread::sleep(u32 ticks) { ASSERT(state() == Thread::Running); diff --git a/Kernel/Thread.h b/Kernel/Thread.h index 74d4c8e938..6fdcf30b79 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -18,6 +18,7 @@ class FileDescription; class Process; class ProcessInspectionHandle; class Region; +class WaitQueue; enum class ShouldUnblockThread { No = 0, @@ -110,6 +111,16 @@ public: void*& m_joinee_exit_value; }; + class WaitQueueBlocker final : public Blocker { + public: + explicit WaitQueueBlocker(WaitQueue&); + virtual bool should_unblock(Thread&, time_t now_s, long us) override; + virtual const char* state_string() const override { return "WaitQueued"; } + + private: + WaitQueue& m_queue; + }; + class FileDescriptionBlocker : public Blocker { public: const FileDescription& blocked_description() const; @@ -257,7 +268,7 @@ public: }; template - [[nodiscard]] BlockResult block(Args&&... args) + [[nodiscard]] BlockResult block_impl(Thread* beneficiary, const char* reason, Args&&... args) { // We should never be blocking a blocked (or otherwise non-active) thread. ASSERT(state() == Thread::Running); @@ -270,7 +281,11 @@ public: set_state(Thread::Blocked); // Yield to the scheduler, and wait for us to resume unblocked. - yield_without_holding_big_lock(); + if (beneficiary) { + donate_and_yield_without_holding_big_lock(beneficiary, reason); + } else { + yield_without_holding_big_lock(); + } // We should no longer be blocked once we woke up ASSERT(state() != Thread::Blocked); @@ -282,7 +297,19 @@ public: return BlockResult::InterruptedBySignal; return BlockResult::WokeNormally; - }; + } + + template + [[nodiscard]] BlockResult block(Args&&... args) + { + return block_impl(nullptr, nullptr, forward(args)...); + } + + template + [[nodiscard]] BlockResult donate_remaining_timeslice_and_block(Thread* beneficiary, const char* reason, Args&&... args) + { + return block_impl(beneficiary, reason, forward(args)...); + } [[nodiscard]] BlockResult block_until(const char* state_string, Function&& condition) { @@ -398,6 +425,7 @@ private: bool m_should_die { false }; void yield_without_holding_big_lock(); + void donate_and_yield_without_holding_big_lock(Thread* beneficiary, const char* reason); }; HashTable& thread_table(); diff --git a/Kernel/WaitQueue.cpp b/Kernel/WaitQueue.cpp new file mode 100644 index 0000000000..c0935f05a1 --- /dev/null +++ b/Kernel/WaitQueue.cpp @@ -0,0 +1,34 @@ +#include +#include + +WaitQueue::WaitQueue() +{ +} + +WaitQueue::~WaitQueue() +{ +} + +void WaitQueue::enqueue(Thread& thread) +{ + InterruptDisabler disabler; + m_threads.append(&thread); +} + +void WaitQueue::wake_one() +{ + InterruptDisabler disabler; + if (m_threads.is_empty()) + return; + if (auto* thread = m_threads.take_first()) + thread->unblock(); +} + +void WaitQueue::wake_all() +{ + InterruptDisabler disabler; + if (m_threads.is_empty()) + return; + while (!m_threads.is_empty()) + m_threads.take_first()->unblock(); +} diff --git a/Kernel/WaitQueue.h b/Kernel/WaitQueue.h new file mode 100644 index 0000000000..0d4a820cab --- /dev/null +++ b/Kernel/WaitQueue.h @@ -0,0 +1,18 @@ +#pragma once + +#include + +class Thread; + +class WaitQueue { +public: + WaitQueue(); + ~WaitQueue(); + + void enqueue(Thread&); + void wake_one(); + void wake_all(); + +private: + SinglyLinkedList m_threads; +};