From 61c8eb07070887af566fdc972533365c7485fe24 Mon Sep 17 00:00:00 2001 From: "Herman S." <429230+has207@users.noreply.github.com> Date: Thu, 9 Apr 2026 21:14:39 +0900 Subject: [PATCH] [Threading] Improve same-CPU spinlock contention Spinlock acquire now checks if the lock holder shares the same guest CPU and yields more aggressively (Sleep(0)) when contending on the same Xenon HW thread, which should better approximate real kernel's implicit serialization. Child threads without an explicit affinity mask now inherit the parent's guest CPU assignment instead of round-robining, so the spinlock check correctly identifies parent-child co-location. --- .../kernel/xboxkrnl/xboxkrnl_threading.cc | 31 ++++++++++++++++--- src/xenia/kernel/xthread.cc | 16 ++++++---- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc index 6dec1c20e..83bb83e18 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc @@ -1138,11 +1138,34 @@ uint32_t xeKeKfAcquireSpinLock(PPCContext* ctx, X_KSPINLOCK* lock, PrefetchForCAS(lock); assert_true(lock->prcb_of_owner != static_cast(ctx->r[13])); + + uint32_t our_pcr = static_cast(ctx->r[13]); + uint8_t our_cpu = + ctx->TranslateVirtualGPR(our_pcr)->prcb_data.current_cpu; + // Lock. - while (!xe::atomic_cas(0, xe::byte_swap(static_cast(ctx->r[13])), - &lock->prcb_of_owner.value)) { - // Spin! - // TODO(benvanik): error on deadlock? + while ( + !xe::atomic_cas(0, xe::byte_swap(our_pcr), &lock->prcb_of_owner.value)) { + // On real hardware, threads sharing a Xenon HW thread are serialized by + // the kernel scheduler — the spinner would be preempted within one + // timeslice (~1ms) so the holder can make progress. In the naive + // host-thread model both threads run truly in parallel, so the spinner + // can burn its entire host quantum without giving the holder a chance. + // + // Check whether the lock holder is assigned to the same guest CPU as us. + // If so, yield the host thread aggressively (Sleep(0)) to force a host + // context switch and give the holder a chance to run and release. + // The relationship is stable — affinity doesn't change while a thread + // holds a spinlock — so one check per contention episode is sufficient. + uint32_t owner_pcr_be = lock->prcb_of_owner.value; + if (owner_pcr_be) { + uint32_t owner_pcr = xe::byte_swap(owner_pcr_be); + auto* owner_kpcr = ctx->TranslateVirtual(owner_pcr); + if (owner_kpcr->prcb_data.current_cpu == our_cpu) { + xe::threading::Sleep(std::chrono::milliseconds(0)); + continue; + } + } xe::threading::MaybeYield(); } diff --git a/src/xenia/kernel/xthread.cc b/src/xenia/kernel/xthread.cc index 19be6a6cd..f7ec64bcb 100644 --- a/src/xenia/kernel/xthread.cc +++ b/src/xenia/kernel/xthread.cc @@ -155,13 +155,17 @@ static uint8_t next_cpu = 0; static uint8_t GetFakeCpuNumber(uint8_t proc_mask) { // NOTE: proc_mask is logical processors, not physical processors or cores. if (!proc_mask) { - next_cpu = (next_cpu + 1) % 6; - return next_cpu; // is this reasonable? - // TODO(Triang3l): Does the following apply here? + // On Xbox 360, threads without an explicit processor assignment stay on + // the same hardware thread as the parent. Preserve this so that the + // guest CPU assignment reflects the game's intent — parent-child thread + // pairs that share a HW thread may rely on implicit serialization. // https://docs.microsoft.com/en-us/windows/win32/dxtecharts/coding-for-multiple-cores - // "On Xbox 360, you must explicitly assign software threads to a particular - // hardware thread by using XSetThreadProcessor. Otherwise, all child - // threads will stay on the same hardware thread as the parent." + XThread* parent = current_xthread_tls_; + if (parent) { + return parent->active_cpu(); + } + next_cpu = (next_cpu + 1) % 6; + return next_cpu; } assert_false(proc_mask & 0xC0);