From ed82a18e2aab203e80bce433d74e07a63c6db58b Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sat, 10 Jan 2026 00:30:30 +0200 Subject: [PATCH] Kernel: Fix deadlock in ext2 filesystem If multiple threads were waiting for more block buffers without anyone releasing them, they ended up in a deadlock. Now we store 6 blocks for 8 threads. If a thread already has a block buffer, it will not have to wait for a new one. Only if there are more than 8 threads using blocks, will it block until there are free slots for a thread available. --- kernel/include/kernel/FS/Ext2/FileSystem.h | 48 +++++++------ kernel/kernel/FS/Ext2/FileSystem.cpp | 78 ++++++++++++++++++++-- 2 files changed, 98 insertions(+), 28 deletions(-) diff --git a/kernel/include/kernel/FS/Ext2/FileSystem.h b/kernel/include/kernel/FS/Ext2/FileSystem.h index 2c75a2aa..2f59d514 100644 --- a/kernel/include/kernel/FS/Ext2/FileSystem.h +++ b/kernel/include/kernel/FS/Ext2/FileSystem.h @@ -28,36 +28,28 @@ namespace Kernel BAN_NON_COPYABLE(BlockBufferWrapper); public: - BlockBufferWrapper(BAN::Span buffer, bool* used, Mutex* mutex, ThreadBlocker* blocker) + BlockBufferWrapper(BAN::Span buffer, void (*callback)(void*, const uint8_t*), void* argument) : m_buffer(buffer) - , m_used(used) - , m_mutex(mutex) - , m_blocker(blocker) - { - ASSERT(m_used && *m_used); - } + , m_callback(callback) + , m_argument(argument) + { } BlockBufferWrapper(BlockBufferWrapper&& other) { *this = BAN::move(other); } ~BlockBufferWrapper() { - if (m_used == nullptr) + if (m_callback == nullptr) return; - m_mutex->lock(); - *m_used = false; - m_blocker->unblock(); - m_mutex->unlock(); + m_callback(m_argument, m_buffer.data()); } BlockBufferWrapper& operator=(BlockBufferWrapper&& other) { this->m_buffer = other.m_buffer; - this->m_used = other.m_used; - this->m_mutex = other.m_mutex; - this->m_blocker = other.m_blocker; + this->m_callback = other.m_callback; + this->m_argument = other.m_argument; other.m_buffer = {}; - other.m_used = nullptr; - other.m_mutex = nullptr; - other.m_blocker = nullptr; + other.m_callback = nullptr; + other.m_argument = nullptr; return *this; } @@ -75,9 +67,8 @@ namespace Kernel private: BAN::Span m_buffer; - bool* m_used; - Mutex* m_mutex; - ThreadBlocker* m_blocker; + void (*m_callback)(void*, const uint8_t*); + void* m_argument; }; public: @@ -130,6 +121,9 @@ namespace Kernel BAN::ErrorOr initialize(size_t block_size); + private: + void destroy_callback(const uint8_t* buffer_ptr); + private: struct BlockBuffer { @@ -137,10 +131,20 @@ namespace Kernel bool used { false }; }; + struct ThreadInfo + { + pid_t tid { 0 }; + size_t buffers { 0 }; + }; + private: + static constexpr size_t max_threads = 8; + static constexpr size_t max_buffers_per_thread = 6; + Mutex m_buffer_mutex; ThreadBlocker m_buffer_blocker; - BAN::Array m_buffers; + BAN::Array m_buffers; + BAN::Array m_thread_infos; }; private: diff --git a/kernel/kernel/FS/Ext2/FileSystem.cpp b/kernel/kernel/FS/Ext2/FileSystem.cpp index bff8ca40..bc2557ae 100644 --- a/kernel/kernel/FS/Ext2/FileSystem.cpp +++ b/kernel/kernel/FS/Ext2/FileSystem.cpp @@ -532,22 +532,88 @@ namespace Kernel }; } - BAN::ErrorOr Ext2FS::BlockBufferManager::get_buffer() + void Ext2FS::BlockBufferManager::destroy_callback(const uint8_t* buffer_ptr) { + const auto tid = Thread::current_tid(); + LockGuard _(m_buffer_mutex); - for (;;) + for (auto& buffer : m_buffers) { - for (auto& buffer : m_buffers) + if (buffer.buffer.data() != buffer_ptr) + continue; + ASSERT(buffer.used); + buffer.used = false; + break; + } + + for (auto& info : m_thread_infos) + { + if (info.tid != tid) + continue; + ASSERT(info.buffers > 0); + info.buffers--; + if (info.buffers != 0) + break; + info.tid = 0; + m_buffer_blocker.unblock(); + break; + } + } + + BAN::ErrorOr Ext2FS::BlockBufferManager::get_buffer() + { + const auto tid = Thread::current_tid(); + ASSERT(tid); + + LockGuard _(m_buffer_mutex); + + ThreadInfo* thread_info = nullptr; + + for (auto& info : m_thread_infos) + { + if (info.tid != tid) + continue; + thread_info = &info; + break; + } + + while (thread_info == nullptr) + { + for (auto& info : m_thread_infos) { - if (buffer.used) + if (info.tid != 0) continue; - buffer.used = true; - return Ext2FS::BlockBufferWrapper(buffer.buffer.span(), &buffer.used, &m_buffer_mutex, &m_buffer_blocker); + thread_info = &info; + break; + } + + if (thread_info) + { + thread_info->tid = tid; + thread_info->buffers = 0; + break; } TRY(Thread::current().block_or_eintr_indefinite(m_buffer_blocker, &m_buffer_mutex)); } + + ASSERT(thread_info->buffers < max_buffers_per_thread); + thread_info->buffers++; + + for (auto& buffer : m_buffers) + { + if (buffer.used) + continue; + buffer.used = true; + return Ext2FS::BlockBufferWrapper { + buffer.buffer.span(), + [](void* self, const uint8_t* buffer) { static_cast(self)->destroy_callback(buffer); }, + this + }; + } + + ASSERT_NOT_REACHED(); } BAN::ErrorOr Ext2FS::BlockBufferManager::initialize(size_t block_size)