From 4fef895edab80cbe2d9f886f5b769973af93a040 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 16 Jan 2019 17:20:58 +0100 Subject: [PATCH] Rework WindowServer to use select() in its main event loop. The system can finally idle without burning CPU. :^) There are some issues with scheduling making the mouse cursor sloppy and unresponsive that need to be dealt with. --- Kernel/Keyboard.cpp | 7 +++- Kernel/Keyboard.h | 12 +++---- Kernel/MemoryManager.cpp | 4 +-- Kernel/PS2MouseDevice.cpp | 1 + Kernel/Process.cpp | 20 +++++++++-- Kernel/Process.h | 7 +++- Kernel/ProcessGUI.cpp | 3 +- Kernel/Scheduler.cpp | 5 +++ Kernel/init.cpp | 7 ++-- Kernel/sync.sh | 1 + WindowServer/WSEventLoop.cpp | 66 ++++++++++++++++++++++++++++++++---- WindowServer/WSEventLoop.h | 7 +++- WindowServer/WSFrameBuffer.h | 2 +- WindowServer/WSScreen.cpp | 4 +-- WindowServer/WSScreen.h | 8 ++--- 15 files changed, 121 insertions(+), 33 deletions(-) diff --git a/Kernel/Keyboard.cpp b/Kernel/Keyboard.cpp index 8aa18b32a3..ecbd936346 100644 --- a/Kernel/Keyboard.cpp +++ b/Kernel/Keyboard.cpp @@ -125,7 +125,12 @@ ssize_t Keyboard::read(Process&, byte* buffer, size_t size) while ((size_t)nread < size) { if (m_queue.is_empty()) break; - buffer[nread++] = m_queue.dequeue().character; + // Don't return partial data frames. + if ((size - nread) < 2) + break; + auto key = m_queue.dequeue(); + buffer[nread++] = key.character; + buffer[nread++] = key.modifiers; } return nread; } diff --git a/Kernel/Keyboard.h b/Kernel/Keyboard.h index 7fab83bb44..e7debc403b 100644 --- a/Kernel/Keyboard.h +++ b/Kernel/Keyboard.h @@ -32,16 +32,16 @@ public: void set_client(KeyboardClient* client) { m_client = client; } + // ^CharacterDevice + virtual ssize_t read(Process&, byte* buffer, size_t) override; + virtual bool can_read(Process&) const override; + virtual ssize_t write(Process&, const byte* buffer, size_t) override; + virtual bool can_write(Process&) const override { return true; } + private: // ^IRQHandler virtual void handle_irq() override; - // ^CharacterDevice - virtual ssize_t read(Process&, byte* buffer, size_t) override; - virtual ssize_t write(Process&, const byte* buffer, size_t) override; - virtual bool can_read(Process&) const override; - virtual bool can_write(Process&) const override { return true; } - void emit(byte); KeyboardClient* m_client { nullptr }; diff --git a/Kernel/MemoryManager.cpp b/Kernel/MemoryManager.cpp index 37cb27619a..6618457f28 100644 --- a/Kernel/MemoryManager.cpp +++ b/Kernel/MemoryManager.cpp @@ -537,7 +537,7 @@ bool MemoryManager::validate_user_read(const Process& process, LinearAddress lad auto pte = PageTableEntry(&pde.pageTableBase()[pageTableIndex]); if (!pte.is_present()) return false; - if (!pte.is_user_allowed()) + if (process.isRing3() && !pte.is_user_allowed()) return false; return true; } @@ -552,7 +552,7 @@ bool MemoryManager::validate_user_write(const Process& process, LinearAddress la auto pte = PageTableEntry(&pde.pageTableBase()[pageTableIndex]); if (!pte.is_present()) return false; - if (!pte.is_user_allowed()) + if (process.isRing3() && !pte.is_user_allowed()) return false; if (!pte.is_writable()) return false; diff --git a/Kernel/PS2MouseDevice.cpp b/Kernel/PS2MouseDevice.cpp index 64360baf60..f64842bc72 100644 --- a/Kernel/PS2MouseDevice.cpp +++ b/Kernel/PS2MouseDevice.cpp @@ -129,6 +129,7 @@ ssize_t PS2MouseDevice::read(Process&, byte* buffer, size_t size) while ((size_t)nread < size) { if (m_queue.is_empty()) break; + // FIXME: Don't return partial data frames. buffer[nread++] = m_queue.dequeue(); } return nread; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index eee7fc70a7..8939fa6b35 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1295,6 +1295,7 @@ int Process::sys$open(const char* path, int options) if (number_of_open_file_descriptors() >= m_max_open_file_descriptors) return -EMFILE; int error = -EWHYTHO; + ASSERT(cwd_inode()); auto descriptor = VFS::the().open(path, error, options, cwd_inode()->identifier()); if (!descriptor) return error; @@ -1960,8 +1961,15 @@ int Process::sys$select(const Syscall::SC_select_params* params) transfer_fds(writefds, m_select_write_fds); transfer_fds(readfds, m_select_read_fds); - block(BlockedSelect); - Scheduler::yield(); +#ifdef DEBUG_IO + dbgprintf("%s<%u> selecting on (read:%u, write:%u)\n", name().characters(), pid(), m_select_read_fds.size(), m_select_write_fds.size()); +#endif + + if (!m_wakeup_requested) { + block(BlockedSelect); + Scheduler::yield(); + } + m_wakeup_requested = false; int markedfds = 0; @@ -1989,3 +1997,11 @@ int Process::sys$select(const Syscall::SC_select_params* params) return markedfds; } + +Inode* Process::cwd_inode() +{ + // FIXME: This is retarded factoring. + if (!m_cwd) + m_cwd = VFS::the().root_inode(); + return m_cwd.ptr(); +} diff --git a/Kernel/Process.h b/Kernel/Process.h index e79b37fa81..8d8238dccb 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -236,7 +236,7 @@ public: template bool validate_read_typed(T* value, size_t count = 1) { return validate_read(value, sizeof(T) * count); } template bool validate_write_typed(T* value, size_t count = 1) { return validate_write(value, sizeof(T) * count); } - Inode* cwd_inode() { return m_cwd.ptr(); } + Inode* cwd_inode(); Inode* executable_inode() { return m_executable.ptr(); } size_t number_of_open_file_descriptors() const; @@ -256,6 +256,9 @@ public: Vector& gui_events() { return m_gui_events; } SpinLock& gui_events_lock() { return m_gui_events_lock; } + bool wakeup_requested() { return m_wakeup_requested; } + void request_wakeup() { m_wakeup_requested = true; } + private: friend class MemoryManager; friend class Scheduler; @@ -357,6 +360,8 @@ private: Vector m_gui_events; SpinLock m_gui_events_lock; int m_next_window_id { 1 }; + + dword m_wakeup_requested { false }; }; extern Process* current; diff --git a/Kernel/ProcessGUI.cpp b/Kernel/ProcessGUI.cpp index ff3ff76fc7..bdd0636ba1 100644 --- a/Kernel/ProcessGUI.cpp +++ b/Kernel/ProcessGUI.cpp @@ -108,8 +108,7 @@ int Process::gui$invalidate_window(int window_id) if (it == m_windows.end()) return -EBADWINDOW; auto& window = *(*it).value; - // FIXME: This should queue up a message that the window server process can read. - // Poking into its data structures is not good. WSEventLoop::the().post_event(&window, make(WSEvent::WM_Invalidate)); + WSEventLoop::the().server_process().request_wakeup(); return 0; } diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index f8f546db1c..24bb48541d 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -65,6 +65,11 @@ bool Scheduler::pick_next() } if (process.state() == Process::BlockedSelect) { + if (process.wakeup_requested()) { + process.m_wakeup_requested = false; + process.unblock(); + return true; + } for (int fd : process.m_select_read_fds) { if (process.m_fds[fd].descriptor->can_read(process)) { process.unblock(); diff --git a/Kernel/init.cpp b/Kernel/init.cpp index 1a6ce48bb8..47822bb317 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -37,6 +37,7 @@ VirtualConsole* tty3; Keyboard* keyboard; PS2MouseDevice* ps2mouse; GUIEventDevice* gui_event_device; +VFS* vfs; #ifdef STRESS_TEST_SPAWNING static void spawn_stress() NORETURN; @@ -62,8 +63,6 @@ static void init_stage2() { Syscall::initialize(); - auto vfs = make(); - auto dev_zero = make(); vfs->register_character_device(*dev_zero); @@ -138,6 +137,9 @@ void init() gdt_init(); idt_init(); + VFS::initialize_globals(); + vfs = new VFS; + keyboard = new Keyboard; ps2mouse = new PS2MouseDevice; gui_event_device = new GUIEventDevice; @@ -153,7 +155,6 @@ void init() MemoryManager::initialize(); - VFS::initialize_globals(); StringImpl::initialize_globals(); PIT::initialize(); diff --git a/Kernel/sync.sh b/Kernel/sync.sh index 85e619b0cc..180586f108 100755 --- a/Kernel/sync.sh +++ b/Kernel/sync.sh @@ -8,6 +8,7 @@ mknod mnt/dev/tty0 c 4 0 mknod mnt/dev/tty1 c 4 1 mknod mnt/dev/tty2 c 4 2 mknod mnt/dev/tty3 c 4 3 +mknod mnt/dev/keyboard c 85 1 mknod mnt/dev/psaux c 10 1 mknod mnt/dev/ptmx c 5 2 mknod mnt/dev/ptm0 c 10 0 diff --git a/WindowServer/WSEventLoop.cpp b/WindowServer/WSEventLoop.cpp index a6769a3510..24ed44ce35 100644 --- a/WindowServer/WSEventLoop.cpp +++ b/WindowServer/WSEventLoop.cpp @@ -4,7 +4,9 @@ #include "WSWindowManager.h" #include "WSScreen.h" #include "PS2MouseDevice.h" -#include "Scheduler.h" +#include +#include +#include "Process.h" //#define WSEVENTLOOP_DEBUG @@ -34,10 +36,19 @@ WSEventLoop& WSEventLoop::the() int WSEventLoop::exec() { m_server_process = current; + + m_keyboard_fd = m_server_process->sys$open("/dev/keyboard", O_RDONLY); + m_mouse_fd = m_server_process->sys$open("/dev/psaux", O_RDONLY); + + ASSERT(m_keyboard_fd >= 0); + ASSERT(m_mouse_fd >= 0); + m_running = true; for (;;) { + if (m_queued_events.is_empty()) - waitForEvent(); + wait_for_event(); + Vector events; { LOCKER(m_lock); @@ -69,20 +80,48 @@ void WSEventLoop::post_event(WSEventReceiver* receiver, OwnPtr&& event) dbgprintf("WSEventLoop::post_event: {%u} << receiver=%p, event=%p\n", m_queued_events.size(), receiver, event.ptr()); #endif m_queued_events.append({ receiver, move(event) }); + + if (current != m_server_process) + m_server_process->request_wakeup(); } -void WSEventLoop::waitForEvent() +void WSEventLoop::wait_for_event() +{ + fd_set rfds; + memset(&rfds, 0, sizeof(rfds)); + auto bitmap = Bitmap::wrap((byte*)&rfds, FD_SETSIZE); + bitmap.set(m_keyboard_fd, true); + bitmap.set(m_mouse_fd, true); + Syscall::SC_select_params params; + params.nfds = max(m_keyboard_fd, m_mouse_fd) + 1; + params.readfds = &rfds; + params.writefds = nullptr; + params.exceptfds = nullptr; + params.timeout = nullptr; + int rc = m_server_process->sys$select(¶ms); + memory_barrier(); + if (rc < 0) { + ASSERT_NOT_REACHED(); + } + + //if (bitmap.get(m_keyboard_fd)) + drain_keyboard(); + //if (bitmap.get(m_mouse_fd)) + drain_mouse(); +} + +void WSEventLoop::drain_mouse() { - auto& mouse = PS2MouseDevice::the(); auto& screen = WSScreen::the(); + auto& mouse = PS2MouseDevice::the(); bool prev_left_button = screen.left_mouse_button_pressed(); bool prev_right_button = screen.right_mouse_button_pressed(); int dx = 0; int dy = 0; while (mouse.can_read(*m_server_process)) { signed_byte data[3]; - ssize_t nread = mouse.read(*m_server_process, (byte*)data, 3); - ASSERT(nread == 3); + ssize_t nread = mouse.read(*m_server_process, (byte*)data, sizeof(data)); + ASSERT(nread == sizeof(data)); bool left_button = data[0] & 1; bool right_button = data[0] & 2; dx += data[1]; @@ -96,3 +135,18 @@ void WSEventLoop::waitForEvent() } } } + +void WSEventLoop::drain_keyboard() +{ + auto& screen = WSScreen::the(); + auto& keyboard = Keyboard::the(); + while (keyboard.can_read(*m_server_process)) { + byte data[2]; + ssize_t nread = keyboard.read(*m_server_process, (byte*)data, sizeof(data)); + ASSERT(nread == sizeof(data)); + Keyboard::Key key; + key.character = data[0]; + key.modifiers = data[1]; + screen.on_receive_keyboard_data(key); + } +} diff --git a/WindowServer/WSEventLoop.h b/WindowServer/WSEventLoop.h index d461a6b52a..e7695d8070 100644 --- a/WindowServer/WSEventLoop.h +++ b/WindowServer/WSEventLoop.h @@ -25,7 +25,9 @@ public: Process& server_process() { return *m_server_process; } private: - void waitForEvent(); + void wait_for_event(); + void drain_mouse(); + void drain_keyboard(); SpinLock m_lock; @@ -37,4 +39,7 @@ private: Process* m_server_process { nullptr }; bool m_running { false }; + + int m_keyboard_fd { -1 }; + int m_mouse_fd { -1 }; }; diff --git a/WindowServer/WSFrameBuffer.h b/WindowServer/WSFrameBuffer.h index 8239fdfd9a..a24bc82f10 100644 --- a/WindowServer/WSFrameBuffer.h +++ b/WindowServer/WSFrameBuffer.h @@ -9,7 +9,7 @@ class WSFrameBuffer final : public WSScreen { public: WSFrameBuffer(unsigned width, unsigned height); WSFrameBuffer(RGBA32*, unsigned width, unsigned height); - virtual ~WSFrameBuffer() override; + ~WSFrameBuffer(); void show(); diff --git a/WindowServer/WSScreen.cpp b/WindowServer/WSScreen.cpp index 2276ff6f5d..d1ce9ec132 100644 --- a/WindowServer/WSScreen.cpp +++ b/WindowServer/WSScreen.cpp @@ -25,8 +25,6 @@ WSScreen::WSScreen(unsigned width, unsigned height) s_the = this; m_cursor_location = rect().center(); - - Keyboard::the().set_client(this); } WSScreen::~WSScreen() @@ -62,7 +60,7 @@ void WSScreen::on_receive_mouse_data(int dx, int dy, bool left_button, bool righ WSWindowManager::the().draw_cursor(); } -void WSScreen::on_key_pressed(Keyboard::Key key) +void WSScreen::on_receive_keyboard_data(Keyboard::Key key) { auto event = make(WSEvent::KeyDown, 0); int key_code = 0; diff --git a/WindowServer/WSScreen.h b/WindowServer/WSScreen.h index 0967bfcbb1..240660ac40 100644 --- a/WindowServer/WSScreen.h +++ b/WindowServer/WSScreen.h @@ -4,9 +4,9 @@ #include #include -class WSScreen : public KeyboardClient { +class WSScreen { public: - virtual ~WSScreen(); + ~WSScreen(); int width() const { return m_width; } int height() const { return m_height; } @@ -23,14 +23,12 @@ public: bool right_mouse_button_pressed() const { return m_right_mouse_button_pressed; } void on_receive_mouse_data(int dx, int dy, bool left_button, bool right_button); + void on_receive_keyboard_data(Keyboard::Key); protected: WSScreen(unsigned width, unsigned height); private: - // ^KeyboardClient - virtual void on_key_pressed(Keyboard::Key) final; - int m_width { 0 }; int m_height { 0 };