From 74d7a48c4b942adbecaa343d3e5a13021bf4f2b7 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sat, 22 Aug 2026 21:06:50 +0300 Subject: [PATCH] Kernel: Make TTY write lock a spinlock I don't really like this but its needed with the current architecture to allow printing while interrupts are disabled. I was hitting a dead lock on few real machines --- kernel/include/kernel/Terminal/TTY.h | 4 +- kernel/include/kernel/Terminal/VirtualTTY.h | 2 + kernel/kernel/Terminal/PseudoTerminal.cpp | 2 +- kernel/kernel/Terminal/TTY.cpp | 32 +++---- kernel/kernel/Terminal/VirtualTTY.cpp | 96 +++++++++++---------- 5 files changed, 71 insertions(+), 65 deletions(-) diff --git a/kernel/include/kernel/Terminal/TTY.h b/kernel/include/kernel/Terminal/TTY.h index 644bfe65..eabbea24 100644 --- a/kernel/include/kernel/Terminal/TTY.h +++ b/kernel/include/kernel/Terminal/TTY.h @@ -100,9 +100,9 @@ namespace Kernel termios m_termios; protected: - Mutex m_mutex; + Mutex m_input_mutex; - Mutex m_write_lock; + RecursiveSpinLock m_write_lock; ThreadBlocker m_write_blocker; }; diff --git a/kernel/include/kernel/Terminal/VirtualTTY.h b/kernel/include/kernel/Terminal/VirtualTTY.h index 80ea5f0e..9fe3e045 100644 --- a/kernel/include/kernel/Terminal/VirtualTTY.h +++ b/kernel/include/kernel/Terminal/VirtualTTY.h @@ -21,6 +21,8 @@ namespace Kernel void clear() override; + BAN::ErrorOr set_terminal_driver(BAN::RefPtr); + protected: BAN::StringView name() const override { return m_name; } bool putchar_impl(uint8_t ch) override; diff --git a/kernel/kernel/Terminal/PseudoTerminal.cpp b/kernel/kernel/Terminal/PseudoTerminal.cpp index 48b58780..67a35555 100644 --- a/kernel/kernel/Terminal/PseudoTerminal.cpp +++ b/kernel/kernel/Terminal/PseudoTerminal.cpp @@ -146,7 +146,7 @@ namespace Kernel auto slave = m_slave.lock(); if (!slave) return BAN::Error::from_errno(EIO); - LockGuard _(slave->m_mutex); + LockGuard _(slave->m_input_mutex); for (size_t i = 0; i < buffer.size(); i++) slave->handle_input_byte(buffer[i]); return buffer.size(); diff --git a/kernel/kernel/Terminal/TTY.cpp b/kernel/kernel/Terminal/TTY.cpp index fd04f551..2e0540a2 100644 --- a/kernel/kernel/Terminal/TTY.cpp +++ b/kernel/kernel/Terminal/TTY.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -220,7 +221,7 @@ namespace Kernel if (ansi_c_str == nullptr) return; - LockGuard _(m_mutex); + LockGuard _(m_input_mutex); while (*ansi_c_str) handle_input_byte(*ansi_c_str++); after_write(); @@ -269,7 +270,7 @@ namespace Kernel bool should_flush = false; bool force_echo = false; - LockGuard _(m_mutex); + LockGuard _(m_input_mutex); if (!(termios.c_lflag & ICANON)) should_flush = true; @@ -377,7 +378,8 @@ namespace Kernel const auto termios = get_termios(); - LockGuard _(m_write_lock); + SpinLockGuard _(m_write_lock); + if (termios.c_oflag & OPOST) { if ((termios.c_oflag & ONLCR) && ch == NL) @@ -385,14 +387,16 @@ namespace Kernel if ((termios.c_oflag & OCRNL) && ch == CR) return putchar_impl(NL); } + return putchar_impl(ch); } BAN::ErrorOr TTY::read_impl(off_t, BAN::ByteSpan buffer) { - LockGuard _(m_mutex); + LockGuard _(m_input_mutex); + while (!m_output.flush) - TRY(Thread::current().block_or_eintr_indefinite(m_output.thread_blocker, &m_mutex)); + TRY(Thread::current().block_or_eintr_indefinite(m_output.thread_blocker, &m_input_mutex)); if (m_output.buffer->empty()) { @@ -424,13 +428,14 @@ namespace Kernel BAN::ErrorOr TTY::write_impl(off_t, BAN::ConstByteSpan buffer) { - LockGuard write_guard(m_write_lock); + SpinLockGuard _(m_write_lock); while (!can_write()) { if (master_has_closed()) return BAN::Error::from_errno(EIO); - TRY(Thread::current().block_or_eintr_indefinite(m_write_blocker, &m_write_lock)); + BlockableSpinLock block(m_write_lock); + TRY(Thread::current().block_or_eintr_indefinite(m_write_blocker, &block)); } size_t written = 0; @@ -447,15 +452,12 @@ namespace Kernel void TTY::putchar_current(uint8_t ch) { - ASSERT(s_tty); + auto tty = s_tty; + ASSERT(tty); - while (!s_tty->m_write_lock.try_lock()) - Processor::pause(); - - s_tty->putchar(ch); - s_tty->after_write(); - - s_tty->m_write_lock.unlock(); + SpinLockGuard _(tty->m_write_lock); + tty->putchar(ch); + tty->after_write(); } bool TTY::is_initialized() diff --git a/kernel/kernel/Terminal/VirtualTTY.cpp b/kernel/kernel/Terminal/VirtualTTY.cpp index 638a0b6e..f826b6f7 100644 --- a/kernel/kernel/Terminal/VirtualTTY.cpp +++ b/kernel/kernel/Terminal/VirtualTTY.cpp @@ -50,17 +50,48 @@ namespace Kernel , m_foreground(driver->palette()[15]) , m_background(driver->palette()[0]) { - m_width = m_terminal_driver->width(); - m_height = m_terminal_driver->height(); - update_winsize(m_width, m_height); + MUST(set_terminal_driver(driver)); + } - m_buffer = new Cell[m_width * m_height]; - ASSERT(m_buffer); + BAN::ErrorOr VirtualTTY::set_terminal_driver(BAN::RefPtr terminal_driver) + { + const uint32_t new_width = terminal_driver->width(); + const uint32_t new_height = terminal_driver->height(); + + SpinLockGuard _(m_write_lock); + + if (m_width != new_width || m_height != new_height) + { + Cell* new_buffer = new Cell[new_width * new_height]; + ASSERT(new_buffer); + + for (uint32_t i = 0; i < new_width * m_height; i++) + new_buffer[i] = { .foreground = m_foreground, .background = m_background, .codepoint = ' ' }; + + for (uint32_t y = 0; y < BAN::Math::min(m_height, new_height); y++) + for (uint32_t x = 0; x < BAN::Math::min(m_width, new_width); x++) + new_buffer[y * new_width + x] = m_buffer[y * m_width + x]; + + delete[] m_buffer; + m_buffer = new_buffer; + m_width = new_width; + m_height = new_height; + } + + m_terminal_driver = terminal_driver; + + for (uint32_t y = 0; y < m_height; y++) + for (uint32_t x = 0; x < m_width; x++) + render_from_buffer(x, y); + + update_winsize(new_width, new_height); + + return {}; } void VirtualTTY::clear() { - LockGuard _(m_write_lock); + SpinLockGuard _(m_write_lock); for (uint32_t i = 0; i < m_width * m_height; i++) m_buffer[i] = { .foreground = m_foreground, .background = m_background, .codepoint = ' ' }; m_terminal_driver->clear(m_background); @@ -70,46 +101,14 @@ namespace Kernel { if (!m_terminal_driver->has_font()) return BAN::Error::from_errno(EINVAL); - - { - LockGuard _(m_write_lock); - - TRY(m_terminal_driver->set_font(BAN::move(font))); - - uint32_t new_width = m_terminal_driver->width(); - uint32_t new_height = m_terminal_driver->height(); - - if (m_width != new_width || m_height != new_height) - { - Cell* new_buffer = new Cell[new_width * new_height]; - ASSERT(new_buffer); - - for (uint32_t i = 0; i < new_width * m_height; i++) - new_buffer[i] = { .foreground = m_foreground, .background = m_background, .codepoint = ' ' }; - - for (uint32_t y = 0; y < BAN::Math::min(m_height, new_height); y++) - for (uint32_t x = 0; x < BAN::Math::min(m_width, new_width); x++) - new_buffer[y * new_width + x] = m_buffer[y * m_width + x]; - - delete[] m_buffer; - m_buffer = new_buffer; - m_width = new_width; - m_height = new_height; - } - - for (uint32_t y = 0; y < m_height; y++) - for (uint32_t x = 0; x < m_width; x++) - render_from_buffer(x, y); - } - - update_winsize(m_width, m_height); - + TRY(m_terminal_driver->set_font(BAN::move(font))); + MUST(set_terminal_driver(m_terminal_driver)); return {}; } void VirtualTTY::reset_ansi() { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); m_ansi_state = { .nums = { -1, -1, -1, -1, -1 }, .index = 0, @@ -120,7 +119,7 @@ namespace Kernel void VirtualTTY::handle_ansi_csi_color(uint8_t value) { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); auto& palette = m_terminal_driver->palette(); @@ -203,7 +202,8 @@ namespace Kernel void VirtualTTY::handle_ansi_csi(uint8_t ch) { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); + switch (ch) { case '0' ... '9': @@ -445,7 +445,7 @@ namespace Kernel void VirtualTTY::render_from_buffer(uint32_t x, uint32_t y) { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); ASSERT(x < m_width && y < m_height); const auto& cell = m_buffer[y * m_width + x]; m_terminal_driver->putchar_at(cell.codepoint, x, y, cell.foreground, cell.background); @@ -453,7 +453,7 @@ namespace Kernel void VirtualTTY::putchar_at(uint32_t codepoint, uint32_t x, uint32_t y) { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); ASSERT(x < m_width && y < m_height); auto& cell = m_buffer[y * m_width + x]; cell.codepoint = codepoint; @@ -464,6 +464,8 @@ namespace Kernel void VirtualTTY::scroll_if_needed() { + ASSERT(m_write_lock.current_processor_has_lock()); + while (m_row >= m_height) { memmove(m_buffer, m_buffer + m_width, m_width * (m_height - 1) * sizeof(Cell)); @@ -487,7 +489,7 @@ namespace Kernel void VirtualTTY::putcodepoint(uint32_t codepoint) { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); switch (codepoint) { @@ -533,7 +535,7 @@ namespace Kernel bool VirtualTTY::putchar_impl(uint8_t ch) { - ASSERT(m_write_lock.is_locked_by_current_thread()); + ASSERT(m_write_lock.current_processor_has_lock()); uint32_t codepoint = ch;