diff --git a/kernel/include/kernel/FS/Inode.h b/kernel/include/kernel/FS/Inode.h index 9ba4088f..64d135fa 100644 --- a/kernel/include/kernel/FS/Inode.h +++ b/kernel/include/kernel/FS/Inode.h @@ -182,7 +182,7 @@ namespace Kernel private: BAN::WeakPtr m_shared_region; - Mutex m_epoll_mutex; + SpinLock m_epoll_lock; BAN::LinkedList m_epolls; friend class Epoll; friend class FileBackedRegion; diff --git a/kernel/include/kernel/Lock/LockGuard.h b/kernel/include/kernel/Lock/LockGuard.h index b5a5b591..e925d17e 100644 --- a/kernel/include/kernel/Lock/LockGuard.h +++ b/kernel/include/kernel/Lock/LockGuard.h @@ -29,30 +29,4 @@ namespace Kernel Lock& m_lock; }; - template - class LockFreeGuard - { - BAN_NON_COPYABLE(LockFreeGuard); - BAN_NON_MOVABLE(LockFreeGuard); - - public: - LockFreeGuard(Lock& lock) - : m_lock(lock) - , m_depth(lock.lock_depth()) - { - for (uint32_t i = 0; i < m_depth; i++) - m_lock.unlock(); - } - - ~LockFreeGuard() - { - for (uint32_t i = 0; i < m_depth; i++) - m_lock.lock(); - } - - private: - Lock& m_lock; - const uint32_t m_depth; - }; - } diff --git a/kernel/include/kernel/Lock/Mutex.h b/kernel/include/kernel/Lock/Mutex.h index a3aae89f..c199f805 100644 --- a/kernel/include/kernel/Lock/Mutex.h +++ b/kernel/include/kernel/Lock/Mutex.h @@ -9,7 +9,19 @@ namespace Kernel { - class Mutex + class BaseMutex + { + public: + virtual void lock() = 0; + virtual bool try_lock() = 0; + virtual void unlock() = 0; + + virtual pid_t locker() const = 0; + virtual bool is_locked() const = 0; + virtual uint32_t lock_depth() const = 0; + }; + + class Mutex : public BaseMutex { BAN_NON_COPYABLE(Mutex); BAN_NON_MOVABLE(Mutex); @@ -17,9 +29,10 @@ namespace Kernel public: Mutex() = default; - void lock() + void lock() override { const auto tid = Thread::current_tid(); + ASSERT(!tid || !Thread::current().has_spinlock()); if (tid == m_locker) ASSERT(m_lock_depth > 0); else @@ -37,9 +50,10 @@ namespace Kernel m_lock_depth++; } - bool try_lock() + bool try_lock() override { const auto tid = Thread::current_tid(); + ASSERT(!tid || !Thread::current().has_spinlock()); if (tid == m_locker) ASSERT(m_lock_depth > 0); else @@ -55,7 +69,7 @@ namespace Kernel return true; } - void unlock() + void unlock() override { const auto tid = Thread::current_tid(); ASSERT(m_locker == tid); @@ -68,16 +82,16 @@ namespace Kernel } } - pid_t locker() const { return m_locker; } - bool is_locked() const { return m_locker != -1; } - uint32_t lock_depth() const { return m_lock_depth; } + pid_t locker() const override { return m_locker; } + bool is_locked() const override { return m_locker != -1; } + uint32_t lock_depth() const override { return m_lock_depth; } private: BAN::Atomic m_locker { -1 }; uint32_t m_lock_depth { 0 }; }; - class PriorityMutex + class PriorityMutex : public BaseMutex { BAN_NON_COPYABLE(PriorityMutex); BAN_NON_MOVABLE(PriorityMutex); @@ -85,7 +99,7 @@ namespace Kernel public: PriorityMutex() = default; - void lock() + void lock() override { const auto tid = Thread::current_tid(); ASSERT(!tid || !Thread::current().has_spinlock()); @@ -110,7 +124,7 @@ namespace Kernel m_lock_depth++; } - bool try_lock() + bool try_lock() override { const auto tid = Thread::current_tid(); ASSERT(!tid || !Thread::current().has_spinlock()); @@ -133,7 +147,7 @@ namespace Kernel return true; } - void unlock() + void unlock() override { const auto tid = Thread::current_tid(); ASSERT(m_locker == tid); @@ -149,9 +163,9 @@ namespace Kernel } } - pid_t locker() const { return m_locker; } - bool is_locked() const { return m_locker != -1; } - uint32_t lock_depth() const { return m_lock_depth; } + pid_t locker() const override { return m_locker; } + bool is_locked() const override { return m_locker != -1; } + uint32_t lock_depth() const override { return m_lock_depth; } private: BAN::Atomic m_locker { -1 }; diff --git a/kernel/include/kernel/Lock/SpinLock.h b/kernel/include/kernel/Lock/SpinLock.h index a10d4338..91417fe0 100644 --- a/kernel/include/kernel/Lock/SpinLock.h +++ b/kernel/include/kernel/Lock/SpinLock.h @@ -24,6 +24,8 @@ namespace Kernel void unlock(InterruptState state); + uint32_t lock_depth() const { return current_processor_has_lock(); } + bool current_processor_has_lock() const { return m_locker.load(BAN::MemoryOrder::memory_order_relaxed) == Processor::current_id().as_u32(); @@ -72,6 +74,8 @@ namespace Kernel Processor::set_interrupt_state(state); } + uint32_t lock_depth() const { return m_lock_depth; } + bool current_processor_has_lock() const { return m_locker.load(BAN::MemoryOrder::memory_order_relaxed) == Processor::current_id().as_u32(); @@ -82,6 +86,9 @@ namespace Kernel uint32_t m_lock_depth { 0 }; }; + template + class SpinLockGuardAsMutex; + template class SpinLockGuard { @@ -103,6 +110,7 @@ namespace Kernel private: Lock& m_lock; InterruptState m_state; + friend class SpinLockGuardAsMutex; }; } diff --git a/kernel/include/kernel/Memory/MemoryRegion.h b/kernel/include/kernel/Memory/MemoryRegion.h index 32fc282c..bddf04eb 100644 --- a/kernel/include/kernel/Memory/MemoryRegion.h +++ b/kernel/include/kernel/Memory/MemoryRegion.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -41,9 +42,9 @@ namespace Kernel size_t virtual_page_count() const { return BAN::Math::div_round_up(m_size, PAGE_SIZE); } size_t physical_page_count() const { return m_physical_page_count; } - void pin() { m_pinned_count++; } - void unpin() { if (--m_pinned_count == 0) m_pinned_blocker.unblock(); } - void wait_not_pinned() { while (m_pinned_count) m_pinned_blocker.block_with_timeout_ms(100); } + void pin(); + void unpin(); + void wait_not_pinned(); virtual BAN::ErrorOr msync(vaddr_t, size_t, int) = 0; @@ -68,6 +69,7 @@ namespace Kernel vaddr_t m_vaddr { 0 }; size_t m_physical_page_count { 0 }; + Mutex m_pinned_mutex; BAN::Atomic m_pinned_count { 0 }; ThreadBlocker m_pinned_blocker; }; diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index cbd1d788..3d8e6cdc 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -331,7 +331,6 @@ namespace Kernel bool m_is_userspace { false }; - SpinLock m_child_exit_lock; BAN::Vector m_child_exit_statuses; ThreadBlocker m_child_exit_blocker; diff --git a/kernel/include/kernel/Scheduler.h b/kernel/include/kernel/Scheduler.h index a791a0a4..ada6ae44 100644 --- a/kernel/include/kernel/Scheduler.h +++ b/kernel/include/kernel/Scheduler.h @@ -11,6 +11,7 @@ namespace Kernel { + class BaseMutex; class Thread; class ThreadBlocker; @@ -86,7 +87,7 @@ namespace Kernel // if thread is already bound, this will never fail BAN::ErrorOr add_thread(Thread*); - void block_current_thread(ThreadBlocker* thread_blocker, uint64_t wake_time_ns); + void block_current_thread(ThreadBlocker* thread_blocker, uint64_t wake_time_ns, BaseMutex* mutex); void unblock_thread(Thread*); Thread& current_thread(); diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 4bb081dc..3c509b6c 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -58,13 +58,27 @@ namespace Kernel bool add_signal(int signal); // blocks current thread and returns either on unblock, eintr, spuriously or after timeout - BAN::ErrorOr sleep_or_eintr_ms(uint64_t ms) { ASSERT(!BAN::Math::will_multiplication_overflow(ms, 1'000'000)); return sleep_or_eintr_ns(ms * 1'000'000); } + // if mutex is not nullptr, it will be atomically freed before blocking and automatically locked on wake BAN::ErrorOr sleep_or_eintr_ns(uint64_t ns); - BAN::ErrorOr block_or_eintr_indefinite(ThreadBlocker& thread_blocker); - BAN::ErrorOr block_or_eintr_or_timeout_ms(ThreadBlocker& thread_blocker, uint64_t timeout_ms, bool etimedout) { ASSERT(!BAN::Math::will_multiplication_overflow(timeout_ms, 1'000'000)); return block_or_eintr_or_timeout_ns(thread_blocker, timeout_ms * 1'000'000, etimedout); } - BAN::ErrorOr block_or_eintr_or_waketime_ms(ThreadBlocker& thread_blocker, uint64_t wake_time_ms, bool etimedout) { ASSERT(!BAN::Math::will_multiplication_overflow(wake_time_ms, 1'000'000)); return block_or_eintr_or_waketime_ns(thread_blocker, wake_time_ms * 1'000'000, etimedout); } - BAN::ErrorOr block_or_eintr_or_timeout_ns(ThreadBlocker& thread_blocker, uint64_t timeout_ns, bool etimedout); - BAN::ErrorOr block_or_eintr_or_waketime_ns(ThreadBlocker& thread_blocker, uint64_t wake_time_ns, bool etimedout); + BAN::ErrorOr block_or_eintr_indefinite(ThreadBlocker& thread_blocker, BaseMutex* mutex); + BAN::ErrorOr block_or_eintr_or_timeout_ns(ThreadBlocker& thread_blocker, uint64_t timeout_ns, bool etimedout, BaseMutex* mutex); + BAN::ErrorOr block_or_eintr_or_waketime_ns(ThreadBlocker& thread_blocker, uint64_t wake_time_ns, bool etimedout, BaseMutex* mutex); + + BAN::ErrorOr sleep_or_eintr_ms(uint64_t ms) + { + ASSERT(!BAN::Math::will_multiplication_overflow(ms, 1'000'000)); + return sleep_or_eintr_ns(ms * 1'000'000); + } + BAN::ErrorOr block_or_eintr_or_timeout_ms(ThreadBlocker& thread_blocker, uint64_t timeout_ms, bool etimedout, BaseMutex* mutex) + { + ASSERT(!BAN::Math::will_multiplication_overflow(timeout_ms, 1'000'000)); + return block_or_eintr_or_timeout_ns(thread_blocker, timeout_ms * 1'000'000, etimedout, mutex); + } + BAN::ErrorOr block_or_eintr_or_waketime_ms(ThreadBlocker& thread_blocker, uint64_t wake_time_ms, bool etimedout, BaseMutex* mutex) + { + ASSERT(!BAN::Math::will_multiplication_overflow(wake_time_ms, 1'000'000)); + return block_or_eintr_or_waketime_ns(thread_blocker, wake_time_ms * 1'000'000, etimedout, mutex); + } pid_t tid() const { return m_tid; } diff --git a/kernel/include/kernel/ThreadBlocker.h b/kernel/include/kernel/ThreadBlocker.h index 8c8cc2d3..e19f9f0e 100644 --- a/kernel/include/kernel/ThreadBlocker.h +++ b/kernel/include/kernel/ThreadBlocker.h @@ -10,13 +10,23 @@ namespace Kernel class ThreadBlocker { public: - void block_indefinite(); - void block_with_timeout_ms(uint64_t timeout_ms) { ASSERT(!BAN::Math::will_multiplication_overflow(timeout_ms, 1'000'000)); return block_with_timeout_ns(timeout_ms * 1'000'000); } - void block_with_wake_time_ms(uint64_t wake_time_ms) { ASSERT(!BAN::Math::will_multiplication_overflow(wake_time_ms, 1'000'000)); return block_with_wake_time_ns(wake_time_ms * 1'000'000); } - void block_with_timeout_ns(uint64_t timeout_ns); - void block_with_wake_time_ns(uint64_t wake_time_ns); + void block_indefinite(BaseMutex*); + void block_with_timeout_ns(uint64_t timeout_ns, BaseMutex*); + void block_with_wake_time_ns(uint64_t wake_time_ns, BaseMutex*); void unblock(); + + void block_with_timeout_ms(uint64_t timeout_ms, BaseMutex* mutex) + { + ASSERT(!BAN::Math::will_multiplication_overflow(timeout_ms, 1'000'000)); + return block_with_timeout_ns(timeout_ms * 1'000'000, mutex); + } + void block_with_wake_time_ms(uint64_t wake_time_ms, BaseMutex* mutex) + { + ASSERT(!BAN::Math::will_multiplication_overflow(wake_time_ms, 1'000'000)); + return block_with_wake_time_ns(wake_time_ms * 1'000'000, mutex); + } + private: void add_thread_to_block_queue(SchedulerQueue::Node*); void remove_blocked_thread(SchedulerQueue::Node*); diff --git a/kernel/kernel/ACPI/ACPI.cpp b/kernel/kernel/ACPI/ACPI.cpp index 74e6aa1e..d9e20ff5 100644 --- a/kernel/kernel/ACPI/ACPI.cpp +++ b/kernel/kernel/ACPI/ACPI.cpp @@ -963,7 +963,7 @@ acpi_release_global_lock: // FIXME: this can cause missing of event if it happens between // reading the status and blocking - m_event_thread_blocker.block_with_timeout_ms(100); + m_event_thread_blocker.block_with_timeout_ms(100, nullptr); continue; handle_event: diff --git a/kernel/kernel/Epoll.cpp b/kernel/kernel/Epoll.cpp index 7bab70e5..5e1e98f4 100644 --- a/kernel/kernel/Epoll.cpp +++ b/kernel/kernel/Epoll.cpp @@ -1,5 +1,6 @@ #include #include +#include #include namespace Kernel @@ -45,10 +46,12 @@ namespace Kernel TRY(inode->add_epoll(this)); it->value.add_fd(fd, event); - auto processing_it = m_processing_events.find(inode); - if (processing_it == m_processing_events.end()) - processing_it = MUST(m_processing_events.insert(inode, 0)); - processing_it->value |= event.events; + SpinLockGuard _(m_ready_lock); + auto ready_it = m_ready_events.find(inode); + if (ready_it == m_ready_events.end()) + ready_it = MUST(m_ready_events.insert(inode, 0)); + ready_it->value |= event.events; + m_thread_blocker.unblock(); return {}; } @@ -61,10 +64,12 @@ namespace Kernel it->value.events[fd] = event; - auto processing_it = m_processing_events.find(inode); - if (processing_it == m_processing_events.end()) - processing_it = MUST(m_processing_events.insert(inode, 0)); - processing_it->value |= event.events; + SpinLockGuard _(m_ready_lock); + auto ready_it = m_ready_events.find(inode); + if (ready_it == m_ready_events.end()) + ready_it = MUST(m_ready_events.insert(inode, 0)); + ready_it->value |= event.events; + m_thread_blocker.unblock(); return {}; } @@ -196,8 +201,14 @@ namespace Kernel const uint64_t current_ns = SystemTimer::get().ns_since_boot(); if (current_ns >= waketime_ns) break; + + SpinLockGuard guard(m_ready_lock); + if (!m_ready_events.empty()) + continue; + + SpinLockGuardAsMutex smutex(guard); const uint64_t timeout_ns = BAN::Math::min(100'000'000, waketime_ns - current_ns); - TRY(Thread::current().block_or_eintr_or_timeout_ns(m_thread_blocker, timeout_ns, false)); + TRY(Thread::current().block_or_eintr_or_timeout_ns(m_thread_blocker, timeout_ns, false, &smutex)); } return event_count; diff --git a/kernel/kernel/FS/DevFS/FileSystem.cpp b/kernel/kernel/FS/DevFS/FileSystem.cpp index 3f7ae92d..fd6e298b 100644 --- a/kernel/kernel/FS/DevFS/FileSystem.cpp +++ b/kernel/kernel/FS/DevFS/FileSystem.cpp @@ -46,54 +46,54 @@ namespace Kernel void DevFileSystem::initialize_device_updater() { Process::create_kernel( - [](void*) + [](void* _devfs) { + auto* devfs = static_cast(_devfs); while (true) { { - LockGuard _(s_instance->m_device_lock); - for (auto& device : s_instance->m_devices) + LockGuard _(devfs->m_device_lock); + for (auto& device : devfs->m_devices) device->update(); } SystemTimer::get().sleep_ms(10); } - }, nullptr + }, s_instance ); auto* sync_process = Process::create_kernel(); sync_process->add_thread(MUST(Thread::create_kernel( - [](void*) + [](void* _devfs) { + auto* devfs = static_cast(_devfs); while (true) { - LockGuard _(s_instance->m_device_lock); - while (!s_instance->m_should_sync) - { - LockFreeGuard _(s_instance->m_device_lock); - s_instance->m_sync_thread_blocker.block_indefinite(); - } + LockGuard _(devfs->m_device_lock); + while (!devfs->m_should_sync) + devfs->m_sync_thread_blocker.block_indefinite(&devfs->m_device_lock); - for (auto& device : s_instance->m_devices) + for (auto& device : devfs->m_devices) if (device->is_storage_device()) if (auto ret = static_cast(device.ptr())->sync_disk_cache(); ret.is_error()) dwarnln("disk sync: {}", ret.error()); - s_instance->m_should_sync = false; - s_instance->m_sync_done.unblock(); + devfs->m_should_sync = false; + devfs->m_sync_done.unblock(); } - }, nullptr, sync_process + }, s_instance, sync_process ))); sync_process->add_thread(MUST(Kernel::Thread::create_kernel( - [](void*) + [](void* _devfs) { + auto* devfs = static_cast(_devfs); while (true) { SystemTimer::get().sleep_ms(10'000); - s_instance->initiate_sync(false); + devfs->initiate_sync(false); } - }, nullptr, sync_process + }, s_instance, sync_process ))); sync_process->register_to_scheduler(); @@ -101,13 +101,11 @@ namespace Kernel void DevFileSystem::initiate_sync(bool should_block) { - { - LockGuard _(m_device_lock); - m_should_sync = true; - m_sync_thread_blocker.unblock(); - } - if (should_block) - m_sync_done.block_indefinite(); + LockGuard _(m_device_lock); + m_should_sync = true; + m_sync_thread_blocker.unblock(); + while (should_block && m_should_sync) + m_sync_done.block_indefinite(&m_device_lock); } void DevFileSystem::add_device(BAN::RefPtr device) diff --git a/kernel/kernel/FS/Inode.cpp b/kernel/kernel/FS/Inode.cpp index c26be539..8118b1b3 100644 --- a/kernel/kernel/FS/Inode.cpp +++ b/kernel/kernel/FS/Inode.cpp @@ -278,14 +278,14 @@ namespace Kernel BAN::ErrorOr Inode::add_epoll(class Epoll* epoll) { - LockGuard _(m_epoll_mutex); + SpinLockGuard _(m_epoll_lock); TRY(m_epolls.push_back(epoll)); return {}; } void Inode::del_epoll(class Epoll* epoll) { - LockGuard _(m_epoll_mutex); + SpinLockGuard _(m_epoll_lock); for (auto it = m_epolls.begin(); it != m_epolls.end(); it++) { if (*it != epoll) @@ -297,7 +297,7 @@ namespace Kernel void Inode::epoll_notify(uint32_t event) { - LockGuard _(m_epoll_mutex); + SpinLockGuard _(m_epoll_lock); for (auto* epoll : m_epolls) epoll->notify(this, event); } diff --git a/kernel/kernel/FS/Pipe.cpp b/kernel/kernel/FS/Pipe.cpp index 2eddadb9..d261e2b8 100644 --- a/kernel/kernel/FS/Pipe.cpp +++ b/kernel/kernel/FS/Pipe.cpp @@ -44,6 +44,8 @@ namespace Kernel void Pipe::on_close(int status_flags) { + LockGuard _(m_mutex); + if (status_flags & O_WRONLY) { auto old_writing_count = m_writing_count.fetch_sub(1); @@ -71,8 +73,7 @@ namespace Kernel { if (m_writing_count == 0) return 0; - LockFreeGuard lock_free(m_mutex); - TRY(Thread::current().block_or_eintr_or_timeout_ms(m_thread_blocker, 100, false)); + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); } const size_t to_copy = BAN::Math::min(buffer.size(), m_buffer_size); @@ -108,8 +109,7 @@ namespace Kernel Thread::current().add_signal(SIGPIPE); return BAN::Error::from_errno(EPIPE); } - LockFreeGuard lock_free(m_mutex); - TRY(Thread::current().block_or_eintr_or_timeout_ms(m_thread_blocker, 100, false)); + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); } const size_t to_copy = BAN::Math::min(buffer.size(), m_buffer.size() - m_buffer_size); diff --git a/kernel/kernel/FS/ProcFS/FileSystem.cpp b/kernel/kernel/FS/ProcFS/FileSystem.cpp index 01a3ae2d..0a1d376c 100644 --- a/kernel/kernel/FS/ProcFS/FileSystem.cpp +++ b/kernel/kernel/FS/ProcFS/FileSystem.cpp @@ -1,6 +1,5 @@ #include #include -#include namespace Kernel { diff --git a/kernel/kernel/Input/InputDevice.cpp b/kernel/kernel/Input/InputDevice.cpp index 39c9fd27..11380542 100644 --- a/kernel/kernel/Input/InputDevice.cpp +++ b/kernel/kernel/Input/InputDevice.cpp @@ -1,7 +1,7 @@ #include #include #include -#include +#include #include #include @@ -181,23 +181,18 @@ namespace Kernel if (buffer.size() < m_event_size) return BAN::Error::from_errno(ENOBUFS); - auto state = m_event_lock.lock(); + SpinLockGuard guard(m_event_lock); while (m_event_count == 0) { - m_event_lock.unlock(state); - { - LockFreeGuard _(m_mutex); - TRY(Thread::current().block_or_eintr_indefinite(m_event_thread_blocker)); - } - state = m_event_lock.lock(); + // FIXME: should m_mutex be unlocked? + SpinLockGuardAsMutex smutex(guard); + TRY(Thread::current().block_or_eintr_indefinite(m_event_thread_blocker, &smutex)); } memcpy(buffer.data(), &m_event_buffer[m_event_tail * m_event_size], m_event_size); m_event_tail = (m_event_tail + 1) % m_max_event_count; m_event_count--; - m_event_lock.unlock(state); - return m_event_size; } @@ -256,8 +251,8 @@ namespace Kernel return bytes; } - LockFreeGuard _(m_mutex); - TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker)); + // FIXME: race condition as notify doesn't lock mutex + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); } } @@ -308,8 +303,8 @@ namespace Kernel return bytes; } - LockFreeGuard _(m_mutex); - TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker)); + // FIXME: race condition as notify doesn't lock mutex + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); } } diff --git a/kernel/kernel/Memory/MemoryBackedRegion.cpp b/kernel/kernel/Memory/MemoryBackedRegion.cpp index 47e0bdf9..8723b970 100644 --- a/kernel/kernel/Memory/MemoryBackedRegion.cpp +++ b/kernel/kernel/Memory/MemoryBackedRegion.cpp @@ -1,4 +1,3 @@ -#include #include #include diff --git a/kernel/kernel/Memory/MemoryRegion.cpp b/kernel/kernel/Memory/MemoryRegion.cpp index cb334217..f0e11d0d 100644 --- a/kernel/kernel/Memory/MemoryRegion.cpp +++ b/kernel/kernel/Memory/MemoryRegion.cpp @@ -1,3 +1,4 @@ +#include #include namespace Kernel @@ -59,4 +60,24 @@ namespace Kernel return ret; } + void MemoryRegion::pin() + { + LockGuard _(m_pinned_mutex); + m_pinned_count++; + } + + void MemoryRegion::unpin() + { + LockGuard _(m_pinned_mutex); + if (--m_pinned_count == 0) + m_pinned_blocker.unblock(); + } + + void MemoryRegion::wait_not_pinned() + { + LockGuard _(m_pinned_mutex); + while (m_pinned_count) + m_pinned_blocker.block_with_timeout_ms(100, &m_pinned_mutex); + } + } diff --git a/kernel/kernel/Memory/VirtualRange.cpp b/kernel/kernel/Memory/VirtualRange.cpp index 32dae5a9..ea821d2e 100644 --- a/kernel/kernel/Memory/VirtualRange.cpp +++ b/kernel/kernel/Memory/VirtualRange.cpp @@ -1,4 +1,3 @@ -#include #include #include diff --git a/kernel/kernel/Networking/ARPTable.cpp b/kernel/kernel/Networking/ARPTable.cpp index 901108bf..237e56ef 100644 --- a/kernel/kernel/Networking/ARPTable.cpp +++ b/kernel/kernel/Networking/ARPTable.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -158,16 +159,15 @@ namespace Kernel for (;;) { PendingArpPacket pending = ({ - auto state = m_pending_lock.lock(); + SpinLockGuard guard(m_pending_lock); while (m_pending_packets.empty()) { - m_pending_lock.unlock(state); - m_pending_thread_blocker.block_with_timeout_ms(100); - state = m_pending_lock.lock(); + SpinLockGuardAsMutex smutex(guard); + m_pending_thread_blocker.block_indefinite(&smutex); } + auto packet = m_pending_packets.front(); m_pending_packets.pop(); - m_pending_lock.unlock(state); packet; }); diff --git a/kernel/kernel/Networking/IPv4Layer.cpp b/kernel/kernel/Networking/IPv4Layer.cpp index a347e51c..94768a8f 100644 --- a/kernel/kernel/Networking/IPv4Layer.cpp +++ b/kernel/kernel/Networking/IPv4Layer.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -331,16 +332,15 @@ namespace Kernel for (;;) { PendingIPv4Packet pending = ({ - auto state = m_pending_lock.lock(); + SpinLockGuard guard(m_pending_lock); while (m_pending_packets.empty()) { - m_pending_lock.unlock(state); - m_pending_thread_blocker.block_with_timeout_ms(100); - state = m_pending_lock.lock(); + SpinLockGuardAsMutex smutex(guard); + m_pending_thread_blocker.block_indefinite(&smutex); } + auto packet = m_pending_packets.front(); m_pending_packets.pop(); - m_pending_lock.unlock(state); packet; }); diff --git a/kernel/kernel/Networking/RTL8169/RTL8169.cpp b/kernel/kernel/Networking/RTL8169/RTL8169.cpp index 5fd47e79..14fedddf 100644 --- a/kernel/kernel/Networking/RTL8169/RTL8169.cpp +++ b/kernel/kernel/Networking/RTL8169/RTL8169.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -205,13 +206,18 @@ namespace Kernel return BAN::Error::from_errno(EADDRNOTAVAIL); auto state = m_lock.lock(); + const uint32_t tx_current = m_tx_current; m_tx_current = (m_tx_current + 1) % m_tx_descriptor_count; - m_lock.unlock(state); auto& descriptor = reinterpret_cast(m_tx_descriptor_region->vaddr())[tx_current]; while (descriptor.command & RTL8169_DESC_CMD_OWN) - m_thread_blocker.block_with_timeout_ms(100); + { + SpinLockAsMutex smutex(m_lock, state); + m_thread_blocker.block_indefinite(&smutex); + } + + m_lock.unlock(state); auto* tx_buffer = reinterpret_cast(m_tx_buffer_region->vaddr() + tx_current * buffer_size); @@ -246,7 +252,10 @@ namespace Kernel } if (interrupt_status & RTL8169_IR_TOK) + { + SpinLockGuard _(m_lock); m_thread_blocker.unblock(); + } if (interrupt_status & RTL8169_IR_RER) dwarnln("Rx error"); diff --git a/kernel/kernel/Networking/TCPSocket.cpp b/kernel/kernel/Networking/TCPSocket.cpp index e9cceafb..a424bc72 100644 --- a/kernel/kernel/Networking/TCPSocket.cpp +++ b/kernel/kernel/Networking/TCPSocket.cpp @@ -73,10 +73,7 @@ namespace Kernel return BAN::Error::from_errno(EINVAL); while (m_pending_connections.empty()) - { - LockFreeGuard _(m_mutex); - TRY(Thread::current().block_or_eintr_or_timeout_ms(m_thread_blocker, 100, false)); - } + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); auto connection = m_pending_connections.front(); m_pending_connections.pop(); @@ -111,12 +108,7 @@ namespace Kernel const uint64_t wake_time_ms = SystemTimer::get().ms_since_boot() + 5000; while (!return_inode->m_has_connected) - { - if (SystemTimer::get().ms_since_boot() >= wake_time_ms) - return BAN::Error::from_errno(ECONNABORTED); - LockFreeGuard free(m_mutex); - TRY(Thread::current().block_or_eintr_or_waketime_ms(return_inode->m_thread_blocker, wake_time_ms, true)); - } + TRY(Thread::current().block_or_eintr_or_waketime_ms(return_inode->m_thread_blocker, wake_time_ms, true, &m_mutex)); if (address) { @@ -168,12 +160,7 @@ namespace Kernel const uint64_t wake_time_ms = SystemTimer::get().ms_since_boot() + 5000; while (!m_has_connected) - { - if (SystemTimer::get().ms_since_boot() >= wake_time_ms) - return BAN::Error::from_errno(ECONNREFUSED); - LockFreeGuard free(m_mutex); - TRY(Thread::current().block_or_eintr_or_waketime_ms(m_thread_blocker, wake_time_ms, true)); - } + TRY(Thread::current().block_or_eintr_or_waketime_ms(m_thread_blocker, wake_time_ms, true, &m_mutex)); return {}; } @@ -208,8 +195,7 @@ namespace Kernel { if (m_state != State::Established) return return_with_maybe_zero(); - LockFreeGuard free(m_mutex); - TRY(Thread::current().block_or_eintr_or_timeout_ms(m_thread_blocker, 100, false)); + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); } const uint32_t to_recv = BAN::Math::min(buffer.size(), m_recv_window.data_size); @@ -239,8 +225,7 @@ namespace Kernel { if (m_state != State::Established) return return_with_maybe_zero(); - LockFreeGuard free(m_mutex); - TRY(Thread::current().block_or_eintr_or_timeout_ms(m_thread_blocker, 100, false)); + TRY(Thread::current().block_or_eintr_indefinite(m_thread_blocker, &m_mutex)); } const size_t to_send = BAN::Math::min(message.size(), m_send_window.buffer->size() - m_send_window.data_size); @@ -519,8 +504,10 @@ namespace Kernel } auto socket = it->value; - LockFreeGuard _(m_mutex); + m_mutex.unlock(); socket->receive_packet(buffer, sender, sender_len); + m_mutex.lock(); + return; } break; @@ -660,116 +647,114 @@ namespace Kernel BAN::RefPtr keep_alive { this }; this->unref(); + LockGuard _(m_mutex); + while (m_process) { const uint64_t current_ms = SystemTimer::get().ms_since_boot(); + if (m_state == State::TimeWait && current_ms >= m_time_wait_start_ms + 30'000) { - LockGuard _(m_mutex); + set_connection_as_closed(); + continue; + } - if (m_state == State::TimeWait && current_ms >= m_time_wait_start_ms + 30'000) + // This is the last instance + if (ref_count() == 1) + { + if (m_state == State::Listen) { set_connection_as_closed(); continue; } - - // This is the last instance - if (ref_count() == 1) + if (m_state == State::Established) { - if (m_state == State::Listen) - { - set_connection_as_closed(); - continue; - } - if (m_state == State::Established) - { - m_next_flags = FIN | ACK; - m_next_state = State::FinWait1; - } - } - - if (m_next_flags) - { - ASSERT(m_connection_info.has_value()); - auto* target_address = reinterpret_cast(&m_connection_info->address); - auto target_address_len = m_connection_info->address_len; - if (auto ret = m_network_layer.sendto(*this, {}, target_address, target_address_len); ret.is_error()) - dwarnln("{}", ret.error()); - const bool hungup_before = has_hungup_impl(); - m_state = m_next_state; - if (m_state == State::Established) - m_has_connected = true; - if (!hungup_before && has_hungup_impl()) - epoll_notify(EPOLLHUP); - continue; - } - - if (m_send_window.data_size > 0 && m_send_window.current_ack - m_send_window.has_ghost_byte > m_send_window.start_seq) - { - uint32_t acknowledged_bytes = m_send_window.current_ack - m_send_window.start_seq - m_send_window.has_ghost_byte; - ASSERT(acknowledged_bytes <= m_send_window.data_size); - - m_send_window.data_size -= acknowledged_bytes; - m_send_window.start_seq += acknowledged_bytes; - - if (m_send_window.data_size > 0) - { - auto* send_buffer = reinterpret_cast(m_send_window.buffer->vaddr()); - memmove(send_buffer, send_buffer + acknowledged_bytes, m_send_window.data_size); - } - - m_send_window.sent_size -= acknowledged_bytes; - - epoll_notify(EPOLLOUT); - - dprintln_if(DEBUG_TCP, "Target acknowledged {} bytes", acknowledged_bytes); - - continue; - } - - const bool should_retransmit = m_send_window.data_size > 0 && current_ms >= m_send_window.last_send_ms + retransmit_timeout_ms; - - if (m_send_window.data_size > m_send_window.sent_size || should_retransmit) - { - ASSERT(m_connection_info.has_value()); - auto* target_address = reinterpret_cast(&m_connection_info->address); - auto target_address_len = m_connection_info->address_len; - - const uint32_t send_base = should_retransmit ? 0 : m_send_window.sent_size; - - const uint32_t total_send = BAN::Math::min(m_send_window.data_size - send_base, m_send_window.scaled_size()); - - m_send_window.current_seq = m_send_window.start_seq; - - auto* send_buffer = reinterpret_cast(m_send_window.buffer->vaddr() + send_base); - for (uint32_t i = 0; i < total_send;) - { - const uint32_t to_send = BAN::Math::min(total_send - i, m_send_window.mss); - - auto message = BAN::ConstByteSpan(send_buffer + i, to_send); - - m_next_flags = ACK; - if (auto ret = m_network_layer.sendto(*this, message, target_address, target_address_len); ret.is_error()) - { - dwarnln("{}", ret.error()); - break; - } - - dprintln_if(DEBUG_TCP, "Sent {} bytes", to_send); - - m_send_window.sent_size += to_send; - m_send_window.current_seq += to_send; - i += to_send; - } - - m_send_window.last_send_ms = current_ms; - - continue; + m_next_flags = FIN | ACK; + m_next_state = State::FinWait1; } } + if (m_next_flags) + { + ASSERT(m_connection_info.has_value()); + auto* target_address = reinterpret_cast(&m_connection_info->address); + auto target_address_len = m_connection_info->address_len; + if (auto ret = m_network_layer.sendto(*this, {}, target_address, target_address_len); ret.is_error()) + dwarnln("{}", ret.error()); + const bool hungup_before = has_hungup_impl(); + m_state = m_next_state; + if (m_state == State::Established) + m_has_connected = true; + if (!hungup_before && has_hungup_impl()) + epoll_notify(EPOLLHUP); + continue; + } + + if (m_send_window.data_size > 0 && m_send_window.current_ack - m_send_window.has_ghost_byte > m_send_window.start_seq) + { + uint32_t acknowledged_bytes = m_send_window.current_ack - m_send_window.start_seq - m_send_window.has_ghost_byte; + ASSERT(acknowledged_bytes <= m_send_window.data_size); + + m_send_window.data_size -= acknowledged_bytes; + m_send_window.start_seq += acknowledged_bytes; + + if (m_send_window.data_size > 0) + { + auto* send_buffer = reinterpret_cast(m_send_window.buffer->vaddr()); + memmove(send_buffer, send_buffer + acknowledged_bytes, m_send_window.data_size); + } + + m_send_window.sent_size -= acknowledged_bytes; + + epoll_notify(EPOLLOUT); + + dprintln_if(DEBUG_TCP, "Target acknowledged {} bytes", acknowledged_bytes); + + continue; + } + + const bool should_retransmit = m_send_window.data_size > 0 && current_ms >= m_send_window.last_send_ms + retransmit_timeout_ms; + + if (m_send_window.data_size > m_send_window.sent_size || should_retransmit) + { + ASSERT(m_connection_info.has_value()); + auto* target_address = reinterpret_cast(&m_connection_info->address); + auto target_address_len = m_connection_info->address_len; + + const uint32_t send_base = should_retransmit ? 0 : m_send_window.sent_size; + + const uint32_t total_send = BAN::Math::min(m_send_window.data_size - send_base, m_send_window.scaled_size()); + + m_send_window.current_seq = m_send_window.start_seq; + + auto* send_buffer = reinterpret_cast(m_send_window.buffer->vaddr() + send_base); + for (uint32_t i = 0; i < total_send;) + { + const uint32_t to_send = BAN::Math::min(total_send - i, m_send_window.mss); + + auto message = BAN::ConstByteSpan(send_buffer + i, to_send); + + m_next_flags = ACK; + if (auto ret = m_network_layer.sendto(*this, message, target_address, target_address_len); ret.is_error()) + { + dwarnln("{}", ret.error()); + break; + } + + dprintln_if(DEBUG_TCP, "Sent {} bytes", to_send); + + m_send_window.sent_size += to_send; + m_send_window.current_seq += to_send; + i += to_send; + } + + m_send_window.last_send_ms = current_ms; + + continue; + } + m_thread_blocker.unblock(); - m_thread_blocker.block_with_wake_time_ms(current_ms + retransmit_timeout_ms); + m_thread_blocker.block_with_wake_time_ms(current_ms + retransmit_timeout_ms, &m_mutex); } m_thread_blocker.unblock(); diff --git a/kernel/kernel/Networking/UDPSocket.cpp b/kernel/kernel/Networking/UDPSocket.cpp index 93e06912..03aa47d6 100644 --- a/kernel/kernel/Networking/UDPSocket.cpp +++ b/kernel/kernel/Networking/UDPSocket.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -93,12 +94,12 @@ namespace Kernel } ASSERT(m_port != PORT_NONE); - auto state = m_packet_lock.lock(); + SpinLockGuard guard(m_packet_lock); + while (m_packets.empty()) { - m_packet_lock.unlock(state); - TRY(Thread::current().block_or_eintr_indefinite(m_packet_thread_blocker)); - state = m_packet_lock.lock(); + SpinLockGuardAsMutex smutex(guard); + TRY(Thread::current().block_or_eintr_indefinite(m_packet_thread_blocker, &smutex)); } auto packet_info = m_packets.front(); @@ -120,8 +121,6 @@ namespace Kernel m_packet_total_size -= packet_info.packet_size; - m_packet_lock.unlock(state); - if (address && address_len) { if (*address_len > (socklen_t)sizeof(sockaddr_storage)) diff --git a/kernel/kernel/Networking/UNIX/Socket.cpp b/kernel/kernel/Networking/UNIX/Socket.cpp index 221911b6..30085c21 100644 --- a/kernel/kernel/Networking/UNIX/Socket.cpp +++ b/kernel/kernel/Networking/UNIX/Socket.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -16,6 +17,8 @@ namespace Kernel static constexpr size_t s_packet_buffer_size = 10 * PAGE_SIZE; + // FIXME: why is this using spinlocks instead of mutexes?? + BAN::ErrorOr> UnixDomainSocket::create(Socket::Type socket_type, const Socket::Info& info) { auto socket = TRY(BAN::RefPtr::create(socket_type, info)); @@ -91,13 +94,16 @@ namespace Kernel if (!connection_info.listening) return BAN::Error::from_errno(EINVAL); - while (connection_info.pending_connections.empty()) - TRY(Thread::current().block_or_eintr_indefinite(connection_info.pending_thread_blocker)); BAN::RefPtr pending; { - SpinLockGuard _(connection_info.pending_lock); + SpinLockGuard guard(connection_info.pending_lock); + + SpinLockGuardAsMutex smutex(guard); + while (connection_info.pending_connections.empty()) + TRY(Thread::current().block_or_eintr_indefinite(connection_info.pending_thread_blocker, &smutex)); + pending = connection_info.pending_connections.front(); connection_info.pending_connections.pop(); connection_info.pending_thread_blocker.unblock(); @@ -176,16 +182,18 @@ namespace Kernel for (;;) { auto& target_info = target->m_info.get(); + + SpinLockGuard guard(target_info.pending_lock); + + if (target_info.pending_connections.size() < target_info.pending_connections.capacity()) { - SpinLockGuard _(target_info.pending_lock); - if (target_info.pending_connections.size() < target_info.pending_connections.capacity()) - { - MUST(target_info.pending_connections.push(this)); - target_info.pending_thread_blocker.unblock(); - break; - } + MUST(target_info.pending_connections.push(this)); + target_info.pending_thread_blocker.unblock(); + break; } - TRY(Thread::current().block_or_eintr_indefinite(target_info.pending_thread_blocker)); + + SpinLockGuardAsMutex smutex(guard); + TRY(Thread::current().block_or_eintr_indefinite(target_info.pending_thread_blocker, &smutex)); } target->epoll_notify(EPOLLIN); @@ -269,9 +277,8 @@ namespace Kernel auto state = m_packet_lock.lock(); while (m_packet_sizes.full() || m_packet_size_total + packet.size() > s_packet_buffer_size) { - m_packet_lock.unlock(state); - TRY(Thread::current().block_or_eintr_indefinite(m_packet_thread_blocker)); - state = m_packet_lock.lock(); + SpinLockAsMutex smutex(m_packet_lock, state); + TRY(Thread::current().block_or_eintr_indefinite(m_packet_thread_blocker, &smutex)); } uint8_t* packet_buffer = reinterpret_cast(m_packet_buffer->vaddr() + m_packet_size_total); @@ -405,9 +412,8 @@ namespace Kernel } } - m_packet_lock.unlock(state); - TRY(Thread::current().block_or_eintr_indefinite(m_packet_thread_blocker)); - state = m_packet_lock.lock(); + SpinLockAsMutex smutex(m_packet_lock, state); + TRY(Thread::current().block_or_eintr_indefinite(m_packet_thread_blocker, &smutex)); } uint8_t* packet_buffer = reinterpret_cast(m_packet_buffer->vaddr()); diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 094cc8f2..71b640fe 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -279,6 +279,8 @@ namespace Kernel if (parent.pid() != m_parent) return BAN::Iteration::Continue; + LockGuard _(parent.m_process_lock); + for (auto& child : parent.m_child_exit_statuses) { if (child.pid != pid()) @@ -767,13 +769,13 @@ namespace Kernel return child.pid == pid; }; + LockGuard _(m_process_lock); + for (;;) { pid_t exited_pid = 0; int exit_code = 0; { - SpinLockGuard _(m_child_exit_lock); - bool found = false; for (auto& child : m_child_exit_statuses) { @@ -796,7 +798,6 @@ namespace Kernel { if (stat_loc) { - LockGuard _(m_process_lock); TRY(validate_pointer_access(stat_loc, sizeof(stat_loc), true)); *stat_loc = exit_code; } @@ -810,7 +811,7 @@ namespace Kernel if (options & WNOHANG) return 0; - m_child_exit_blocker.block_indefinite(); + m_child_exit_blocker.block_indefinite(&m_process_lock); } } @@ -2609,11 +2610,7 @@ namespace Kernel for (;;) { - { - LockFreeGuard _(m_process_lock); - m_pthread_exit_blocker.block_with_timeout_ms(100); - } - + TRY(Thread::current().block_or_eintr_indefinite(m_pthread_exit_blocker, &m_process_lock)); if (wait_thread()) return 0; } diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index 2e08e6e0..248a8539 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -599,7 +600,7 @@ namespace Kernel return {}; } - void Scheduler::block_current_thread(ThreadBlocker* blocker, uint64_t wake_time_ns) + void Scheduler::block_current_thread(ThreadBlocker* blocker, uint64_t wake_time_ns, BaseMutex* mutex) { auto state = Processor::get_interrupt_state(); Processor::set_interrupt_state(InterruptState::Disabled); @@ -612,9 +613,23 @@ namespace Kernel if (blocker) blocker->add_thread_to_block_queue(m_current); update_most_loaded_node_queue(m_current, &m_block_queue); + + uint32_t lock_depth = 0; + if (mutex != nullptr) + { + ASSERT(mutex->is_locked() && mutex->locker() == m_current->thread->tid()); + lock_depth = mutex->lock_depth(); + } + + for (uint32_t i = 0; i < lock_depth; i++) + mutex->unlock(); + Processor::yield(); Processor::set_interrupt_state(state); + + for (uint32_t i = 0; i < lock_depth; i++) + mutex->lock(); } void Scheduler::unblock_thread(Thread* thread) diff --git a/kernel/kernel/Storage/DiskCache.cpp b/kernel/kernel/Storage/DiskCache.cpp index df74b5a7..80b47536 100644 --- a/kernel/kernel/Storage/DiskCache.cpp +++ b/kernel/kernel/Storage/DiskCache.cpp @@ -1,5 +1,4 @@ #include -#include #include #include #include diff --git a/kernel/kernel/Storage/NVMe/Queue.cpp b/kernel/kernel/Storage/NVMe/Queue.cpp index 7241696f..e074d03b 100644 --- a/kernel/kernel/Storage/NVMe/Queue.cpp +++ b/kernel/kernel/Storage/NVMe/Queue.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include #include @@ -72,7 +72,7 @@ namespace Kernel // scheduler has put the current thread blocking. // EINTR should also be handled here. while (!(m_done_mask & cid_mask) && SystemTimer::get().ms_since_boot() < start_time_ms + s_nvme_command_timeout_ms) - m_thread_blocker.block_with_wake_time_ms(start_time_ms + s_nvme_command_timeout_ms); + m_thread_blocker.block_with_wake_time_ms(start_time_ms + s_nvme_command_timeout_ms, nullptr); if (m_done_mask & cid_mask) { @@ -87,12 +87,12 @@ namespace Kernel uint16_t NVMeQueue::reserve_cid() { - auto state = m_lock.lock(); + SpinLockGuard guard(m_lock); + while (~m_used_mask == 0) { - m_lock.unlock(state); - m_thread_blocker.block_with_timeout_ms(s_nvme_command_timeout_ms); - state = m_lock.lock(); + SpinLockGuardAsMutex smutex(guard); + m_thread_blocker.block_with_timeout_ms(s_nvme_command_timeout_ms, &smutex); } uint16_t cid = 0; @@ -104,7 +104,6 @@ namespace Kernel m_used_mask |= (size_t)1 << cid; - m_lock.unlock(state); return cid; } diff --git a/kernel/kernel/Terminal/PseudoTerminal.cpp b/kernel/kernel/Terminal/PseudoTerminal.cpp index 4cdd6a19..e6b62036 100644 --- a/kernel/kernel/Terminal/PseudoTerminal.cpp +++ b/kernel/kernel/Terminal/PseudoTerminal.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -88,17 +89,15 @@ namespace Kernel bool PseudoTerminalMaster::putchar(uint8_t ch) { - { - SpinLockGuard _(m_buffer_lock); + SpinLockGuard _(m_buffer_lock); - if (m_buffer_size >= m_buffer->size()) - return false; + if (m_buffer_size >= m_buffer->size()) + return false; - reinterpret_cast(m_buffer->vaddr())[(m_buffer_tail + m_buffer_size) % m_buffer->size()] = ch; - m_buffer_size++; + reinterpret_cast(m_buffer->vaddr())[(m_buffer_tail + m_buffer_size) % m_buffer->size()] = ch; + m_buffer_size++; - m_buffer_blocker.unblock(); - } + m_buffer_blocker.unblock(); epoll_notify(EPOLLIN); @@ -107,13 +106,12 @@ namespace Kernel BAN::ErrorOr PseudoTerminalMaster::read_impl(off_t, BAN::ByteSpan buffer) { - auto state = m_buffer_lock.lock(); + SpinLockGuard guard(m_buffer_lock); while (m_buffer_size == 0) { - m_buffer_lock.unlock(state); - TRY(Thread::current().block_or_eintr_indefinite(m_buffer_blocker)); - m_buffer_lock.lock(); + SpinLockGuardAsMutex smutex(guard); + TRY(Thread::current().block_or_eintr_indefinite(m_buffer_blocker, &smutex)); } const size_t to_copy = BAN::Math::min(buffer.size(), m_buffer_size); @@ -132,8 +130,6 @@ namespace Kernel m_buffer_size -= to_copy; m_buffer_tail = (m_buffer_tail + to_copy) % m_buffer->size(); - m_buffer_lock.unlock(state); - epoll_notify(EPOLLOUT); return to_copy; diff --git a/kernel/kernel/Terminal/TTY.cpp b/kernel/kernel/Terminal/TTY.cpp index 4589634a..c1c9290b 100644 --- a/kernel/kernel/Terminal/TTY.cpp +++ b/kernel/kernel/Terminal/TTY.cpp @@ -92,6 +92,8 @@ namespace Kernel if (flags & ~(TTY_FLAG_ENABLE_INPUT | TTY_FLAG_ENABLE_OUTPUT)) return BAN::Error::from_errno(EINVAL); + LockGuard _(m_mutex); + switch (command) { case TTY_CMD_SET: @@ -129,12 +131,16 @@ namespace Kernel while (true) { - while (!TTY::current()->m_tty_ctrl.receive_input) - TTY::current()->m_tty_ctrl.thread_blocker.block_indefinite(); + { + LockGuard _(TTY::current()->m_mutex); + while (!TTY::current()->m_tty_ctrl.receive_input) + TTY::current()->m_tty_ctrl.thread_blocker.block_indefinite(&TTY::current()->m_mutex); + } while (TTY::current()->m_tty_ctrl.receive_input) { LockGuard _(keyboard_inode->m_mutex); + if (!keyboard_inode->can_read()) { SystemTimer::get().sleep_ms(1); @@ -395,10 +401,7 @@ namespace Kernel BAN::ErrorOr TTY::read_impl(off_t, BAN::ByteSpan buffer) { while (!m_output.flush) - { - LockFreeGuard _(m_mutex); - TRY(Thread::current().block_or_eintr_indefinite(m_output.thread_blocker)); - } + TRY(Thread::current().block_or_eintr_indefinite(m_output.thread_blocker, &m_mutex)); if (m_output.bytes == 0) { diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index 5d7208c4..7763385e 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -581,27 +581,27 @@ namespace Kernel return {}; } - BAN::ErrorOr Thread::block_or_eintr_indefinite(ThreadBlocker& thread_blocker) + BAN::ErrorOr Thread::block_or_eintr_indefinite(ThreadBlocker& thread_blocker, BaseMutex* mutex) { if (is_interrupted_by_signal()) return BAN::Error::from_errno(EINTR); - thread_blocker.block_indefinite(); + thread_blocker.block_indefinite(mutex); if (is_interrupted_by_signal()) return BAN::Error::from_errno(EINTR); return {}; } - BAN::ErrorOr Thread::block_or_eintr_or_timeout_ns(ThreadBlocker& thread_blocker, uint64_t timeout_ns, bool etimedout) + BAN::ErrorOr Thread::block_or_eintr_or_timeout_ns(ThreadBlocker& thread_blocker, uint64_t timeout_ns, bool etimedout, BaseMutex* mutex) { const uint64_t wake_time_ns = SystemTimer::get().ns_since_boot() + timeout_ns; - return block_or_eintr_or_waketime_ns(thread_blocker, wake_time_ns, etimedout); + return block_or_eintr_or_waketime_ns(thread_blocker, wake_time_ns, etimedout, mutex); } - BAN::ErrorOr Thread::block_or_eintr_or_waketime_ns(ThreadBlocker& thread_blocker, uint64_t wake_time_ns, bool etimedout) + BAN::ErrorOr Thread::block_or_eintr_or_waketime_ns(ThreadBlocker& thread_blocker, uint64_t wake_time_ns, bool etimedout, BaseMutex* mutex) { if (is_interrupted_by_signal()) return BAN::Error::from_errno(EINTR); - thread_blocker.block_with_wake_time_ns(wake_time_ns); + thread_blocker.block_with_wake_time_ns(wake_time_ns, mutex); if (is_interrupted_by_signal()) return BAN::Error::from_errno(EINTR); if (etimedout && SystemTimer::get().ms_since_boot() >= wake_time_ns) diff --git a/kernel/kernel/ThreadBlocker.cpp b/kernel/kernel/ThreadBlocker.cpp index c33c0b6f..d73977cb 100644 --- a/kernel/kernel/ThreadBlocker.cpp +++ b/kernel/kernel/ThreadBlocker.cpp @@ -5,19 +5,19 @@ namespace Kernel { - void ThreadBlocker::block_indefinite() + void ThreadBlocker::block_indefinite(BaseMutex* mutex) { - Processor::scheduler().block_current_thread(this, static_cast(-1)); + Processor::scheduler().block_current_thread(this, static_cast(-1), mutex); } - void ThreadBlocker::block_with_timeout_ns(uint64_t timeout_ns) + void ThreadBlocker::block_with_timeout_ns(uint64_t timeout_ns, BaseMutex* mutex) { - Processor::scheduler().block_current_thread(this, SystemTimer::get().ns_since_boot() + timeout_ns); + Processor::scheduler().block_current_thread(this, SystemTimer::get().ns_since_boot() + timeout_ns, mutex); } - void ThreadBlocker::block_with_wake_time_ns(uint64_t wake_time_ns) + void ThreadBlocker::block_with_wake_time_ns(uint64_t wake_time_ns, BaseMutex* mutex) { - Processor::scheduler().block_current_thread(this, wake_time_ns); + Processor::scheduler().block_current_thread(this, wake_time_ns, mutex); } void ThreadBlocker::unblock() diff --git a/kernel/kernel/Timer/Timer.cpp b/kernel/kernel/Timer/Timer.cpp index ca9cb056..e9929ff2 100644 --- a/kernel/kernel/Timer/Timer.cpp +++ b/kernel/kernel/Timer/Timer.cpp @@ -83,9 +83,7 @@ namespace Kernel { if (ns == 0) return; - - const uint64_t wake_time_ns = ns_since_boot() + ns; - Processor::scheduler().block_current_thread(nullptr, wake_time_ns); + Processor::scheduler().block_current_thread(nullptr, ns_since_boot() + ns, nullptr); } timespec SystemTimer::real_time() const diff --git a/kernel/kernel/USB/Hub/HubDriver.cpp b/kernel/kernel/USB/Hub/HubDriver.cpp index 1253d1f9..96f8b556 100644 --- a/kernel/kernel/USB/Hub/HubDriver.cpp +++ b/kernel/kernel/USB/Hub/HubDriver.cpp @@ -269,7 +269,8 @@ namespace Kernel m_is_init_done = true; } - m_changed_port_blocker.block_with_timeout_ms(100); + // FIXME: race condition + m_changed_port_blocker.block_with_timeout_ms(100, nullptr); continue; } diff --git a/kernel/kernel/USB/XHCI/Controller.cpp b/kernel/kernel/USB/XHCI/Controller.cpp index e9ceb40e..516f46ed 100644 --- a/kernel/kernel/USB/XHCI/Controller.cpp +++ b/kernel/kernel/USB/XHCI/Controller.cpp @@ -322,7 +322,8 @@ namespace Kernel m_ports_initialized = true; } - m_port_thread_blocker.block_with_timeout_ms(100); + // FIXME: prevent race condition + m_port_thread_blocker.block_with_timeout_ms(100, nullptr); expected = true; } }