From 1a0a0423e7c2d68e57fd5c32318e9c19eb961fbd Mon Sep 17 00:00:00 2001 From: TellowKrinkle Date: Sun, 27 Mar 2022 22:47:55 -0500 Subject: [PATCH] Core: Switch MTGS and SysThreadBase to WorkSema --- common/PersistentThread.h | 2 +- pcsx2/GS.h | 4 +- pcsx2/MTGS.cpp | 130 ++++++++++++--------------------- pcsx2/System/SysCoreThread.cpp | 2 +- pcsx2/System/SysThreadBase.cpp | 10 +-- 5 files changed, 54 insertions(+), 94 deletions(-) diff --git a/common/PersistentThread.h b/common/PersistentThread.h index ce7ca56e56..734d083160 100644 --- a/common/PersistentThread.h +++ b/common/PersistentThread.h @@ -100,7 +100,7 @@ namespace Threading uptr m_native_id; // typically an id, but implementing platforms can do whatever. uptr m_native_handle; // typically a pointer/handle, but implementing platforms can do whatever. - Semaphore m_sem_event; // general wait event that's needed by most threads + WorkSema m_sem_event; // general wait event that's needed by most threads Semaphore m_sem_startup; // startup sync tool Mutex m_mtx_InThread; // used for canceling and closing threads in a deadlock-safe manner MutexRecursive m_mtx_start; // used to lock the Start() code from starting simultaneous threads accidentally. diff --git a/pcsx2/GS.h b/pcsx2/GS.h index bdc066327b..264bbdc4a9 100644 --- a/pcsx2/GS.h +++ b/pcsx2/GS.h @@ -329,15 +329,13 @@ public: std::atomic m_ReadPos; // cur pos gs is reading from std::atomic m_WritePos; // cur pos ee thread is writing to - std::atomic m_RingBufferIsBusy; std::atomic m_SignalRingEnable; std::atomic m_SignalRingPosition; std::atomic m_QueuedFrameCount; std::atomic m_VsyncSignalListener; - Mutex m_mtx_RingBufferBusy; // Is obtained while processing ring-buffer data - Mutex m_mtx_RingBufferBusy2; // This one gets released on semaXGkick waiting... + Mutex m_mtx_RingBufferBusy2; // Gets released on semaXGkick waiting... Mutex m_mtx_WaitGS; Semaphore m_sem_OnRingReset; Semaphore m_sem_Vsync; diff --git a/pcsx2/MTGS.cpp b/pcsx2/MTGS.cpp index 26d3ddb1a6..0e384de2d3 100644 --- a/pcsx2/MTGS.cpp +++ b/pcsx2/MTGS.cpp @@ -19,6 +19,7 @@ #include #include +#include "common/ScopedGuard.h" #include "common/StringUtil.h" #include "GS.h" @@ -76,7 +77,6 @@ void SysMtgsThread::OnStart() m_ReadPos = 0; m_WritePos = 0; - m_RingBufferIsBusy = false; m_packet_size = 0; m_packet_writepos = 0; @@ -176,12 +176,6 @@ void SysMtgsThread::PostVsyncStart(bool registers_written) m_VsyncSignalListener.store(true, std::memory_order_release); //Console.WriteLn( Color_Blue, "(EEcore Sleep) Vsync\t\tringpos=0x%06x, writepos=0x%06x", m_ReadPos.load(), m_WritePos.load() ); - // We will wait a vsync event from the MTGS ring. If the ring is already purged, the event will never come ! - // To avoid this potential deadlock, ring must be wake up after m_VsyncSignalListener - // Note: potentially we can also miss the previous wake up if we optimize away the post just before the release of busy signal of the ring - // So let's ensure the ring doesn't sleep - m_sem_event.Post(); - m_sem_Vsync.WaitNoCancel(); } @@ -219,46 +213,6 @@ void SysMtgsThread::OpenGS() GSsetGameCRC(ElfCRC, 0); } -class RingBufferLock -{ - ScopedLock m_lock1; - ScopedLock m_lock2; - SysMtgsThread& m_mtgs; - -public: - RingBufferLock(SysMtgsThread& mtgs) - : m_lock1(mtgs.m_mtx_RingBufferBusy) - , m_lock2(mtgs.m_mtx_RingBufferBusy2) - , m_mtgs(mtgs) - { - m_mtgs.m_RingBufferIsBusy.store(true, std::memory_order_relaxed); - } - virtual ~RingBufferLock() - { - m_mtgs.m_RingBufferIsBusy.store(false, std::memory_order_relaxed); - } - void Acquire() - { - m_lock1.Acquire(); - m_lock2.Acquire(); - m_mtgs.m_RingBufferIsBusy.store(true, std::memory_order_relaxed); - } - void Release() - { - m_mtgs.m_RingBufferIsBusy.store(false, std::memory_order_relaxed); - m_lock2.Release(); - m_lock1.Release(); - } - void PartialAcquire() - { - m_lock2.Acquire(); - } - void PartialRelease() - { - m_lock2.Release(); - } -}; - void SysMtgsThread::ExecuteTaskInThread() { // Threading info: run in MTGS thread @@ -268,19 +222,22 @@ void SysMtgsThread::ExecuteTaskInThread() PacketTagType prevCmd; #endif - RingBufferLock busy(*this); + ScopedGuard kill_on_exception([this]{ m_sem_event.Kill(); }); + ScopedLock mtvu_lock(m_mtx_RingBufferBusy2); while (true) { - busy.Release(); // Performance note: Both of these perform cancellation tests, but pthread_testcancel // is very optimized (only 1 instruction test in most cases), so no point in trying // to avoid it. - m_sem_event.Wait(); + m_mtx_RingBufferBusy2.Release(); + + m_sem_event.WaitForWork(); StateCheckInThread(); - busy.Acquire(); + + m_mtx_RingBufferBusy2.Acquire(); // note: m_ReadPos is intentionally not volatile, because it should only // ever be modified by this thread. @@ -402,10 +359,10 @@ void SysMtgsThread::ExecuteTaskInThread() MTVU_LOG("MTGS - Waiting on semaXGkick!"); if (!vu1Thread.semaXGkick.TryWait()) { - busy.PartialRelease(); + mtvu_lock.Release(); // Wait for MTVU to complete vu1 program vu1Thread.semaXGkick.WaitWithoutYield(); - busy.PartialAcquire(); + mtvu_lock.Acquire(); } Gif_Path& path = gifUnit.gifPath[GIF_PATH_1]; GS_Packet gsPack = path.GetGSPacketMTVU(); // Get vu1 program's xgkick packet(s) @@ -537,7 +494,7 @@ void SysMtgsThread::ExecuteTaskInThread() } } - busy.Release(); + // TODO: With the new race-free WorkSema do we still need these? // Safety valve in case standard signals fail for some reason -- this ensures the EEcore // won't sleep the eternity, even if SignalRingPosition didn't reach 0 for some reason. @@ -605,33 +562,43 @@ void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU) return; Gif_Path& path = gifUnit.gifPath[GIF_PATH_1]; - u32 startP1Packs = weakWait ? path.GetPendingGSPackets() : 0; // Both m_ReadPos and m_WritePos can be relaxed as we only want to test if the queue is empty but // we don't want to access the content of the queue - if (isMTVU || m_ReadPos.load(std::memory_order_relaxed) != m_WritePos.load(std::memory_order_relaxed)) + SetEvent(); + if (weakWait) { - SetEvent(); - RethrowException(); - for (;;) + // On weakWait we will stop waiting on the MTGS thread if the + // MTGS thread has processed a vu1 xgkick packet, or is pending on + // its final vu1 xgkick packet (!curP1Packs)... + // Note: m_WritePos doesn't seem to have proper atomic write + // code, so reading it from the MTVU thread might be dangerous; + // hence it has been avoided... + u32 startP1Packs = path.GetPendingGSPackets(); + if (startP1Packs) { - if (weakWait) + while (true) + { m_mtx_RingBufferBusy2.Wait(); - else - m_mtx_RingBufferBusy.Wait(); - RethrowException(); - if (!isMTVU && m_ReadPos.load(std::memory_order_relaxed) == m_WritePos.load(std::memory_order_relaxed)) - break; - u32 curP1Packs = weakWait ? path.GetPendingGSPackets() : 0; - if (weakWait && ((startP1Packs - curP1Packs) || !curP1Packs)) - break; - // On weakWait we will stop waiting on the MTGS thread if the - // MTGS thread has processed a vu1 xgkick packet, or is pending on - // its final vu1 xgkick packet (!curP1Packs)... - // Note: m_WritePos doesn't seem to have proper atomic write - // code, so reading it from the MTVU thread might be dangerous; - // hence it has been avoided... + RethrowException(); + if (path.GetPendingGSPackets() != startP1Packs) + break; + } + } + } + else + { + if (!m_sem_event.WaitForEmpty()) + { + // There's a small race here as the semaphore is killed before the exception is set + // Try a few times to recover the actual exception before throwing something more generic + for (int i = 0; i < 5; i++) + { + std::this_thread::yield(); + RethrowException(); + } + throw Exception::RuntimeError(std::runtime_error("MTGS Thread Died")); } } @@ -647,9 +614,7 @@ void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU) // For use in loops that wait on the GS thread to do certain things. void SysMtgsThread::SetEvent() { - if (!m_RingBufferIsBusy.load(std::memory_order_relaxed)) - m_sem_event.Post(); - + m_sem_event.NotifyOfWork(); m_CopyDataTally = 0; } @@ -677,7 +642,7 @@ void SysMtgsThread::SendDataPacket() { WaitGS(); } - else if (!m_RingBufferIsBusy.load(std::memory_order_relaxed)) + else { m_CopyDataTally += m_packet_size; if (m_CopyDataTally > 0x2000) @@ -840,12 +805,9 @@ void SysMtgsThread::SendSimpleGSPacket(MTGS_RingCommand type, u32 offset, u32 si if (!EmuConfig.GS.SynchronousMTGS) { - if (!m_RingBufferIsBusy.load(std::memory_order_relaxed)) - { - m_CopyDataTally += size / 16; - if (m_CopyDataTally > 0x2000) - SetEvent(); - } + m_CopyDataTally += size / 16; + if (m_CopyDataTally > 0x2000) + SetEvent(); } } diff --git a/pcsx2/System/SysCoreThread.cpp b/pcsx2/System/SysCoreThread.cpp index 0269620ddd..769d8d488b 100644 --- a/pcsx2/System/SysCoreThread.cpp +++ b/pcsx2/System/SysCoreThread.cpp @@ -316,7 +316,7 @@ void SysCoreThread::DoCpuExecute() void SysCoreThread::ExecuteTaskInThread() { Threading::EnableHiresScheduler(); // Note that *something* in SPU2 and GS also set the timer resolution to 1ms. - m_sem_event.WaitWithoutYield(); + m_sem_event.WaitForWork(); m_mxcsr_saved.bitmask = _mm_getcsr(); diff --git a/pcsx2/System/SysThreadBase.cpp b/pcsx2/System/SysThreadBase.cpp index 291f7ecb8e..7cdb66ff9d 100644 --- a/pcsx2/System/SysThreadBase.cpp +++ b/pcsx2/System/SysThreadBase.cpp @@ -39,7 +39,7 @@ void SysThreadBase::Start() pxAssertDev((m_ExecMode == ExecMode_Closing) || (m_ExecMode == ExecMode_Closed), "Unexpected thread status during SysThread startup."); - m_sem_event.Post(); + m_sem_event.NotifyOfWork(); } @@ -118,7 +118,7 @@ void SysThreadBase::Suspend(bool isBlocking) } pxAssertDev(m_ExecMode == ExecMode_Closing, "ExecMode should be nothing other than Closing..."); - m_sem_event.Post(); + m_sem_event.NotifyOfWork(); } if (isBlocking) @@ -156,7 +156,7 @@ void SysThreadBase::Pause(SystemsMask systemsToTearDown, bool debug) OnPauseDebug(); else OnPause(); - m_sem_event.Post(); + m_sem_event.NotifyOfWork(); } m_RunningLock.Wait(); @@ -174,7 +174,7 @@ void SysThreadBase::PauseSelf() m_ExecMode = ExecMode_Pausing; OnPause(); - m_sem_event.Post(); + m_sem_event.NotifyOfWork(); } } @@ -190,7 +190,7 @@ void SysThreadBase::PauseSelfDebug() m_ExecMode = ExecMode_Pausing; OnPauseDebug(); - m_sem_event.Post(); + m_sem_event.NotifyOfWork(); } }