Repository navigation
drm/asahi: kill a G17 context at most once - #513
Merged
Merged
Conversation
A closed context qualifies for a render-only kill while one of its pairs holds executed but unstamped commands and no compute queue still runs it, switches away from it or killed it. A context killed through its compute queue normally leaves that state at once, since the acknowledged kill releases the pair in the same service pass, but only that timing kept the render-only path from killing it again: the compute queue's own record of the kill ends when the queue is rebound. A second kill of a killed context is harmful. In a diagnostic run that held the pair's release back for 100 ms after the kill, 31 contexts were killed twice, 0.1 to 2.5 ms apart, and twice the firmware recovered the GPU 0.3 ms after acknowledging the second kill, on a page fault in the killed pair's own tiling queue. The 35 contexts killed once in that run caused no recovery. Record on the context that a kill was published, by either path, and never publish a render-only kill for a context that has one.
Contributor
Author
|
Two-client ray-query pairs run (10 minutes) on this build (module b0e2c572):
The physical queue table reached 127/128, with killed pairs reused 11 times and no refusals. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A closed G17 context could be killed twice: once through its compute queue (#505) and again on its render pairs' behalf (#508). Only timing prevented it, and a second kill is harmful. This records on the context that a kill was published and never publishes another.
The hazard
#508 publishes a render-only kill for a closed context whose pairs hold executed but unstamped commands, while no compute queue still runs it, switches away from it or killed it. A context killed through its compute queue normally leaves that state in the same service pass, because the acknowledged kill releases the pair. The compute queue's own record of the kill ends when the queue is rebound, so if the pair's release lags behind that, the context looks like a render-only candidate again.
A diagnostic run that held the pair's release back for 100 ms after the kill showed what follows:
No production run so far had a double kill, but nothing in the code ruled it out.
The change
Contextgains akill_publishedflag, set by bothpublish_compute_exitandpublish_render_killonce the kill record is published.render_kill_candidaterefuses a context with the flag set. The eligibility test is a pure helper (render_kill_eligible) with a KUnit case.Testing