From 766bf5c89ec356699dff88f7d9ddd74e62d208b4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 19 Aug 2022 14:39:15 +0200 Subject: [PATCH] Kernel: Don't take thread lock for signal dispatch Signal dispatch is already protected by the global scheduler lock, but in some cases we also took Thread::m_lock for some reason. This led to a number of different deadlocks that started showing up with 4+ CPU's attached to the system. As a first step towards solving this, simply don't take the thread lock and let the scheduler lock cover it. Eventually, we should work in the other direction and break the scheduler lock into much finer-grained locks, but let's get out of the deadlock swamp first. --- Kernel/Thread.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index a47908d5956..9aa7537a962 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -686,7 +686,6 @@ void Thread::check_dispatch_pending_signal() { SpinlockLocker scheduler_lock(g_scheduler_lock); if (pending_signals_for_state() != 0) { - SpinlockLocker lock(m_lock); result = dispatch_one_pending_signal(); } } @@ -739,7 +738,6 @@ void Thread::send_signal(u8 signal, [[maybe_unused]] Process* sender) return; if (m_state == Thread::State::Stopped) { - SpinlockLocker lock(m_lock); if (pending_signals_for_state() != 0) { dbgln_if(SIGNAL_DEBUG, "Signal: Resuming stopped {} to deliver signal {}", *this, signal); resume_from_stopped(); @@ -814,7 +812,7 @@ void Thread::send_urgent_signal_to_self(u8 signal) DispatchSignalResult Thread::dispatch_one_pending_signal() { - VERIFY(m_lock.is_locked_by_current_processor()); + VERIFY(g_scheduler_lock.is_locked_by_current_processor()); u32 signal_candidates = pending_signals_for_state() & ~m_signal_mask; if (signal_candidates == 0) return DispatchSignalResult::Continue; @@ -832,7 +830,6 @@ DispatchSignalResult Thread::try_dispatch_one_pending_signal(u8 signal) { VERIFY(signal != 0); SpinlockLocker scheduler_lock(g_scheduler_lock); - SpinlockLocker lock(m_lock); u32 signal_candidates = pending_signals_for_state() & ~m_signal_mask; if ((signal_candidates & (1 << (signal - 1))) == 0) return DispatchSignalResult::Continue; @@ -1272,7 +1269,6 @@ void Thread::set_state(State new_state, u8 stop_signal) return; { - SpinlockLocker thread_lock(m_lock); previous_state = m_state; if (previous_state == Thread::State::Invalid) { // If we were *just* created, we may have already pending signals