From b81b2c3fe76ad210c5386cd84256c05e70acf4df Mon Sep 17 00:00:00 2001
From: Liav A <liavalb@gmail.com>
Date: Sat, 19 Aug 2023 19:07:17 +0300
Subject: [PATCH] Kernel: Ensure only user processes are terminated properly in
 shutdown

This patch ensures that the shutdown procedure can complete due to the
fact we don't kill kernel processes anymore, and only stop the scheduler
from running after the filesystems unmount procedure.

We also need kernel processes during the shutdown procedure, because we
rely on the WorkQueue threads to run WorkQueue items to complete async
IO requests initiated by filesystem sync & unmounting, etc.

This is also simplifying the code around the killing processes, because
we don't need to worry about edge cases such as the FinalizerTask
anymore.
---
 Kernel/Tasks/PowerStateSwitchTask.cpp | 37 +++++++++++++++------------
 Kernel/Tasks/PowerStateSwitchTask.h   |  6 +----
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/Kernel/Tasks/PowerStateSwitchTask.cpp b/Kernel/Tasks/PowerStateSwitchTask.cpp
index a12bc640528..687f35f1b30 100644
--- a/Kernel/Tasks/PowerStateSwitchTask.cpp
+++ b/Kernel/Tasks/PowerStateSwitchTask.cpp
@@ -90,14 +90,11 @@ ErrorOr<void> PowerStateSwitchTask::perform_shutdown()
     g_in_system_shutdown = true;
 
     // Make sure to kill all user processes first, otherwise we might get weird hangups.
-    TRY(kill_processes(ProcessKind::User, finalizer_process->pid()));
-    TRY(kill_processes(ProcessKind::Kernel, finalizer_process->pid()));
+    TRY(kill_all_user_processes());
 
-    finalizer_process->die();
-    finalizer_process->finalize();
     size_t alive_process_count = 0;
     Process::all_instances().for_each([&](Process& process) {
-        if (process.pid() != Process::current().pid() && !process.is_dead())
+        if (!process.is_kernel_process() && !process.is_dead())
             alive_process_count++;
     });
     // Don't panic here (since we may panic in a bit anyways) but report the probable cause of an unclean shutdown.
@@ -143,6 +140,12 @@ ErrorOr<void> PowerStateSwitchTask::perform_shutdown()
         }
     }
 
+    // NOTE: We don't really need to kill kernel processes, because in contrast
+    // to user processes, kernel processes will simply not make syscalls
+    // or do some other unexpected behavior.
+    // Therefore, we just lock the scheduler big lock to ensure nothing happens
+    // beyond this point forward.
+    SpinlockLocker lock(g_scheduler_lock);
     dbgln("Attempting system shutdown...");
 
     arch_specific_poweroff();
@@ -152,15 +155,15 @@ ErrorOr<void> PowerStateSwitchTask::perform_shutdown()
     Processor::halt();
 }
 
-ErrorOr<void> PowerStateSwitchTask::kill_processes(ProcessKind kind, ProcessID finalizer_pid)
+ErrorOr<void> PowerStateSwitchTask::kill_all_user_processes()
 {
-    bool kill_kernel_processes = kind == ProcessKind::Kernel;
-
-    Process::all_instances().for_each([&](Process& process) {
-        if (process.pid() != Process::current().pid() && process.pid() != finalizer_pid && process.is_kernel_process() == kill_kernel_processes) {
-            process.die();
-        }
-    });
+    {
+        SpinlockLocker lock(g_scheduler_lock);
+        Process::all_instances().for_each([&](Process& process) {
+            if (!process.is_kernel_process())
+                process.die();
+        });
+    }
 
     // Although we *could* finalize processes ourselves (g_in_system_shutdown allows this),
     // we're nice citizens and let the finalizer task perform final duties before we kill it.
@@ -171,7 +174,7 @@ ErrorOr<void> PowerStateSwitchTask::kill_processes(ProcessKind kind, ProcessID f
         Scheduler::yield();
         alive_process_count = 0;
         Process::all_instances().for_each([&](Process& process) {
-            if (process.pid() != Process::current().pid() && !process.is_dead() && process.pid() != finalizer_pid && process.is_kernel_process() == kill_kernel_processes)
+            if (!process.is_kernel_process() && !process.is_dead())
                 alive_process_count++;
         });
 
@@ -181,9 +184,9 @@ ErrorOr<void> PowerStateSwitchTask::kill_processes(ProcessKind kind, ProcessID f
 
             if constexpr (PROCESS_DEBUG) {
                 Process::all_instances().for_each_const([&](Process const& process) {
-                    if (process.pid() != Process::current().pid() && !process.is_dead() && process.pid() != finalizer_pid && process.is_kernel_process() == kill_kernel_processes) {
-                        dbgln("Process {:2} kernel={} dead={} dying={} ({})",
-                            process.pid(), process.is_kernel_process(), process.is_dead(), process.is_dying(),
+                    if (!process.is_kernel_process() && !process.is_dead()) {
+                        dbgln("Process (user) {:2} dead={} dying={} ({})",
+                            process.pid(), process.is_dead(), process.is_dying(),
                             process.name().with([](auto& name) { return name.representable_view(); }));
                     }
                 });
diff --git a/Kernel/Tasks/PowerStateSwitchTask.h b/Kernel/Tasks/PowerStateSwitchTask.h
index 8e7f091b8c3..1c87226001c 100644
--- a/Kernel/Tasks/PowerStateSwitchTask.h
+++ b/Kernel/Tasks/PowerStateSwitchTask.h
@@ -32,11 +32,7 @@ private:
     static ErrorOr<void> perform_reboot();
     static ErrorOr<void> perform_shutdown();
 
-    enum class ProcessKind {
-        User,
-        Kernel,
-    };
-    static ErrorOr<void> kill_processes(ProcessKind, ProcessID finalizer_pid);
+    static ErrorOr<void> kill_all_user_processes();
 };
 
 }