Repository navigation
fix(vcores): hos_vcore_back ends an escape too, so no inner block takes a core early - #2387
Conversation
…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>
|
Rebased onto main |
b52842d to
5044fa9
Compare
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for following up on the #2302 nit. Clearing vc_escaped in hos_vcore_back before vc_acquire is the right spot: vc_acquire sets the flag again if the thread escapes again, so after any back the flag is up only while the thread really is running without a core. Your reason for not putting it in vc_take holds too, since vc_grant runs on the releasing thread.
What I checked:
- Races:
vc_escapedis still written only by its own thread (vc_acquireont,hos_vcore_away, nowhos_vcore_back).vc_grant/vc_releasenever touch it, so the extra store at eachbackadds no cross-thread access. - Nesting stays balanced: the flag can only be up while
vc_held == 0, so the outermostawaystill answers 1 and clears it, inner ones answer 0, and only the outerbacktakes a core. The paired callers (hos_thread_wait,hos_cv_sleep, the otherif (away) hos_vcore_back()sites, the #2301/#2322 users) behave exactly as before. The two unpairedbacks at thread start see a zeroed flag, so nothing changes there today, as the description says. vc_yieldgoes throughvc_release/vc_acquiredirectly rather thanback, so a re-escape there still leaves the flag up until the next block, as intended.- The two new test cases cover the right things, and the second one (re-escape inside
back) guards against moving the clear aftervc_acquire.
CI: test_vcore_escape.py and test_invalidate_vcore.py pass on Ubuntu and Windows. The two red jobs aren't from this PR: Windows part 1/2 fails in test_native_astc_texture_count.py (scanf deprecated under -Werror with clang on Windows), and appimage timed out fetching system packages.
Looks good to me.
|
Thanks for reviewing the flag ownership and re-escape cases. The unrelated Windows ASTC fixture failure is tracked in #2397; no further scheduler change is needed for that CI failure. |
What this is for: this makes the virtual-core escape fix of #2302 (issue #1929) safe against one kind of future change. @hckrman101 asked for it as an optional point in his review (comment). Nothing that runs today changes, because no call in the code reaches the case: it is there so that a later
hos_vcore_back()without itshos_vcore_away()cannot leave an escape flag up. What is measured: the test with g++ and clang++, on this branch and against main'shost/hos_thread.c, and the whole oftests/run_all.pyon Arch Linux. Not measured: no game run, because no call reaches the changed case. Windows was not tried.Problem
vc_acquire), andvc_escapedis set. Since fix(vcores): an escaped thread takes a core again after its next block (#1929) #2302 the thread's nexthos_vcore_away()answers 1 and clears the flag, so thehos_vcore_back()paired with it takes a core again.hos_vcore_back()itself leaves the flag alone. Today everybackfollows anawaythat answered 1, except the two at a thread's start (hos_boot_main_thread,thread_main). No escape can be up there yet: the thread is new and zeroed, and thatbackis its first wait for a core. So the case below cannot happen now.backwithout itsawayon a thread that has escaped. The thread would take a core and keep the flag. Its next outer block would give the core up as usual. A block nested inside that one would then answer 1 and take a core in the middle of the outer block, which is the opposite of what the nesting rule is for.What changed
host/hos_thread.chos_vcore_back: clearsvc_escapedbeforevc_acquire, which sets it again if the thread escapes again. So after anyback, the flag is up only when the thread is again running without a core. The comment on the struct field says this now.backand notvc_take(the other place the review named): a thread waiting in the queue gets its core fromvc_grant.vc_grantruns on the thread that gives the core up, so a clear invc_takelimited tot == curwould miss that path.tests/test_vcore_escape.pyhas two new cases.backwithout anaway, then nests two blocks. The innerawaymust answer 0, and no core may be taken inside the outer block.backescapes again while both cores are held. The thread must still take a core after its next block.How it was tested
Arch Linux, kernel 7.2.15, g++ 16.2.1, clang++ 23.1.3, 2026-11-11. Branch head b52842d on main b744b1e.
ok.host/hos_thread.c(b744b1e), with both compilers:vc_acquireinstead, the flag of an escape that has just happened is wiped, and 8 checks fail with both compilers.tests/test_invalidate_vcore.py, which cutshos_vcore_awayandhos_vcore_backout of the source, passes with both compilers.tests/run_all.py: 373 passed, 4 failed, 10 skipped. Two of the failures fail on this machine whatever the change:test_updater_asset_digest.py(only the .NET 10 SDK is installed here, and the fixture needs net8.0) andtest_wayland_libdecor_hint.py(clang 23's-Wunused-but-set-globalunder-Werror). The other two,test_vcore_fs_open.pyandtest_vcore_video_acquire.py, fail on main itself until test: test_vcore_fs_open and test_vcore_video_acquire build again with env_on #2380 lands: since fix: switches that behave as players expect, and dead ones removed #2042 they no longer compile.Not tested
back, but no call today has a flag up there, so I expect no difference in the game. That is an inference from the code, not a measurement.Risks
back. The flag is touched only by its own thread, as before.Closes #2386.
Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com