From 5582a0a254c691db0c2ea8cf79bdbb0a2d74f00a Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 7 Feb 2019 11:12:23 +0100 Subject: [PATCH] Kernel: When a lock is busy, donate remaining process ticks to lock holder. Since we know who's holding the lock, and we're gonna have to yield anyway, we can just ask the scheduler to donate any remaining ticks to that process. --- AK/Lock.h | 9 ++++++--- Kernel/DoubleBuffer.h | 1 + Kernel/FileSystem.cpp | 3 ++- Kernel/IDEDiskDevice.cpp | 1 + Kernel/PTYMultiplexer.cpp | 1 + Kernel/ProcFS.cpp | 5 +++-- Kernel/Process.cpp | 10 ++++++++++ Kernel/Process.h | 3 ++- Kernel/Scheduler.cpp | 24 ++++++++++++++++++------ Kernel/Scheduler.h | 1 + Kernel/SyntheticFileSystem.cpp | 1 + Userland/top.cpp | 2 +- WindowServer/WSMessageLoop.cpp | 1 + WindowServer/WSWindow.cpp | 3 ++- WindowServer/WSWindowManager.cpp | 1 + 15 files changed, 51 insertions(+), 15 deletions(-) diff --git a/AK/Lock.h b/AK/Lock.h index b5dc2e53965..50d5336c28c 100644 --- a/AK/Lock.h +++ b/AK/Lock.h @@ -27,16 +27,19 @@ static inline dword CAS(volatile dword* mem, dword newval, dword oldval) class Lock { public: - Lock() { } + Lock(const char* name = nullptr) : m_name(name) { } ~Lock() { } void lock(); void unlock(); + const char* name() const { return m_name; } + private: volatile dword m_lock { 0 }; dword m_level { 0 }; Process* m_holder { nullptr }; + const char* m_name { nullptr }; }; class Locker { @@ -65,7 +68,7 @@ inline void Lock::lock() } m_lock = 0; } - Scheduler::yield(); + Scheduler::donate_to(m_holder, m_name); } } @@ -86,7 +89,7 @@ inline void Lock::unlock() m_lock = 0; return; } - Scheduler::yield(); + Scheduler::donate_to(m_holder, m_name); } } diff --git a/Kernel/DoubleBuffer.h b/Kernel/DoubleBuffer.h index bc1efc0ac9f..e7212a5f8a9 100644 --- a/Kernel/DoubleBuffer.h +++ b/Kernel/DoubleBuffer.h @@ -9,6 +9,7 @@ public: DoubleBuffer() : m_write_buffer(&m_buffer1) , m_read_buffer(&m_buffer2) + , m_lock("DoubleBuffer") { } diff --git a/Kernel/FileSystem.cpp b/Kernel/FileSystem.cpp index 855ba34d9c0..953ef55abe0 100644 --- a/Kernel/FileSystem.cpp +++ b/Kernel/FileSystem.cpp @@ -85,7 +85,8 @@ FS::DirectoryEntry::DirectoryEntry(const char* n, size_t nl, InodeIdentifier i, } Inode::Inode(FS& fs, unsigned index) - : m_fs(fs) + : m_lock("Inode") + , m_fs(fs) , m_index(index) { all_inodes().set(this); diff --git a/Kernel/IDEDiskDevice.cpp b/Kernel/IDEDiskDevice.cpp index 944ffbe91c5..b036d78f731 100644 --- a/Kernel/IDEDiskDevice.cpp +++ b/Kernel/IDEDiskDevice.cpp @@ -39,6 +39,7 @@ RetainPtr IDEDiskDevice::create() IDEDiskDevice::IDEDiskDevice() : IRQHandler(IRQ_FIXED_DISK) + , m_lock("IDEDiskDevice") { initialize(); } diff --git a/Kernel/PTYMultiplexer.cpp b/Kernel/PTYMultiplexer.cpp index 75e5a2e26e2..ef3583a45f8 100644 --- a/Kernel/PTYMultiplexer.cpp +++ b/Kernel/PTYMultiplexer.cpp @@ -14,6 +14,7 @@ PTYMultiplexer& PTYMultiplexer::the() PTYMultiplexer::PTYMultiplexer() : CharacterDevice(5, 2) + , m_lock("PTYMultiplexer") { s_the = this; m_freelist.ensure_capacity(s_max_pty_pairs); diff --git a/Kernel/ProcFS.cpp b/Kernel/ProcFS.cpp index e80b61a18da..117caaff6ba 100644 --- a/Kernel/ProcFS.cpp +++ b/Kernel/ProcFS.cpp @@ -496,7 +496,7 @@ ByteBuffer procfs$all(InodeIdentifier) auto processes = Process::all_processes(); StringBuilder builder; auto build_process_line = [&builder] (Process* process) { - builder.appendf("%u,%u,%u,%u,%u,%u,%u,%s,%u,%u,%s,%s,%u,%u,%u,%u\n", + builder.appendf("%u,%u,%u,%u,%u,%u,%u,%s,%u,%u,%s,%s,%u,%u,%u,%u,%u\n", process->pid(), process->times_scheduled(), process->tty() ? process->tty()->pgid() : 0, @@ -512,7 +512,8 @@ ByteBuffer procfs$all(InodeIdentifier) process->amount_virtual(), process->amount_resident(), process->amount_shared(), - process->amount_in_bitmaps() + process->amount_in_bitmaps(), + process->ticks() ); }; build_process_line(Scheduler::colonel()); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 1fc1fb017e6..f6fae8c9d57 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2192,3 +2192,13 @@ void Process::finalize_dying_processes() for (auto* process : dying_processes) process->finalize(); } + +bool Process::tick() +{ + ++m_ticks; + if (tss().cs & 3) + ++m_ticks_in_user; + else + ++m_ticks_in_kernel; + return --m_ticks_left; +} diff --git a/Kernel/Process.h b/Kernel/Process.h index 41339740c89..2b0806626cd 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -131,8 +131,9 @@ public: template static void for_each_living(Callback); template void for_each_child(Callback); - bool tick() { ++m_ticks; return --m_ticks_left; } + bool tick(); void set_ticks_left(dword t) { m_ticks_left = t; } + dword ticks_left() const { return m_ticks_left; } void set_selector(word s) { m_far_ptr.selector = s; } void set_state(State s) { m_state = s; } diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 66f36902fe5..a3e2d6b8d13 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -192,6 +192,24 @@ bool Scheduler::pick_next() } } +bool Scheduler::donate_to(Process* beneficiary, const char* reason) +{ + (void)reason; + unsigned ticks_left = current->ticks_left(); + if (!beneficiary || beneficiary->state() != Process::Runnable || ticks_left <= 1) { + return yield(); + } + + unsigned ticks_to_donate = ticks_left - 1; +#ifdef SCHEDULER_DEBUG + dbgprintf("%s(%u) donating %u ticks to %s(%u), reason=%s\n", current->name().characters(), current->pid(), ticks_to_donate, beneficiary->name().characters(), beneficiary->pid(), reason); +#endif + context_switch(*beneficiary); + beneficiary->set_ticks_left(ticks_to_donate); + switch_now(); + return 0; +} + bool Scheduler::yield() { InterruptDisabler disabler; @@ -229,12 +247,6 @@ bool Scheduler::context_switch(Process& process) process.set_ticks_left(time_slice); process.did_schedule(); - if (process.tss().cs & 3) { - ++process.m_ticks_in_user; - } else { - ++process.m_ticks_in_kernel; - } - if (current == &process) return false; diff --git a/Kernel/Scheduler.h b/Kernel/Scheduler.h index b6db08dddb9..918b553b3a0 100644 --- a/Kernel/Scheduler.h +++ b/Kernel/Scheduler.h @@ -17,6 +17,7 @@ public: static void pick_next_and_switch_now(); static void switch_now(); static bool yield(); + static bool donate_to(Process*, const char* reason); static bool context_switch(Process&); static void prepare_to_modify_tss(Process&); static Process* colonel(); diff --git a/Kernel/SyntheticFileSystem.cpp b/Kernel/SyntheticFileSystem.cpp index 221168ee52e..a94fcab773f 100644 --- a/Kernel/SyntheticFileSystem.cpp +++ b/Kernel/SyntheticFileSystem.cpp @@ -11,6 +11,7 @@ RetainPtr SynthFS::create() } SynthFS::SynthFS() + : m_lock("SynthFS") { } diff --git a/Userland/top.cpp b/Userland/top.cpp index 2e0d20bfa7f..cc5c79e9c4d 100644 --- a/Userland/top.cpp +++ b/Userland/top.cpp @@ -43,7 +43,7 @@ static Snapshot get_snapshot() if (!ptr) break; auto parts = String(buf, Chomp).split(','); - if (parts.size() < 16) + if (parts.size() < 17) break; bool ok; pid_t pid = parts[0].to_uint(ok); diff --git a/WindowServer/WSMessageLoop.cpp b/WindowServer/WSMessageLoop.cpp index 224595a7e10..7ab5781e861 100644 --- a/WindowServer/WSMessageLoop.cpp +++ b/WindowServer/WSMessageLoop.cpp @@ -13,6 +13,7 @@ static WSMessageLoop* s_the; WSMessageLoop::WSMessageLoop() + : m_lock("WSMessageLoop") { if (!s_the) s_the = this; diff --git a/WindowServer/WSWindow.cpp b/WindowServer/WSWindow.cpp index e98a7ed3686..d651c916f7d 100644 --- a/WindowServer/WSWindow.cpp +++ b/WindowServer/WSWindow.cpp @@ -5,7 +5,8 @@ #include "Process.h" WSWindow::WSWindow(Process& process, int window_id) - : m_process(&process) + : m_lock("WSWindow") + , m_process(&process) , m_window_id(window_id) , m_pid(process.pid()) { diff --git a/WindowServer/WSWindowManager.cpp b/WindowServer/WSWindowManager.cpp index 0999ccde735..efc2c427f7f 100644 --- a/WindowServer/WSWindowManager.cpp +++ b/WindowServer/WSWindowManager.cpp @@ -130,6 +130,7 @@ void WSWindowManager::flip_buffers() WSWindowManager::WSWindowManager() : m_screen(WSScreen::the()) , m_screen_rect(m_screen.rect()) + , m_lock("WSWindowManager") { #ifndef DEBUG_COUNTERS (void)m_compose_count;