Kernel: Migrate process list locking to ProtectedValue

The existing recursive spinlock is repurposed for profiling only, as it
was shared with the process list.

5 files changed, 76 insertions, 86 deletions

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 6cc5892c32..d920b4ff2f 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -43,9 +43,9 @@ namespace Kernel {
 
 static void create_signal_trampoline();
 
-RecursiveSpinLock g_processes_lock;
+RecursiveSpinLock g_profiling_lock;
 static Atomic<pid_t> next_pid;
-static AK::Singleton<Process::List> s_processes;
+static AK::Singleton<ProtectedValue<Process::List>> s_processes;
 READONLY_AFTER_INIT HashMap<String, OwnPtr<Module>>* g_modules;
 READONLY_AFTER_INIT Memory::Region* g_signal_trampoline_region;
 
@@ -56,7 +56,7 @@ ProtectedValue<String>& hostname()
     return *s_hostname;
 }
 
-Process::List& processes()
+ProtectedValue<Process::List>& processes()
 {
     return *s_processes;
 }
@@ -88,20 +88,22 @@ UNMAP_AFTER_INIT void Process::initialize()
 Vector<ProcessID> Process::all_pids()
 {
     Vector<ProcessID> pids;
-    ScopedSpinLock lock(g_processes_lock);
-    pids.ensure_capacity(processes().size_slow());
-    for (auto& process : processes())
-        pids.append(process.pid());
+    processes().with_shared([&](const auto& list) {
+        pids.ensure_capacity(list.size_slow());
+        for (const auto& process : list)
+            pids.append(process.pid());
+    });
     return pids;
 }
 
 NonnullRefPtrVector<Process> Process::all_processes()
 {
     NonnullRefPtrVector<Process> output;
-    ScopedSpinLock lock(g_processes_lock);
-    output.ensure_capacity(processes().size_slow());
-    for (auto& process : processes())
-        output.append(NonnullRefPtr<Process>(process));
+    processes().with_shared([&](const auto& list) {
+        output.ensure_capacity(list.size_slow());
+        for (const auto& process : list)
+            output.append(NonnullRefPtr<Process>(process));
+    });
     return output;
 }
 
@@ -149,10 +151,9 @@ void Process::register_new(Process& process)
 {
     // Note: this is essentially the same like process->ref()
     RefPtr<Process> new_process = process;
-    {
-        ScopedSpinLock lock(g_processes_lock);
-        processes().prepend(process);
-    }
+    processes().with_exclusive([&](auto& list) {
+        list.prepend(process);
+    });
     ProcFSComponentRegistry::the().register_new_process(process);
 }
 
@@ -162,14 +163,10 @@ RefPtr<Process> Process::create_user_process(RefPtr<Thread>& first_thread, const
     if (arguments.is_empty()) {
         arguments.append(parts.last());
     }
-    RefPtr<Custody> cwd;
-    {
-        ScopedSpinLock lock(g_processes_lock);
-        if (auto parent = Process::from_pid(parent_pid)) {
-            cwd = parent->m_cwd;
-        }
-    }
 
+    RefPtr<Custody> cwd;
+    if (auto parent = Process::from_pid(parent_pid))
+        cwd = parent->m_cwd;
     if (!cwd)
         cwd = VirtualFileSystem::the().root_custody();
 
@@ -308,11 +305,10 @@ Process::~Process()
 
     PerformanceManager::add_process_exit_event(*this);
 
-    {
-        ScopedSpinLock processes_lock(g_processes_lock);
-        if (m_list_node.is_in_list())
-            processes().remove(*this);
-    }
+    if (m_list_node.is_in_list())
+        processes().with_exclusive([&](auto& list) {
+            list.remove(*this);
+        });
 }
 
 // Make sure the compiler doesn't "optimize away" this function:
@@ -417,13 +413,13 @@ void Process::crash(int signal, FlatPtr ip, bool out_of_memory)
 
 RefPtr<Process> Process::from_pid(ProcessID pid)
 {
-    ScopedSpinLock lock(g_processes_lock);
-    for (auto& process : processes()) {
-        process.pid();
-        if (process.pid() == pid)
-            return &process;
-    }
-    return {};
+    return processes().with_shared([&](const auto& list) -> RefPtr<Process> {
+        for (auto& process : list) {
+            if (process.pid() == pid)
+                return &process;
+        }
+        return {};
+    });
 }
 
 const Process::FileDescriptionAndFlags& Process::FileDescriptions::at(size_t i) const
@@ -637,13 +633,10 @@ void Process::finalize()
         }
     }
 
-    {
-        ScopedSpinLock processses_lock(g_processes_lock);
-        if (!!ppid()) {
-            if (auto parent = Process::from_pid(ppid())) {
-                parent->m_ticks_in_user_for_dead_children += m_ticks_in_user + m_ticks_in_user_for_dead_children;
-                parent->m_ticks_in_kernel_for_dead_children += m_ticks_in_kernel + m_ticks_in_kernel_for_dead_children;
-            }
+    if (!!ppid()) {
+        if (auto parent = Process::from_pid(ppid())) {
+            parent->m_ticks_in_user_for_dead_children += m_ticks_in_user + m_ticks_in_user_for_dead_children;
+            parent->m_ticks_in_kernel_for_dead_children += m_ticks_in_kernel + m_ticks_in_kernel_for_dead_children;
         }
     }
 
@@ -692,9 +685,8 @@ void Process::die()
         m_threads_for_coredump.append(thread);
     });
 
-    {
-        ScopedSpinLock lock(g_processes_lock);
-        for (auto it = processes().begin(); it != processes().end();) {
+    processes().with_shared([&](const auto& list) {
+        for (auto it = list.begin(); it != list.end();) {
             auto& process = *it;
             ++it;
             if (process.has_tracee_thread(pid())) {
@@ -705,7 +697,7 @@ void Process::die()
                     dbgln("Failed to send the SIGSTOP signal to {} ({})", process.name(), process.pid());
             }
         }
-    }
+    });
 
     kill_all_threads();
 #ifdef ENABLE_KERNEL_COVERAGE_COLLECTION
diff --git a/Kernel/Process.h b/Kernel/Process.h
index d221cde8ed..d060194a40 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -9,6 +9,7 @@
 #include <AK/Concepts.h>
 #include <AK/HashMap.h>
 #include <AK/IntrusiveList.h>
+#include <AK/IntrusiveListRelaxedConst.h>
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/String.h>
 #include <AK/Userspace.h>
@@ -580,7 +581,7 @@ private:
             return nullptr;
     }
 
-    IntrusiveListNode<Process> m_list_node;
+    mutable IntrusiveListNode<Process> m_list_node;
 
     String m_name;
 
@@ -777,39 +778,41 @@ private:
     NonnullRefPtrVector<Thread> m_threads_for_coredump;
 
 public:
-    using List = IntrusiveList<Process, RawPtr<Process>, &Process::m_list_node>;
+    using List = IntrusiveListRelaxedConst<Process, RawPtr<Process>, &Process::m_list_node>;
 };
 
-Process::List& processes();
-extern RecursiveSpinLock g_processes_lock;
+extern RecursiveSpinLock g_profiling_lock;
+
+ProtectedValue<Process::List>& processes();
 
 template<IteratorFunction<Process&> Callback>
 inline void Process::for_each(Callback callback)
 {
     VERIFY_INTERRUPTS_DISABLED();
-    ScopedSpinLock lock(g_processes_lock);
-    for (auto it = processes().begin(); it != processes().end();) {
-        auto& process = *it;
-        ++it;
-        if (callback(process) == IterationDecision::Break)
-            break;
-    }
+    processes().with_shared([&](const auto& list) {
+        for (auto it = list.begin(); it != list.end();) {
+            auto& process = *it;
+            ++it;
+            if (callback(process) == IterationDecision::Break)
+                break;
+        }
+    });
 }
 
 template<IteratorFunction<Process&> Callback>
 inline void Process::for_each_child(Callback callback)
 {
-    VERIFY_INTERRUPTS_DISABLED();
     ProcessID my_pid = pid();
-    ScopedSpinLock lock(g_processes_lock);
-    for (auto it = processes().begin(); it != processes().end();) {
-        auto& process = *it;
-        ++it;
-        if (process.ppid() == my_pid || process.has_tracee_thread(pid())) {
-            if (callback(process) == IterationDecision::Break)
-                break;
+    processes().with_shared([&](const auto& list) {
+        for (auto it = list.begin(); it != list.end();) {
+            auto& process = *it;
+            ++it;
+            if (process.ppid() == my_pid || process.has_tracee_thread(pid())) {
+                if (callback(process) == IterationDecision::Break)
+                    break;
+            }
         }
-    }
+    });
 }
 
 template<IteratorFunction<Thread&> Callback>
@@ -839,16 +842,16 @@ inline IterationDecision Process::for_each_thread(Callback callback)
 template<IteratorFunction<Process&> Callback>
 inline void Process::for_each_in_pgrp(ProcessGroupID pgid, Callback callback)
 {
-    VERIFY_INTERRUPTS_DISABLED();
-    ScopedSpinLock lock(g_processes_lock);
-    for (auto it = processes().begin(); it != processes().end();) {
-        auto& process = *it;
-        ++it;
-        if (!process.is_dead() && process.pgid() == pgid) {
-            if (callback(process) == IterationDecision::Break)
-                break;
+    processes().with_shared([&](const auto& list) {
+        for (auto it = list.begin(); it != list.end();) {
+            auto& process = *it;
+            ++it;
+            if (!process.is_dead() && process.pgid() == pgid) {
+                if (callback(process) == IterationDecision::Break)
+                    break;
+            }
         }
-    }
+    });
 }
 
 template<VoidFunction<Process&> Callback>
diff --git a/Kernel/Syscalls/kill.cpp b/Kernel/Syscalls/kill.cpp
index 64a41a2073..4033348e94 100644
--- a/Kernel/Syscalls/kill.cpp
+++ b/Kernel/Syscalls/kill.cpp
@@ -65,8 +65,7 @@ KResult Process::do_killall(int signal)
     KResult error = KSuccess;
 
     // Send the signal to all processes we have access to for.
-    ScopedSpinLock lock(g_processes_lock);
-    for (auto& process : processes()) {
+    processes().for_each_shared([&](auto& process) {
         KResult res = KSuccess;
         if (process.pid() == pid())
             res = do_killself(signal);
@@ -77,7 +76,7 @@ KResult Process::do_killall(int signal)
             any_succeeded = true;
         else
             error = res;
-    }
+    });
 
     if (any_succeeded)
         return KSuccess;
@@ -117,7 +116,6 @@ KResultOr<FlatPtr> Process::sys$kill(pid_t pid_or_pgid, int signal)
         return do_killself(signal);
     }
     VERIFY(pid_or_pgid >= 0);
-    ScopedSpinLock lock(g_processes_lock);
     auto peer = Process::from_pid(pid_or_pgid);
     if (!peer)
         return ESRCH;
diff --git a/Kernel/Syscalls/profiling.cpp b/Kernel/Syscalls/profiling.cpp
index 3a7a44df32..7f64e57753 100644
--- a/Kernel/Syscalls/profiling.cpp
+++ b/Kernel/Syscalls/profiling.cpp
@@ -31,7 +31,7 @@ KResultOr<FlatPtr> Process::sys$profiling_enable(pid_t pid, u64 event_mask)
         else
             g_global_perf_events = PerformanceEventBuffer::try_create_with_size(32 * MiB).leak_ptr();
 
-        ScopedSpinLock lock(g_processes_lock);
+        ScopedSpinLock lock(g_profiling_lock);
         if (!TimeManagement::the().enable_profile_timer())
             return ENOTSUP;
         g_profiling_all_threads = true;
@@ -44,7 +44,6 @@ KResultOr<FlatPtr> Process::sys$profiling_enable(pid_t pid, u64 event_mask)
         return 0;
     }
 
-    ScopedSpinLock lock(g_processes_lock);
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
@@ -52,6 +51,7 @@ KResultOr<FlatPtr> Process::sys$profiling_enable(pid_t pid, u64 event_mask)
         return ESRCH;
     if (!is_superuser() && process->uid() != euid())
         return EPERM;
+    ScopedSpinLock lock(g_profiling_lock);
     g_profiling_event_mask = PERF_EVENT_PROCESS_CREATE | PERF_EVENT_THREAD_CREATE | PERF_EVENT_MMAP;
     process->set_profiling(true);
     if (!process->create_perf_events_buffer_if_needed()) {
@@ -81,12 +81,12 @@ KResultOr<FlatPtr> Process::sys$profiling_disable(pid_t pid)
         return 0;
     }
 
-    ScopedSpinLock lock(g_processes_lock);
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
     if (!is_superuser() && process->uid() != euid())
         return EPERM;
+    ScopedSpinLock lock(g_profiling_lock);
     if (!process->is_profiling())
         return EINVAL;
     // FIXME: If we enabled the profile timer and it's not supported, how do we disable it now?
@@ -117,12 +117,12 @@ KResultOr<FlatPtr> Process::sys$profiling_free_buffer(pid_t pid)
         return 0;
     }
 
-    ScopedSpinLock lock(g_processes_lock);
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
     if (!is_superuser() && process->uid() != euid())
         return EPERM;
+    ScopedSpinLock lock(g_profiling_lock);
     if (process->is_profiling())
         return EINVAL;
     process->delete_perf_events_buffer();
diff --git a/Kernel/Syscalls/setpgid.cpp b/Kernel/Syscalls/setpgid.cpp
index 4467c5c63e..bfceefad36 100644
--- a/Kernel/Syscalls/setpgid.cpp
+++ b/Kernel/Syscalls/setpgid.cpp
@@ -16,7 +16,6 @@ KResultOr<FlatPtr> Process::sys$getsid(pid_t pid)
     REQUIRE_PROMISE(proc);
     if (pid == 0)
         return sid().value();
-    ScopedSpinLock lock(g_processes_lock);
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
@@ -51,7 +50,6 @@ KResultOr<FlatPtr> Process::sys$getpgid(pid_t pid)
     REQUIRE_PROMISE(proc);
     if (pid == 0)
         return pgid().value();
-    ScopedSpinLock lock(g_processes_lock); // FIXME: Use a ProcessHandle
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
@@ -68,7 +66,6 @@ KResultOr<FlatPtr> Process::sys$getpgrp()
 SessionID Process::get_sid_from_pgid(ProcessGroupID pgid)
 {
     // FIXME: This xor sys$setsid() uses the wrong locking mechanism.
-    ScopedSpinLock lock(g_processes_lock);
 
     SessionID sid { -1 };
     Process::for_each_in_pgrp(pgid, [&](auto& process) {
@@ -83,7 +80,7 @@ KResultOr<FlatPtr> Process::sys$setpgid(pid_t specified_pid, pid_t specified_pgi
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
     REQUIRE_PROMISE(proc);
-    ScopedSpinLock lock(g_processes_lock); // FIXME: Use a ProcessHandle
+    // FIXME: Use a ProcessHandle
     ProcessID pid = specified_pid ? ProcessID(specified_pid) : this->pid();
     if (specified_pgid < 0) {
         // The value of the pgid argument is less than 0, or is not a value supported by the implementation.