Repository navigation
fix(vcores): an escaped thread takes a core again after its next block (#1929) - #2302
Conversation
NotRexed#1929) A guest thread that waited too long for a virtual core runs without one (an escape). hos_vcore_away answered 0 for it, as it held no core to give up, so every `if (away) hos_vcore_back();` after its later blocks skipped the take: the thread ran with no core, and with the core number of a core another thread held, for the rest of its life. Now the escape lasts until its next block, after which it waits for a core as every other thread does. tests/test_vcore_escape.py compiles the actual HosThread and virtual-core block with a single-threaded harness: an escape, then a block (hos_vcore_away and back, and hos_cv_sleep), nested blocks, and a thread that never escaped. The invalidate test's fixture thread gets the new field. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Two things from our side that bear on this PR. 1. The “escape never ends” finding matches our instrumentation. An escaped thread keeps running coreless with the core number it cached at thread start; 2. It does not remove the corruption in the first concurrent interval. The escaped waiter still runs concurrently with the holder of the core it reports. Our host-stack capture at the 250 ms escape shows the holder is a progressing guest decompressor (priority 19 vs the waiter's 26), polling at every back-edge (26 polls, 18 compiled). Because the holder is better priority, We have a complementary condition in The two compose: progress-aware decides whether, 🤖 Generated with opencode using DeepSeek V4.1 Flash. |
|
Thanks. I read the two the same way: yours decides whether a thread escapes, this one bounds how long. Rebasing yours on top is fine with me. Our counts are at 250 ms already, so they compare with yours directly. The runs set A finding that bears on your rule. With this change at 250 ms the escapes per run went up: 16, 20, 27 against 7, 8, 2 on main's code (3 runs each, the issue's steps, 165 s). Each one is short now, but the same thread names escape again after their next block. Who held the core, from the escape lines of those three runs (63 escapes, one holder named at each):
In a diagnostic run on main's scheduler, A correction to my own text. I wrote that these threads escaped "up to four times each in one run". By name, On the tests: this branch has the one for the next block ( |
ghost
left a comment
There was a problem hiding this comment.
Thanks, this is a small, well-aimed fix. Before, hos_vcore_away answered 0 for a thread with no core, so after an escape every if (away) hos_vcore_back(); was skipped and the thread ran coreless until it exited. Answering 1 once at the next block makes the escape last one stretch of guest code, which is what the scheduler comment already says the escape is meant to be (a deadlock breaker).
I went through the accounting and found no race:
vc_escapedis only ever touched by its own thread: it is set invc_acquire(undervc_lock, on the waiter's own thread) and read and cleared inhos_vcore_away.vc_grantonly hands cores to queued threads, and the escaped thread was dequeued, so no other thread can give it a core while the flag is set. That meansvc_escapedimplies!vc_heldfor as long as the flag is up.- Nested blocks work as before. The outer
awayanswers 1 and clears the flag, inner ones answer 0, and only the outerbacktakes a core. That coverssocket_poll->hos_sleep_ns, for example. vc_yieldandhos_vcore_checkpointreturn early for a coreless thread, so a yield doesn't cut the escape short. Only a real block does, which is right.- Exit paths (
thread_main,svcExitThread) callhos_vcore_away()and ignore the result. Clearing the flag there does no harm. hos_cv_sleepnow drops and retakeslkaround the core wait for an escaped thread, the same as for any other holder. The spurious wake-up is already documented there.
One thing to keep an eye on, not a blocker. When an escape really was breaking a stall (the holder keeps the core until the waiter makes progress), the old code paid the wait once. Now the waiter can pay it again after every block. With the 2 s default and #1929's numbers (no escapes at 2 s in three runs) I expect this to be rare. If someone later reports repeated ~2 s hitches with a ran without a core line per hitch for the same thread, this is where to look. It also fits CIB's progress-aware rule, which would decide whether to escape, and you've both agreed to stack that on top of this.
A side note on the issue's evidence, also not for this PR. The CORETLS ... cached_core_plus1=5 live_core=4 line suggests the game also keys some state on the core number it cached at thread start. A thread that takes a core again prefers vc_last, but it can land on another core in its mask, so this bounds the escape without guaranteeing the cached index is exclusive. That is the same for any thread that changes cores today, so it is out of scope here.
The test is good: it compiles the real HosThread and the whole vcore block rather than a copy, and it covers away/back, hos_cv_sleep, nesting and the never-escaped case. The invalidate_vcore_test.cpp fixture update is needed and correct.
On the siblings: #2301 (nn_fs.c OpenFile) and #2322 (nn_video.c dec_acquire) touch different files and don't conflict with this textually or in meaning. Each early return there pairs its back with the away, which is what this change relies on: an escaped thread that goes through those paths takes a core again after them. They can land in any order. Landing this one first just means their new away points also end an escape.
CI: the test workflows (Game-free source regressions, Linux AppImage, Windows builder) are waiting for a maintainer to approve the run, so tests/test_vcore_escape.py hasn't run on the Windows (llvm-mingw clang++) runner yet. Worth approving the workflows before merging. NotRexed hasn't weighed in yet either.
| if (!hos_vcores_on() || !t || !t->vc_held) return 0; | ||
| if (!hos_vcores_on() || !t) return 0; | ||
| if (!t->vc_held) { | ||
| const int escaped = t->vc_escaped; /* (only the outermost block of nested ones takes the core again) */ |
There was a problem hiding this comment.
Nit, optional: the invariant here (vc_escaped set implies no core held) holds today because only the thread-start calls use hos_vcore_back() without an away. Clearing t->vc_escaped in hos_vcore_back too, or in vc_take when t is the current thread, would keep a future unpaired back from leaving a stale flag. A stale flag would make a later inner away answer 1 and take a core in the middle of an outer block. It isn't reachable now.
There was a problem hiding this comment.
Done in #2387, as its own change because this one was merged first: hos_vcore_back now clears vc_escaped before vc_acquire, which sets the flag again if the thread escapes again. The new case in tests/test_vcore_escape.py calls back without an away on an escaped thread and then nests two blocks; the inner away must answer 0. It fails on main and passes with the change, with g++ and with clang++. I put the clear in back and not in vc_take, because a queued thread gets its core from vc_grant, which runs on the thread that releases the core, and a t == cur check there would leave the flag up.
…es a core early A guest thread that waited too long for a virtual core runs without one (an escape, vc_acquire) and has vc_escaped set; since NotRexed#2302 its next hos_vcore_away answers 1 and clears the flag, so the matching hos_vcore_back takes a core again. hos_vcore_back itself left the flag alone. Every back today follows an away that answered 1, except the two at a thread's start, where no escape can be up yet, so this is not reachable now. A later back without its away on an escaped thread would take a core and leave the flag up; the thread's next outer block would release the core, and an inner block inside it would then answer 1 and take a core in the middle of the outer block. hos_vcore_back now clears the flag before vc_acquire, which sets it again if the thread escapes again. tests/test_vcore_escape.py: a back without an away on an escaped thread, then a block inside a block (the inner away answers 0 and no core is taken inside the outer block); and such a back that escapes again (the next block still takes a core). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A thread that escapes a long wait for a virtual core keeps running without one for the rest of its life, not just for the
one wait, so it shares the per-core state of whichever thread holds the core it reports (#1929). This ends the escape at
the thread's next block.
What changed
host/hos_thread.c: an escape (vc_acquireletting a waiter run without a core) marks the thread (vc_escaped).hos_vcore_awayanswers 1 for such a thread at its next block, so theif (away) hos_vcore_back();after that blocktakes a core again, as for every other thread. Before, it answered 0 (the thread held no core to give up), every later
hos_vcore_backwas skipped, and the thread ran coreless, reporting its ideal core's number, until it ended. Only theoutermost of nested blocks takes the core again.
tests/test_vcore_escape.py(new): compiles the actualHosThread,hos_thread_coreand the virtual-core block ofhos_thread.cwith a single-threaded harness and checks: an escape, then a block throughhos_vcore_away/backandthrough
hos_cv_sleep; nested blocks; a thread that never escaped. It fails on main (7 checks) and passes with this,with g++ and clang++.
tests/invalidate_vcore_test.cpp: its fixture thread gets the new field.This does not change when an escape happens (#2127 moved that to 2 s); it bounds how long one lasts.
How it was tested
Linux (Arch), RTX 4090, NVIDIA 615.78.08, headless, the issue's steps (fresh profile, Start about 95 s into the title,
165 s per run), three runs per arm interleaved in one sitting, escape at 250 ms so that escapes happen (at 2 s there were
none in three runs on main). The game was built with this patch on #2127's commit. Not a quiet machine: load average 4-8
at the start of each run except one main run (66).
There are more escapes with the change, and that is expected from what it does: on main a thread escapes once and then
never waits for a core again; with the change it waits again after its next block and can escape again. The same
low-priority threads (bake::ModuleMgr and BakeProbe at 28, PreActorModelCompose 26, tera::FileResourceMgr and
aglTextureDataMgr::Compaction 25) escaped again and again in one run (by name up to 11 times), in the scene load after Start, mostly while
BaseProcCreateAndDeleteThread (23) held their core. Frame times were the same within the spread (the main runs' title
numbers include the run started at load 66). (Corrected 2026-11-11 16:19 UTC: I had written "up to four times each in one run". Several threads share one name, so these logs give no count per thread; by name the most was
PreActorModelCompose, 11 times in the first of the three runs.)tests/run_all.pyon this branch's head (4608fb4, on main 49f75dc): 363 passed, 2 failed, 10 skipped. The twofailures are the ones this machine has on every branch:
test_updater_asset_digest.py(only the .NET 10 SDK here, thefixture needs .NET 8) and
test_wayland_libdecor_hint.py(clang 23's-Wunused-but-set-globalunder-Werror).Not tested
Windows, AMD, the Steam Deck; anything past the first minute after Start. With the 2 s default, how often a thread
escapes twice was not measured (no escapes in three runs).
Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com