Repository navigation
drm/asahi: G17 leak fix, render notifications and hop fixes from aurora (stacked on #229) - #244
Merged
Merged
Conversation
…t record When a compute command fails on the GPU the firmware writes a zero-duration completion record for it, which quarantines its queue, and reports the failure with a firmware event of type 7 carrying the queue and the stamp the command would have stored. It never stores that stamp. The driver ignored the event, so the quarantine waited for a retirement witness that could not arrive, and the queue kept its pool, its traversal scratch and its share of the lower GPU VA window for as long as the module stayed loaded. Under ray query stress that fails this way every few seconds (for example ray_query.stress.geometry_shader beside other clients) the retained queues filled the lower window within minutes. Every later allocation there failed: publications returned EBUSY, and gnome-shell crashed on ENOMEM creating a 16K texture after about 7 minutes. Decode the event as ComputeFault and accept its stamp as the faulted command's completion stamp for the quarantine's retirement witness, while it is newer than the stored one. The witness still requires the queue's ring to be fully consumed, so commands published after the failed one keep waiting for their own stamp. (cherry picked from commit 9a76756) Signed-off-by: DjDeveloperr <hey@djdev.me>
A logical render queue admitted 16 commands at a time, although its pair has command records (TA and fragment descriptors, configuration and timestamp slots) for 32. At depth the firmware runs the renders already announced when it starts a batch, then writes their completions one after another before it starts the next, so the window bounded every batch: at render depth 32 the median render ran in a batch of 8, at most 14, and the fixed cost of each batch edge fell on few renders. Admit as many commands as a pair has records; the scheduler's credits follow. At depth 32 and 64 the median render now runs in a batch of 11 and 15, at most 32. Each pair keeps a Generation per command in flight, 16 more of them, about 1.75 MiB more per render queue. A build-time check keeps the depth within the records. Measured on Neo in one boot, 16 against 32 commands, alternating, three rounds (us per submission, medians): render.light tri, depth 32 30.41 -> 29.50 render.light tri, depth 64 30.34 -> 29.38 Shallower depths, several queues or processes and the GPU time of each render are unchanged within run-to-run spread. (cherry picked from commit 347831e) Signed-off-by: DjDeveloperr <hey@djdev.me>
A published job's deadline is renewed, up to thirty seconds, while some command retired within the last five seconds: the GPU is then shared, not hung. Two ray traversal dispatches side by side retire nothing until they end, though both progress. Alone each took about 6.3 s; side by side they took 10 to 17 s, past the ten seconds a long job gets without that renewal, so the watchdog failed most of them with ETIMEDOUT and their clients lost the device. Also count the GPU as shared while a long job of another queue runs beside a long job. The thirty-second bound still ends a pair that never finishes. (cherry picked from commit 3db24da) Signed-off-by: DjDeveloperr <hey@djdev.me>
An event pass considered a render pair only once both of its queues had posted a stamp notification, the tiling queue's and the fragment queue's. The tiling notification adds nothing a retirement needs: the fragment kick starts only after its fragment barrier saw the tiling stamp, and the observation still requires both stamps in memory to cover the pass. Recovery attribution reads the stamps, and parameter-buffer growth arrives as its own record. Select a pair, and decide that it needs a rescan, on the fragment queue's notification alone, through one helper with KUnit coverage. A pass whose only news is a tiling stamp no longer borrows the pair for an observation that cannot retire. (cherry picked from commit de0b22a) Signed-off-by: DjDeveloperr <hey@djdev.me>
…only The tiling descriptor has a word at the position of the fragment descriptor's first-kick word, which the fragment stage sets only on a queue pair's first kick. The tiling setup set it on every kick. With it set, the firmware also notifies the host of the tiling queue's completion on every pass, so each render cost two firmware notifications, two mailbox interrupts and two event passes, while retirement needs only the fragment queue's notification (previous commit). Name the word first_kick and set it for ordinal 0 only, as for the fragment stage, on the A18 Pro firmware (26A428). The M4 Pro firmware (25G83) keeps the old value on every kick until it is measured there. Measured on Neo in one boot through a diagnostic switch that toggled only this word (3 rounds each, medians, us per submit): GPU mailbox IRQs per submit (render.light d32) 2.18 -> 1.10 render.light tri queue d1 105.26 -> 104.12 render.light tri queue d8 30.98 -> 29.30 render.light tri queue d32 29.67 -> 27.80 render.light tri queue d64 29.67 -> 27.78 render.light tri procs d2 52.12 -> 49.73 render.light tri queues d2 52.16 -> 49.65 compute.light queue d1 103.39 -> 104.08 sync.signal queues d2 timeline / binary 22.79 / 22.37 -> 22.61 / 22.61 (cherry picked from commit 5073eac) Signed-off-by: DjDeveloperr <hey@djdev.me>
When a compute command fails, the firmware holds the commands its context published after it and never runs them; only a kill of the context returns them. The context's render work behind the same failure behaves differently. It runs about a second later, consumes its ring entries and returns its free-list blocks, but the firmware never stores its completion stamp and leaves the job linked in the context's work list until it processes a kill. None of that work could retire, so a closed context whose compute failed kept its pools, its render pairs, its memory and its scheduler teardown until the GPU firmware stopped. Let the kill proceed for such a context. A quarantined render pair whose commands all ran (both pass timestamps written, rings consumed, free list balanced) counts as idle for the kill and is held for it, and the kill no longer waits for render pools that are only released after it. A faulted compute queue with no commands behind the failure is a kill candidate as well. This applies only once the owner has closed; a live owner's quarantined pair stays quarantined. Free only after the firmware acknowledges the kill. The pair's unstamped work is retired once the context's scheduler page reports the kill, and its graph memory stays allocated and out of the reuse cache until then. The detach path enforces the same rule: a context whose work list still links a node and has no acknowledged kill never has its graphs freed or recycled. The firmware leaves a killed context's last job linked even after the kill, so the context's 32 KiB work object is kept out of reuse until the firmware stops; debugfs counts these as firmware_linked_objects and a warning is printed past 256. Reuse a killed context's render pairs last. The kill hold on them ends when the context's scheduler release is consumed. A new owner then takes a clean released pair first, then an unowned slot while the physical queue table still has room for both of its identities, and only then the killed pair released longest ago. Siblings and spares never take killed pairs, and a sibling no longer claims an unowned slot once the table is full. Admission is told whether the table has room whenever the runtime changes it. Publish a failed context's kill only while no other context has a compute command in flight, or once the GPU has retired nothing for the liveness interval. When GPU memory comes within an eighth of the device limit, at most one waiting kill per 250 ms skips that wait. debugfs reports the waits and the pressure kills, and a wait over 5 s is warned about once per kill. The order matters. Earlier versions retired and freed the pair first and killed afterwards, and the firmware then used memory that had been handed out again: GPU recoveries 4-7 ms after the kill and occasional firmware crashes when the kill followed within milliseconds, and a storm of compute watchdog timeouts when it followed minutes later. A run that handed killed pairs to new owners first stranded the teardowns of 24 contexts, and a render stall behind a failed context took the desktop down. Reusing them last without regard to the queue table ran out of physical queues after about 50 kills. A kill published beside other clients' running compute made the firmware recover the GPU twice in ten minutes of a five-lane stress load, which the wait avoids. Waiting without a bound let the dead clients' memory reach the device limit under sustained compute, and gnome-shell then crashed on ENOMEM, which the memory escape avoids. On Neo, ten minutes of a five-lane stress load (render, render2, long ALU, churn and global-priority CTS beside two ray query geometry-shader stress loops) had 52 faults and 15 kills (2 under memory pressure), all acknowledged and their pairs retired. There were 0 GPU recoveries and nothing was left pending afterwards, against 12 teardowns left pending without this change. Ten minutes of two ray query stress loops beside heavy geometry had 51 faults and 51 kills (3 under memory pressure), with 0 recoveries, the physical queue table full at 127 of 128 with no refusals, and 4 render-only teardowns left pending. In both, GPU memory peaked at 4.3 GiB of the 5.0 GiB device limit, no allocation was refused and gnome-shell stayed up. Known limits: - A killed context's 32 KiB work object stays allocated until the GPU firmware restarts. - While other clients keep compute running, a faulted client's memory stays held until its kill publishes: at the next moment without foreign compute, or through the memory escape near the device limit. - A closed context whose compute side is clean but whose render pair holds executed, unstamped work is not killed, since no compute queue is left to carry the kill. A kill that doesn't need a compute exit candidate would close this. - A kill that is safe beside running compute would remove the wait. It is not understood yet. An asahi_g17_teardown tracepoint records fault, unstamped retire, kill, acknowledgement, detach and slot-owner events. (cherry picked from commit 078d744) Signed-off-by: DjDeveloperr <hey@djdev.me>
A closed context whose failed compute command was retired on the firmware's fault record has a clean compute side, yet the render work it queued behind the failure ran without storing its stamp and stays linked in the context's work list. Such a pair is released only after a kill of its context, and that kill was only ever published through one of the context's compute queues, of which none is left to carry it. The context, its pairs and its memory therefore stayed until the GPU firmware stopped: 4 contexts after ten minutes of ray query stress beside heavy geometry. Publish the kill for such a context on its render pairs' behalf. A context qualifies once it has been closed for 250 ms, every pair of it is idle or has executed all of its commands, at least one has, and no compute queue still runs it, switches away from it or has published its kill. The kill names one of the context's own render queues that its scheduler page lists and is refused if the page names a queue of another context; the pairs it holds must have a single admission owner. The rules of a failed context's kill apply unchanged: the pairs are held for the kill, their memory is released only once the scheduler page reports it, the kill waits for a moment without other contexts' compute unless GPU memory is pressed, and the held pairs become killed pairs, reused last, once the context's scheduler release is consumed. The teardown entry keeps the kill's cookie, so only that exact cookie acknowledges it, and its doorbell is retried without duplicating the record. debugfs reports render-only kills, their acknowledgements, the contexts refused by first reason and the kills waiting. On Neo, ten minutes of two ray query stress loops beside heavy geometry had 53 faults, 53 kills through compute queues and 4 render-only kills, all acknowledged, with 0 GPU recoveries and nothing left pending afterwards (4 teardowns before). Ten minutes of the five-lane stress load had 48 faults, 8 kills through compute queues and 2 render-only kills, all acknowledged, with 0 GPU recoveries and nothing left pending. No render-only kill was refused in either run. The asahi_g17_teardown tracepoint records render-only kills as kind 7 and their acknowledgements with queue 0x100. (cherry picked from commit 72771fb) Signed-off-by: DjDeveloperr <hey@djdev.me>
The kunit_tests macro compiles its test modules only when CONFIG_KUNIT is y. With CONFIG_KUNIT=m the four test-only G17 helpers remain compiled and produce dead-code warnings, breaking a CONFIG_WERROR build. Use the same built-in KUnit condition for the helpers as for their callers. Fixes: a36e352 ("drm/asahi: size the G17 compute QID budget by open logical queues") Fixes: 014605a ("drm/asahi: add G17 resource wait domains and use them for queue creation") Fixes: 64f717d ("drm/asahi: wait for G17 compute capacity on the release that frees it") Assisted-by: LLM Signed-off-by: Ryan Murray <ryan@aurorasilicon.org> (cherry picked from commit 91c4792) Signed-off-by: DjDeveloperr <hey@djdev.me>
…aited A queue whose retirement stops at a command with a completion record but no covering stamp yet sets retire_pending, which counted as leaving ordinary retirement: the next publication rang its doorbell at once, and a withheld doorbell was released whatever its predecessors. That happens after most completions. Each doorbell makes the firmware read the work ring, and a hop between queues whose producer ends 30-80 us after one is slowed by 12-24 us. The withheld doorbell already counts only predecessors without a completion record, which covers a front waiting for its stamp. Count retire_pending as settling only when the front has no record, the case of a command the firmware finished without one. In a two-queue semaphore ping-pong (gpubench sync.signal, 0.6 s traced), doorbells rung at publication fall from 2006 to 96 and releases forced by settling from 1280 to 23; all doorbells fall from 8035 to 6707. (cherry picked from commit 5de99fc) Signed-off-by: DjDeveloperr <hey@djdev.me>
…overed A doorbell announces every work slot written on its ring, so a withheld doorbell of one queue is often covered by a doorbell another queue rings. Publications of the first queue after that kept the withheld doorbell's first publication, whose predecessors had long retired: the doorbell was due at once and rang for one or two new slots. In a two-queue semaphore ping-pong almost half of all releases announced a single slot. When a publication finds that another doorbell announced every slot the withheld one covered, it starts the withheld doorbell over from itself, so the doorbell waits for this command's own predecessors. Measured on Neo with this and the previous change against the base in the same boot, alternating, three rounds (us, medians): sync.signal ping-pong doorbells 7409 -> 4625 hops slowed by more than 7 us 19.6% -> 13.9% sync.signal timeline, per hop 23.40 -> 22.83 sync.signal binary, per hop 23.35 -> 22.67 compute.light queue d64 13.89 -> 13.11 The median hop is unchanged; compute.light queue d1/d2/d3/d4, queues d1/d2/d4 and render.light d1/d32 stay within 1%. (cherry picked from commit 83ff3a9) Signed-off-by: DjDeveloperr <hey@djdev.me>
Every completion notification queues an event pass, and every pass ran the full housekeeping after retiring the records: render linger expiry, drained-pair reclaim, compute exit settlement, compute pool recycling and vacating, evicted render pool vacating, heap shrinking and teardown. Each of these scans every compute queue, render entry or pool, so with around 40000 passes a second in light rows the scans took a large part of the event worker's time on the IRQ CPU, while retiring a completion and signalling its fence needs none of them. Run that housekeeping only when a pass is requested through queue_events(), on recovery, while a teardown is pending, while anything waits for channels, progress, queue slots or render capacity, when no work remains in flight, and at the latest on the first pass 1 ms after the last full one. The firmware's event notification, records that arrive during a pass and render events missed while a pair was on loan queue a pass for the records alone. Every other caller keeps getting a full pass. Measured on Neo in the same boot against every pass running the housekeeping, alternating, four rounds (medians): event pass, sync.signal + render.light procs 8.2 -> 6.2 us event worker share of CPU0, all rows 24.9% -> 20.8% compute.light procs d2 36.80 -> 34.81 us compute.light queues d2 36.76 -> 34.96 us submit.empty queue d2 1.78 -> 1.69 us compute.light queue d1/d2/d64, queues d4, render.light d1/d32 and queues d4/d8, render.light procs d2, sync.signal binary/timeline, concurrent.cr and the cross-queue pair stay within 2%. (cherry picked from commit f769da0) Signed-off-by: DjDeveloperr <hey@djdev.me>
The event worker read a render pair's completion memory outside the device mutex. It took the pair out of its entry on loan under one hold, read the front's stamps and graph cursors unlocked, then took the mutex again to return the pair and retire what it had seen. The read is a short memory snapshot, so the loan bought no useful concurrency, and every observed pair cost two acquisitions of the mutex that submission also takes, with publishers able to slip in between. Select the pair, read its front and retire it in one hold. The pair stays in its entry, so observations no longer need the loan bookkeeping (the preparing marker and the borrowed dependency and switch snapshots) or the return path and its invariant-failure handling. Preparers keep using the loan. The read takes the context's work mutex for the shared list head, which now nests inside the device mutex; that mutex is a leaf, held only around accesses to the context's work storage. Measured on Neo across boots, with this change, without it, and with it again (3 rounds each, medians, us per submit): render.light tri queue d8 31.29 / 33.73 / 31.04 (-7.6%) render.light tri queue d1 105.49 / 105.75 / 105.53 render.light tri queue d32/d64 30.06 / 30.01 / 29.68 render.light tri procs d2 52.18 / 52.24 / 52.29 render.light tri queues d2 51.98 / 52.12 / 52.12 compute.light queue d1 103.61 / 103.73 / 104.34 sync.signal queues d2 timeline 22.74 / 22.67 / 22.69 sync.signal queues d2 binary 22.53 / 22.27 / 22.53 (cherry picked from commit cf38d3a) Signed-off-by: DjDeveloperr <hey@djdev.me>
DjDeveloperr
requested review from
chriskearney,
maralcbr and
scottjones
as code owners
October 27, 2026 05:56
maralcbr
reviewed
Oct 27, 2026
maralcbr
left a comment
maralcbr
left a comment
Member
There was a problem hiding this comment.
Thanks @DjDeveloperr, this is a careful port. The kill-before-free ordering is right, and the commit messages spell out each trade-off and its known limits, which made the series much easier to check. I read all twelve against neo/g17-neo-perf-20261025 at 578dd953: they apply cleanly and every commit keeps its -x line and sign-off, Ryan's included.
One thing before merge:
- No Kernel CI checks are reported for this head.
kernel-ci.ymlandkernel-ci-label.ymlonly trigger for pull requests intointegration/aurora(.github/workflows/kernel-ci.yml:20-22,kernel-ci-label.yml:9-11), so a PR stacked on #229's branch gets nothing, and the KUnit run in the body is the only evidence so far. It needs a manual Kernel CI run onneo/g17-aurora-merged-20261027for the package, KUnit and boot jobs. The commit, per-commit compile and device-tree checks only run on a pull request or in the merge queue (kernel-ci-jobs.yml:106,132,185), so those have to come from a new #229 run that contains these commits; its earlier green run doesn't cover them.
Separately, and not a code issue: merging this folds twelve more commits into #229 while it's still under review. Whether they ride in #229 or land as their own PR after it is a maintainer call, and I'll settle it on #229.
Smaller things:
- The render-only kill takes its admission hold before its cookie. In
runtime/compute_exit.rs:378-385, ifnext_cookie()fails withEOVERFLOW, the hold taken at:380is never released. The compute path takes the cookie first (:269); please do the same here. - Two comments no longer match the code.
runtime/render/observe.rs:11-12still says the pair needs both queues' notifications, andqueue/admission.rs:943still says a published hold lasts until both processors stop, butrelease_killed_slotsnow ends it at detach. - A failed
reserve_keptborrows another object's reservation.object.rs:692-701returns without reserving butretain_until_stopstill setskeep, so the matchingkeep_object(:705) decrements a reservation that belongs to someone else. It only ever ends on the leak side, never an unmap, so this is minor. Havingreserve_keptreturn whether it reserved, and keeping that on the object, would fix it. - Nit: 9d942b3's message calls the context work mutex a leaf, but
retain_linked_worktakes the object-pool mutex inside it (context.rs:293). I found no inversion; it's just worth correcting on the aurora side.
Follow-ups, not blocking:
- Firmware-linked retention has no bound. Linked work objects (32 KiB each,
context.rs:289-296) and kept render graphs (runtime/render/teardown.rs:144-148) stay until the firmware stops, which in practice means a crash or an unbind (mod.rs:645-653). A client that faults compute in a loop grows that by roughly 110 MiB an hour at one kill a second. The known limits in 95308a6 say this honestly, and it's far better than the base, which kept whole contexts and their pools. Could the kept graphs get a debugfs count besidefirmware_linked_objects, and a bound that keeps firmware ownership intact be tracked as a follow-up? - Please name the Neo board, both firmware versions and the module build the stress and CTS numbers came from. The body cites the aurora-side qualification only.
For release (not the merge): a Neo run of this tree, since the numbers come from aurora's module rather than this branch. The kill and retention changes also apply under the 25G83 firmware, which has no evidence yet.
| refuse_render_kill(&mut self.queues.teardown, index, RENDER_KILL_HOLD); | ||
| return Ok(()); | ||
| } | ||
| let cookie = exit::next_cookie()?; |
Member
There was a problem hiding this comment.
If this fails, the admission hold taken at :380 stays installed. Please take the cookie before the hold, as publish_compute_exit does at :269.
|
|
||
| impl crate::g17::Firmware { | ||
| fn take_render_observation(&mut self, start: usize) -> Result<Option<(usize, KBox<Pair>)>> { | ||
| /// The first slot from `start` whose pair has news to observe: published work in flight, both |
Member
There was a problem hiding this comment.
nit: selection now needs only the fragment queue's notification (completion_noted), not both.
| @@ -887,18 +941,38 @@ impl State { | |||
| impl Pool { | |||
| /// Hold the departed owner's idle pairs before publishing its exit record. | |||
| /// Successful publication retains these bits until both processors stop. | |||
Member
There was a problem hiding this comment.
nit: the hold now ends at detach through release_killed_slots, not only at processor stop.
|
|
||
| /// Room for one more kept object, taken by the matching `keep_object`. A failed | ||
| /// allocation is reported by that drop, which then leaks the object instead. | ||
| fn reserve_kept(&self) { |
Member
There was a problem hiding this comment.
When the reserve fails, keep is still set, so the matching keep_object decrements another object's reservation. Returning whether this reserved, and keeping that on the object, would fix it.
| if linked { | ||
| // Guard (kill before free): the firmware may still walk this context's work | ||
| // list into the graph, so it is not freed or recycled before the firmware stops. | ||
| kernel::pr_warn_once!("G17: keeping render graphs of an unkilled context that still links work\n"); |
Member
There was a problem hiding this comment.
These kept graphs have no debugfs count and no bound, only this warn-once. Worth a count beside firmware_linked_objects in a follow-up.
…hold A render-only kill took its admission hold on the context's render pairs and only then its cookie. If the cookie counter was exhausted, the EOVERFLOW returned with the hold still installed, so those pairs stayed held for a kill that was never published. Take the cookie first, as a failed context's kill already does. (cherry picked from commit d6d0c93) Signed-off-by: DjDeveloperr <hey@djdev.me>
retain_until_stop reserved a list entry for an object kept until firmware stop, and its drop took that entry in keep_object. When the reservation failed, keep was still set, so the drop decremented a reservation that belonged to another kept object and could take its entry. reserve_kept now reports whether it reserved, and the kept object carries that. An object kept without a reservation is leaked at drop, as the failed reservation already documented, and leaves the other objects' reservations alone. (cherry picked from commit 4e96b0a) Signed-off-by: DjDeveloperr <hey@djdev.me>
Render graphs of a detached context that still links work are kept until the firmware stops, like firmware-linked work objects, but had no counter beside firmware_linked_objects. The capacity view now reports them as firmware_linked_render_graphs, with the number leaked because the list could not grow. Two comments described the code before recent changes: render observation needs only the fragment queue's notification now, and a kill hold on render pairs ends when the killed context's scheduler release is consumed, not only when the processors stop. (cherry picked from commit 34cf645) Signed-off-by: DjDeveloperr <hey@djdev.me>
Collaborator
Author
|
Thanks for the careful read. I've pushed three commits for the code points. They are also an aurora-silicon/linux PR (aurora-silicon/linux#509, merged there), so both trees stay the same.
Where the numbers came from:
|
Collaborator
Author
|
The manual Kernel CI run on |
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. (cherry picked from commit 7a9ebf0) Signed-off-by: DjDeveloperr <hey@djdev.me>
Collaborator
Author
|
Added f17489a "drm/asahi: kill a G17 context at most once" (aurora-silicon/linux AsahiLinux#513, merged as 53205cc). Under forced faults, the compute-exit path and the render-only path could both kill the same context, a few ms apart. The second kill's acknowledgement then raced the firmware still touching the context's tiling queue. That accounted for the only recoveries in the forced-kill legs. The fix sets a per-context flag at the first published kill, and both paths check it.
Kernel CI was dispatched manually on the new head. |
Collaborator
Author
|
Kernel CI (manual dispatch) on f17489a: success (run 40480119193: plan, self-test, KUnit, package, host tests). The compile, commits and device-tree jobs are skipped on workflow_dispatch, as before. |
Collaborator
Author
|
#229's branch now has integration/aurora (30a6518) merged in (691c687; one conflict in
|
maralcbr
previously approved these changes
Oct 28, 2026
maralcbr
left a comment
maralcbr
left a comment
Member
There was a problem hiding this comment.
Thanks @DjDeveloperr. Re-checked at 58b3cf14. Everything from my earlier review is fixed, and f17489a is a good catch.
- Render-only kill cookie: fixed.
next_cookie()now runs before the admission hold (runtime/compute_exit.rs:392), and a failed publication still releases the hold. - Stale comments: fixed (
runtime/render/observe.rs:11-12,queue/admission.rs:943-944). reserve_kept: fixed. The kept object carries whether it reserved, andkeep_objectdecrements only its own reservation and leaks an unreserved object (object.rs:524-526,708-729).- Kept render graphs:
firmware_linked_render_graphs kept= leaked=is now in the capacity view (runtime/render.rs:1148). The bound stays a follow-up, as agreed. - Provenance: thanks for naming the board, firmware and module builds.
- "Leaf" wording: fine to correct in the next aurora commit that touches it.
f17489a looks right to me. Eligibility and publication run under the device mutex, both publication sites set the flag before the doorbell, and a new context starts with it clear, so a reused queue doesn't carry a stale flag.
Small things, none blocking:
runtime/render/teardown.rs:197-200:LEAKED_GRAPHSsits betweenkeep_graph's doc comment and the function, so rustdoc gives that description to the static.runtime/compute_exit.rs:408-409and:292-309:note_kill_published()runs after a fallible step that follows the irrevocable publication (record_render_kill(...)?, and.ok_or(EIO)?on the compute side). Neither can fail today, since the candidate checks rule those cases out under the same mutex. Still, setting the flag right afterpublish_controlsucceeds would keep the at-most-once rule from depending on that.- Taking the cookie first means a refused hold now uses one up. That matches the compute path, and exhaustion fails closed, so it's fine.
CI: the manual Kernel CI runs on 2ef0352, f17489a and now 58b3cf14 (run 40681399826) all passed. A dispatch skips the commit, per-commit compile and device-tree jobs, so those still need to come from #229's run once this is in it. The merge of #229's branch at 58b3cf14 leaves g17/ unchanged.
Merging this into neo/g17-neo-perf-20261025 folds the series into #229, so #229 is where it gets settled.
For release, not the merge: a Neo run of this exact tree (the numbers are from aurora's allpr2 module), and an M4 Pro run under 25G83 for the kill and retention changes.
| /// Keeps `graph` until the firmware stops; if even that cannot be recorded, it is leaked | ||
| /// rather than freed. | ||
| /// Graphs that could not be recorded in `kept_graphs` and were leaked instead, for debugfs. | ||
| pub(super) static LEAKED_GRAPHS: core::sync::atomic::AtomicU32 = core::sync::atomic::AtomicU32::new(0); |
Member
There was a problem hiding this comment.
nit: this static now sits between keep_graph's doc comment and the function, so rustdoc attaches "Keeps graph until the firmware stops…" to LEAKED_GRAPHS. Moving the static above that comment fixes it.
| // Publication is irrevocable. The held pairs stay held until this context's | ||
| // scheduler release is consumed (`release_killed_slots` at detach). | ||
| self.queues.teardown.record_render_kill(index, cookie, now)?; | ||
| context.note_kill_published(); |
Member
There was a problem hiding this comment.
Optional: setting the flag right after publish_control succeeds, before record_render_kill(...)?, keeps the at-most-once rule from depending on that call never failing. Today the candidate checks make it unreachable, so this is hardening only. Same shape at :309 after .ok_or(EIO)?.
LEAKED_GRAPHS was added between keep_graph()'s doc comment and the function, so rustdoc attached "Keeps `graph` until the firmware stops..." to the static. Move the static above that comment. Signed-off-by: DjDeveloperr <hey@djdev.me>
Both kill paths set the context's kill-published flag only after a fallible step that follows the irrevocable publication: record_render_kill(...)? on the render-only path, and the compute queue lookup's .ok_or(EIO)? on the compute path. Neither can fail today, because the candidate checks rule those cases out under the same device mutex, but the at-most-once rule should not depend on that. Set the flag as soon as publish_control() succeeds. Signed-off-by: DjDeveloperr <hey@djdev.me>
…0261027 Pick up #229's review fixes (the M3 GET_PARAMS minimum and the G17-only memory limits). No conflicts. Signed-off-by: DjDeveloperr <hey@djdev.me>
Collaborator
Author
|
Thanks. Both small things are done:
10fe909 merges #229's review fixes into this branch, with no conflicts.
The same two commits are aurora-silicon/linux#550. |
This was referenced Oct 28, 2026
drm/asahi: retire a G17 render group on its pass record when its stamps stall (stacked on #244)
#264
Open
DjDeveloperr
added a commit
that referenced
this pull request
Oct 29, 2026
chriskearney
previously approved these changes
Oct 29, 2026
chriskearney
left a comment
chriskearney
left a comment
There was a problem hiding this comment.
Reviewed current successor 071f1581 for the shared-code/M3 regression scope. Its tree f56154aff85f782215d8a38eff3db399d17700fc matches the clean merge of #244's frozen changes with #229's module-owner fix 6c66b41a; driver.rs retains the repaired owner override and assertion.
The earlier frozen #235+#229+#244 combination passed M3/G15 boot, native GL/Vulkan correctness, unchanged OMT status maps and the bounded A/B/A suite with exact restoration. Detailed timings and scope. The source-only owner repair was checked separately; this is not an updated-head or Neo/G17 hardware result. Single-submit CPU timing remains variable and fresh CI/normal merge requirements still apply.
maralcbr
changed the base branch from
neo/g17-neo-perf-20261025
to
integration/aurora
October 29, 2026 02:26
maralcbr
dismissed stale reviews from chriskearney and themself
October 29, 2026 02:26
The base branch was changed.
maralcbr
approved these changes
Oct 29, 2026
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.
Stacked on #229. This mirrors the G17 (Neo, A18 Pro) driver changes merged into aurora-silicon/linux integration/aurora on 2026-10-27. Each commit is a cherry-pick of the aurora commit (
-x), and all of them applied cleanly on #229's head. After this series the G17 driver matches aurora's except for tree-specific items: aurora's firmware TEXT identification and itsTHIS_MODULEspelling.CONFIG_KUNIT=y.Testing
On aurora, every change was qualified on Neo by its own PR: gate PERFORMANCE-REGRESSION-PASS and targeted CTS with 0 failures, plus the measurements above. The whole set was then qualified together, as all twelve aurora heads merged in one module:
drivers/gpu/drm/asahiis byte-identical to that qualified module's source.On this branch: omacom KUnit, 1713 tests: 1710 pass, 3 skip, kunit.py rc=0, verdict check rc=0.