From 65c4f9db5b6bbf6015db299cd655a3b2db1e496e Mon Sep 17 00:00:00 2001
From: Bananymous <oskari.alaranta@bananymous.com>
Date: Fri, 1 Mar 2024 01:31:28 +0200
Subject: [PATCH] Kernel: Replace CriticalScopes with SpinLocks in
 Process/Thread

---
 kernel/include/kernel/Process.h |  4 ++--
 kernel/include/kernel/Thread.h  |  1 +
 kernel/kernel/Process.cpp       | 25 ++++++++-----------------
 kernel/kernel/Thread.cpp        |  8 +++++---
 4 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h
index ec9ccd54..75d2434a 100644
--- a/kernel/include/kernel/Process.h
+++ b/kernel/include/kernel/Process.h
@@ -224,8 +224,8 @@ namespace Kernel
 		BAN::String m_working_directory;
 		BAN::Vector<Thread*> m_threads;
 
-		vaddr_t m_signal_handlers[_SIGMAX + 1] { };
-		uint64_t m_signal_pending_mask { 0 };
+		BAN::Atomic<vaddr_t> m_signal_handlers[_SIGMAX + 1] { };
+		BAN::Atomic<uint64_t> m_signal_pending_mask { 0 };
 
 		BAN::Vector<BAN::String> m_cmdline;
 		BAN::Vector<BAN::String> m_environ;
diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h
index dd5e4648..32544144 100644
--- a/kernel/include/kernel/Thread.h
+++ b/kernel/include/kernel/Thread.h
@@ -118,6 +118,7 @@ namespace Kernel
 
 		uint64_t					m_signal_pending_mask	{ 0 };
 		uint64_t					m_signal_block_mask		{ 0 };
+		SpinLock					m_signal_lock;
 		static_assert(_SIGMAX < 64);
 
 #if __enable_sse
diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp
index e7421543..6266714a 100644
--- a/kernel/kernel/Process.cpp
+++ b/kernel/kernel/Process.cpp
@@ -1,6 +1,5 @@
 #include <BAN/ScopeGuard.h>
 #include <BAN/StringView.h>
-#include <kernel/CriticalScope.h>
 #include <kernel/FS/DevFS/FileSystem.h>
 #include <kernel/FS/ProcFS/FileSystem.h>
 #include <kernel/FS/VirtualFileSystem.h>
@@ -62,18 +61,13 @@ namespace Kernel
 
 	Process* Process::create_process(const Credentials& credentials, pid_t parent, pid_t sid, pid_t pgrp)
 	{
-		static pid_t s_next_id = 1;
+		static BAN::Atomic<pid_t> s_next_id = 1;
 
-		pid_t pid;
+		pid_t pid = s_next_id++;
+		if (sid == 0 && pgrp == 0)
 		{
-			CriticalScope _;
-			pid = s_next_id;
-			if (sid == 0 && pgrp == 0)
-			{
-				sid = s_next_id;
-				pgrp = s_next_id;
-			}
-			s_next_id++;
+			sid = pid;
+			pgrp = pid;
 		}
 
 		ASSERT(sid > 0);
@@ -1484,7 +1478,6 @@ namespace Kernel
 			TRY(validate_pointer_access((void*)handler, sizeof(handler)));
 		}
 
-		CriticalScope _;
 		m_signal_handlers[signal] = (vaddr_t)handler;
 		return 0;
 	}
@@ -1496,10 +1489,9 @@ namespace Kernel
 		if (signal != 0 && (signal < _SIGMIN || signal > _SIGMAX))
 			return BAN::Error::from_errno(EINVAL);
 
-		if (pid == Process::current().pid())
+		if (pid == m_pid)
 		{
-			CriticalScope _;
-			Process::current().m_signal_pending_mask |= 1 << signal;
+			m_signal_pending_mask |= 1 << signal;
 			return 0;
 		}
 
@@ -1512,9 +1504,8 @@ namespace Kernel
 					found = true;
 					if (signal)
 					{
-						CriticalScope _;
 						process.m_signal_pending_mask |= 1 << signal;
-						// FIXME: This is super hacky
+						// FIXME: This feels hacky
 						Scheduler::get().unblock_thread(process.m_threads.front()->tid());
 					}
 					return (pid > 0) ? BAN::Iteration::Break : BAN::Iteration::Continue;
diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp
index f21be637..efdbb947 100644
--- a/kernel/kernel/Thread.cpp
+++ b/kernel/kernel/Thread.cpp
@@ -29,7 +29,7 @@ namespace Kernel
 
 	void Thread::terminate()
 	{
-		CriticalScope _;
+		set_interrupt_state(InterruptState::Disabled);
 		m_state = Thread::State::Terminated;
 		if (this == &Thread::current())
 			Scheduler::get().execute_current_thread();
@@ -245,10 +245,11 @@ namespace Kernel
 
 	void Thread::handle_signal(int signal)
 	{
-		ASSERT(!interrupts_enabled());
 		ASSERT(&Thread::current() == this);
 		ASSERT(is_userspace());
 
+		SpinLockGuard _(m_signal_lock);
+
 		auto& interrupt_stack = *reinterpret_cast<InterruptStack*>(interrupt_stack_base() + interrupt_stack_size() - sizeof(InterruptStack));
 		ASSERT(GDT::is_user_segment(interrupt_stack.cs));
 
@@ -338,7 +339,8 @@ namespace Kernel
 
 	bool Thread::add_signal(int signal)
 	{
-		ASSERT(!interrupts_enabled());
+		SpinLockGuard _(m_signal_lock);
+
 		uint64_t mask = 1ull << signal;
 		if (!(m_signal_block_mask & mask))
 		{