From 078d744a260cb923d0ddfcc2b58ce8903be5a96d Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Mon, 26 Oct 2026 23:58:01 -0400 Subject: [PATCH] drm/asahi: kill faulted G17 contexts before freeing their render work 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. --- drivers/gpu/drm/asahi/g17/context.rs | 38 ++++ drivers/gpu/drm/asahi/g17/event.rs | 1 + drivers/gpu/drm/asahi/g17/kick.rs | 5 + drivers/gpu/drm/asahi/g17/object.rs | 87 ++++++++- drivers/gpu/drm/asahi/g17/queue/admission.rs | 173 ++++++++++++++++-- drivers/gpu/drm/asahi/g17/queue/compute.rs | 12 ++ .../gpu/drm/asahi/g17/queue/compute/exit.rs | 25 ++- drivers/gpu/drm/asahi/g17/queue/render.rs | 48 ++++- .../drm/asahi/g17/queue/render/retirement.rs | 28 +++ drivers/gpu/drm/asahi/g17/runtime.rs | 7 + .../gpu/drm/asahi/g17/runtime/compute_exit.rs | 138 +++++++++++++- drivers/gpu/drm/asahi/g17/runtime/render.rs | 28 ++- .../gpu/drm/asahi/g17/runtime/render/exit.rs | 13 ++ .../drm/asahi/g17/runtime/render/submit.rs | 3 +- .../drm/asahi/g17/runtime/render/teardown.rs | 35 +++- drivers/gpu/drm/asahi/g17/runtime/teardown.rs | 12 ++ drivers/gpu/drm/asahi/g17/trace.c | 6 + drivers/gpu/drm/asahi/g17/trace.h | 19 ++ drivers/gpu/drm/asahi/g17/trace.rs | 10 + drivers/gpu/drm/asahi/g17/trace_api.h | 1 + 20 files changed, 654 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/context.rs b/drivers/gpu/drm/asahi/g17/context.rs index 0d3948a2b69ff..ba74140d4f243 100644 --- a/drivers/gpu/drm/asahi/g17/context.rs +++ b/drivers/gpu/drm/asahi/g17/context.rs @@ -280,6 +280,28 @@ impl Context { _work: self.work.clone(), } } + /// Called once the firmware consumed this context's scheduler release. If its work list + /// still links a node, the firmware keeps referencing that node: on Neo a render command + /// that ran behind a failed compute command is never unlinked, and a later context given + /// the same work storage (the object cache hands it out again) made the firmware recover + /// the GPU. Keep the storage mapped and out of reuse until firmware stop; returns whether + /// it was linked. + pub(crate) fn retain_linked_work(&self, pool: &Arc) -> bool { + // An unreadable head counts as linked: retaining is the safe side. + let linked = self.work_head().map_or(true, |head| head != [0, self.work_head_va()]); + if linked { + self.work.lock().object.retain_until_stop(pool); + } + linked + } + + /// The firmware may still walk this context's work list into memory it names: a node is + /// linked and the scheduler page shows no kill. Unreadable state counts as linked. + pub(crate) fn firmware_may_link(&self) -> bool { + let killed = super::queue::compute::exit::State::read(self).map_or(0, |state| state.killed); + must_retain(self.work_head().ok(), self.work_head_va(), killed) + } + pub(crate) fn work_head_va(&self) -> u64 { self.work_base + (UAT_PGSZ + 0x400) as u64 } @@ -463,10 +485,26 @@ impl QosIds { } } +/// Memory a context's work list may lead the firmware to stays allocated: a linked node (or +/// an unreadable head) and no acknowledged kill. +fn must_retain(head: Option<[u64; 2]>, head_va: u64, killed: u32) -> bool { + killed == 0 && head.is_none_or(|head| head != [0, head_va]) +} + #[kunit_tests(asahi_g17_qos_capacity)] mod capacity_tests { use super::*; + #[test] + fn linked_unkilled_work_is_retained() { + let va = 0xfffffc20_0000_4400; + assert!(!must_retain(Some([0, va]), va, 0)); + assert!(must_retain(Some([0xfffffc20_0000_0000, 0xfffffc20_0000_0098]), va, 0)); + assert!(must_retain(None, va, 0)); + assert!(!must_retain(Some([0xfffffc20_0000_0000, 0xfffffc20_0000_0098]), va, 1)); + assert!(!must_retain(None, va, 1)); + } + #[test] fn failed_client_construction_returns_every_qos_id() { let ids = QosIds::new().unwrap(); diff --git a/drivers/gpu/drm/asahi/g17/event.rs b/drivers/gpu/drm/asahi/g17/event.rs index e726a75602b86..a175e58392379 100644 --- a/drivers/gpu/drm/asahi/g17/event.rs +++ b/drivers/gpu/drm/asahi/g17/event.rs @@ -75,6 +75,7 @@ fn batch(host: &mut H, role: Role, masks: &mut [u64; 2]) -> Result { Event::FreeListGrow(completion) => host.complete_freelist_grow(completion)?, Event::ContextKilled(cookie) => host.context_killed(cookie), Event::ComputeFault { qid, stamp } => { + crate::g17::trace::teardown(1, u64::from(qid), u64::from(stamp), 0); host.compute_fault(qid, stamp); // Its queue retires or settles its quarantine on this pass. masks[usize::from(qid) / 64] |= 1 << (qid % 64); diff --git a/drivers/gpu/drm/asahi/g17/kick.rs b/drivers/gpu/drm/asahi/g17/kick.rs index a1f8f70f597d4..b5fcd560191d7 100644 --- a/drivers/gpu/drm/asahi/g17/kick.rs +++ b/drivers/gpu/drm/asahi/g17/kick.rs @@ -145,6 +145,11 @@ impl QueueIds { self.render_ids() + (QID_COUNT - self.floor - self.leased) >= 2 * queues + 2 } + /// Whether a render pair not yet built can still reserve both of its identities. + pub(crate) fn render_pair_fits(&self) -> bool { + QID_COUNT - self.floor - self.leased >= 2 + } + /// Whether a fresh compute identity fits beside the render reserve of /// `queues` open logical queues, without reserving it. pub(crate) fn compute_fits(&self, queues: usize) -> bool { diff --git a/drivers/gpu/drm/asahi/g17/object.rs b/drivers/gpu/drm/asahi/g17/object.rs index f1b22ad5d7331..7cc13dbba705e 100644 --- a/drivers/gpu/drm/asahi/g17/object.rs +++ b/drivers/gpu/drm/asahi/g17/object.rs @@ -466,8 +466,17 @@ struct PoolInner { reserved: [usize; POOL_FAMILIES], bytes: usize, stopped: bool, + /// Objects the firmware may still reference after their owner dropped them, kept mapped + /// and out of reuse until firmware stop (see [`PooledObject::retain_until_stop`]). + kept: KVec, + kept_bytes: usize, + /// Kept objects still owned by their users, whose list entries are reserved. + kept_reserved: usize, } +/// Retained objects past this many are still retained, with one warning. +const KEPT_WARN: usize = 256; + /// Per-device cache-safe object recycling. Every converted mapping reserves /// one return slot before its page-table attributes change. Active leases and /// poisoned mappings count against the same capacity as reusable objects. @@ -490,6 +499,7 @@ struct CacheLease { pub(crate) struct PooledObject { object: core::mem::ManuallyDrop, lease: Option, + keep: Option>, } impl core::ops::Deref for PooledObject { @@ -503,11 +513,28 @@ impl core::ops::DerefMut for PooledObject { &mut self.object } } +impl PooledObject { + /// The firmware may still reference this object after its owner drops it: keep it mapped + /// and never hand it out again before firmware stop. It leaves the reuse cache, so it + /// does not take cache capacity. Call from sleepable context: this reserves the list + /// entry, so the drop, wherever it happens, does not allocate. + pub(crate) fn retain_until_stop(&mut self, pool: &kernel::sync::Arc) { + if self.keep.is_none() { + pool.reserve_kept(); + self.keep = Some(pool.clone()); + } + } +} + impl Drop for PooledObject { fn drop(&mut self) { // SAFETY: This destructor is the only operation that moves `object` // out; ManuallyDrop prevents a second implicit drop of the field. let object = unsafe { core::mem::ManuallyDrop::take(&mut self.object) }; + if let Some(pool) = self.keep.take() { + pool.keep_object(object, self.lease.take()); + return; + } match self.lease.take() { Some(lease) => lease.pool.return_object(object, &lease), None => drop(object), @@ -523,7 +550,10 @@ impl Pool { } kernel::sync::Arc::pin_init( pin_init!(Self { - inner <- kernel::new_mutex!(PoolInner { returned, reserved: [0; POOL_FAMILIES], bytes: 0, stopped: false }, "g17::object::Pool"), + inner <- kernel::new_mutex!(PoolInner { + returned, reserved: [0; POOL_FAMILIES], bytes: 0, stopped: false, kept: KVec::new(), + kept_bytes: 0, kept_reserved: 0, + }, "g17::object::Pool"), }), GFP_KERNEL, ) @@ -572,6 +602,7 @@ impl Pool { return Ok(PooledObject { object: core::mem::ManuallyDrop::new(entry.object), lease: Some(lease()), + keep: None, }); } let object = alloc.kernel(size, mmu::UAT_PGSZ as u64, prot, cpu)?; @@ -596,6 +627,7 @@ impl Pool { let mut result = PooledObject { object: core::mem::ManuallyDrop::new(object), lease: reserved.then(lease), + keep: None, }; if !reserved { return Ok(result); @@ -655,10 +687,58 @@ impl Pool { core::mem::forget(object); } + /// 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) { + let mut inner = self.inner.lock(); + let pending = inner.kept_reserved + 1; + if inner.kept.capacity() - inner.kept.len() < pending + && inner.kept.reserve(pending, GFP_KERNEL).is_err() + { + return; + } + inner.kept_reserved = pending; + } + + fn keep_object(&self, object: KernelObject, lease: Option) { + let mut inner = self.inner.lock(); + inner.kept_reserved = inner.kept_reserved.saturating_sub(1); + if let Some(lease) = &lease { + // The object leaves the reuse cache and its capacity. + inner.reserved[lease.family as usize] -= 1; + inner.bytes -= object.size(); + } + if inner.stopped { + drop(inner); + drop(object); + return; + } + let size = object.size(); + if inner.kept.len() == KEPT_WARN { + pr_warn!("G17 keeps {} firmware-linked objects until the GPU firmware restarts\n", KEPT_WARN); + } + // retain_until_stop reserved the entry; this never allocates. + match inner.kept.push_within_capacity(object) { + Ok(()) => inner.kept_bytes += size, + Err(error) => { + // Never unmap an object the firmware may still reference. + pr_err!("G17 could not record a firmware-linked object; leaking it\n"); + core::mem::forget(error.0); + } + } + } + /// Releases retained mappings after both firmware processors have stopped. /// Active objects returned later observe the same stop witness. pub(crate) fn stop_after_firmware(&self) { - self.inner.lock().stopped = true; + let kept = { + let mut inner = self.inner.lock(); + inner.stopped = true; + inner.kept_bytes = 0; + core::mem::take(&mut inner.kept) + }; + // Mapping teardown must not run while the pool mutex is held. + drop(kept); for index in 0..POOL_FAMILIES * POOL_SLOTS { let entry = { let mut inner = self.inner.lock(); @@ -684,6 +764,7 @@ impl Pool { inner.reserved[family], POOL_SLOTS, entries.iter().flatten().count(), entries.iter().flatten().filter(|entry| entry.poisoned).count())?; } - writeln!(f, "cached_object_bytes used={} total={}", inner.bytes, POOL_BYTES) + writeln!(f, "cached_object_bytes used={} total={}", inner.bytes, POOL_BYTES)?; + writeln!(f, "firmware_linked_objects kept={} bytes={}", inner.kept.len(), inner.kept_bytes) } } diff --git a/drivers/gpu/drm/asahi/g17/queue/admission.rs b/drivers/gpu/drm/asahi/g17/queue/admission.rs index e1e7552c243f2..b2083e50aef14 100644 --- a/drivers/gpu/drm/asahi/g17/queue/admission.rs +++ b/drivers/gpu/drm/asahi/g17/queue/admission.rs @@ -71,11 +71,19 @@ struct State { /// their own fence with EBUSY instead of a device-loss error, so the VM stays usable. spared: u64, kill_hold: u64, + /// Slots whose kill hold ended at their killed context's consumed scheduler release, + /// and when. A new owner takes such a pair last, the one released longest ago first. + killed: u64, + killed_at: [u64; cfg::RENDER_SLOTS], + /// The physical queue table still has room for a pair built in an unowned slot. The + /// runtime updates this whenever the table changes. + build_room: bool, released: u64, epoch: u64, footprints: KVec, next_ticket: u64, } + impl State { const MASK: u64 = (u64::MAX >> (64 - cfg::RENDER_SLOTS)) & !1; const fn new() -> Self { @@ -87,6 +95,9 @@ impl State { quarantined: 0, spared: 0, kill_hold: 0, + killed: 0, + killed_at: [0; cfg::RENDER_SLOTS], + build_room: true, released: 0, epoch: 0, footprints: KVec::new(), @@ -120,6 +131,15 @@ impl State { self.epochs[slot] = self.epoch; } fn take_idle(&mut self, slot: usize, owner: u64) -> u8 { + if self.owners[slot] != owner { + crate::g17::trace::teardown( + 6, + slot as u64, + self.owners[slot], + owner | u64::from(self.killed & (1u64 << slot) != 0) << 63, + ); + self.killed &= !(1u64 << slot); + } self.refs[slot] += 1; self.active |= 1u64 << slot; self.released &= !(1u64 << slot); @@ -165,13 +185,23 @@ impl State { } let free = !self.active & Self::MASK & !self.quarantined & !self.kill_hold; let eligible = |slot: usize| free & (1u64 << slot) != 0; - // Reuse a closed, exactly retired owner before consuming a new graph. + let released = |slot: usize| self.released & (1u64 << slot) != 0; + let unowned = || (0..cfg::RENDER_SLOTS).find(|slot| eligible(*slot) && self.owners[*slot] == 0); + // Reuse a closed, exactly retired owner before consuming a new graph. A killed + // context's pair comes after a new graph while one can still be built: on Neo, new + // owners of such pairs had renders that executed but never stored their stamps. + // Once the queue table is full it is reused, the one released longest ago first, + // so killed pairs never strand physical queues. let slot = (0..cfg::RENDER_SLOTS) - .filter(|slot| eligible(*slot) && self.released & (1u64 << slot) != 0) + .filter(|slot| eligible(*slot) && released(*slot) && self.killed & (1u64 << slot) == 0) .min_by_key(|slot| self.epochs[*slot]) + .or_else(|| if self.build_room { unowned() } else { None }) .or_else(|| { - (0..cfg::RENDER_SLOTS).find(|slot| eligible(*slot) && self.owners[*slot] == 0) - })?; + (0..cfg::RENDER_SLOTS) + .filter(|slot| eligible(*slot) && released(*slot)) + .min_by_key(|slot| self.killed_at[*slot]) + }) + .or_else(unowned)?; let first = self.take_idle(slot, owner); self.claim_sibling(owner); Some(first) @@ -180,7 +210,8 @@ impl State { if self.owners.iter().filter(|id| **id == owner).count() >= cfg::RENDER_PAIRS_PER_OWNER { return; } - let free = !self.active & Self::MASK & !self.quarantined & !self.kill_hold; + // A speculative sibling never takes a killed context's pair. + let free = !self.active & Self::MASK & !self.quarantined & !self.kill_hold & !self.killed; let eligible = |slot: usize| free & (1u64 << slot) != 0; let sibling = (0..cfg::RENDER_SLOTS) .filter(|slot| { @@ -190,6 +221,10 @@ impl State { }) .min_by_key(|slot| self.epochs[*slot]) .or_else(|| { + // Nor an unowned slot once no pair can be built there. + if !self.build_room { + return None; + } (0..cfg::RENDER_SLOTS).find(|slot| eligible(*slot) && self.owners[*slot] == 0) }); if let Some(slot) = sibling { @@ -223,6 +258,7 @@ impl State { let bit = 1u64 << slot; let old = self.owners[slot]; if old == 0 + || self.killed & bit != 0 || owner == Some(old) || expected.is_some_and(|expected| expected != old) || self.refs[slot] != 0 @@ -648,9 +684,27 @@ impl Pool { state.quarantined &= !released; state.spared &= !released; state.kill_hold = 0; + state.killed = 0; drop(state); self.returned(); } + + /// Whether the physical queue table can hold another pair (see `State::build_room`). + pub(crate) fn note_build_room(&self, room: bool) { + let mut state = self.state.lock(); + if state.build_room != room { + state.build_room = room; + drop(state); + self.returned(); + } + } + + /// A killed context's pair is free for a new owner. + pub(crate) fn killed_pair_free(&self) -> bool { + let state = self.state.lock(); + state.killed & state.released & !state.active & !state.quarantined & !state.kill_hold & State::MASK != 0 + } + } #[kunit_tests(asahi_g17_admission_accounting)] @@ -863,7 +917,7 @@ impl Drop for RenderLease { } impl State { - fn hold_for_exit_kill(&mut self, owner: u64, slots: u64) -> bool { + fn hold_for_exit_kill(&mut self, owner: u64, slots: u64, executed: u64) -> bool { if owner == 0 || slots & !Self::MASK != 0 { return false; } @@ -872,10 +926,10 @@ impl State { if slots & bit == 0 { continue; } + // A pair whose commands all ran is held with them (see `Pair::executed_for_kill`). if self.owners[slot] != owner - || self.refs[slot] != 0 - || self.active & bit != 0 - || self.released & bit == 0 + || (executed & bit == 0 + && (self.refs[slot] != 0 || self.active & bit != 0 || self.released & bit == 0)) { return false; } @@ -887,11 +941,11 @@ 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. - pub(crate) fn hold_render_slots_for_exit_kill(&self, owner: u64, slots: u64) -> bool { - self.state.lock().hold_for_exit_kill(owner, slots) + pub(crate) fn hold_render_slots_for_exit_kill(&self, owner: u64, slots: u64, executed: u64) -> bool { + self.state.lock().hold_for_exit_kill(owner, slots, executed) } - /// Undo only a hold whose control record was not published. + /// Undo a hold whose control record was not published. pub(crate) fn release_exit_kill_hold(&self, slots: u64) { if slots == 0 { return; @@ -899,6 +953,26 @@ impl Pool { self.state.lock().kill_hold &= !slots; self.returned(); } + + /// End the kill hold of `slots` once their killed context's scheduler release was + /// consumed (the firmware no longer names these pairs). A new owner takes them last. + pub(crate) fn release_killed_slots(&self, slots: u64) { + let mut state = self.state.lock(); + let held = state.kill_hold & slots & State::MASK; + if held == 0 { + return; + } + let now = crate::g17::now_ns(); + state.kill_hold &= !held; + state.killed |= held; + for slot in 0..cfg::RENDER_SLOTS { + if held & (1u64 << slot) != 0 { + state.killed_at[slot] = now; + } + } + drop(state); + self.returned(); + } } impl Pool { @@ -972,13 +1046,86 @@ mod fault_capacity_tests { let bit = 1u64 << lease.slot(); pool.release_owner(1); drop(lease); - assert!(pool.hold_render_slots_for_exit_kill(1, bit)); + assert!(pool.hold_render_slots_for_exit_kill(1, bit, 0)); assert!(!pool.reassign_unused_spare(bit.trailing_zeros() as u8, Some(1), Some(2))); // A failed (unpublished) control may roll back its hold. Published kills // keep the existing processor-stop rule; this test must not weaken it. pool.release_retained_owners_after_processor_stop(); assert_eq!(pool.state.lock().kill_hold, 0); } + + #[test] + fn consumed_scheduler_release_ends_the_kill_hold() { + let pool = Pool::new(MAX_EXECUTION_CONTEXTS).unwrap(); + let lease = pool.acquire_render(1, None, u64::MAX, || Ok(())).unwrap(); + let slot = lease.slot(); + let bit = 1u64 << slot; + pool.release_owner(1); + drop(lease); + assert!(pool.hold_render_slots_for_exit_kill(1, bit, 0)); + assert!(!pool.reassign_unused_spare(slot, Some(1), Some(2))); + pool.release_killed_slots(bit); + assert_eq!(pool.state.lock().kill_hold, 0); + assert_eq!(pool.state.lock().killed, bit); + assert!(pool.killed_pair_free()); + // The killed context's pair is never reassigned as a spare, and a new owner takes + // a new graph before it. + assert!(!pool.reassign_unused_spare(slot, Some(1), Some(2))); + let next = pool.acquire_render(2, None, u64::MAX, || Ok(())).unwrap(); + assert_ne!(next.slot(), slot); + drop(next); + pool.release_retained_owners_after_processor_stop(); + assert_eq!(pool.state.lock().killed, 0); + } + + #[test] + fn killed_pairs_are_taken_last_oldest_first() { + let pool = Pool::new(MAX_EXECUTION_CONTEXTS).unwrap(); + { + let mut state = pool.state.lock(); + for slot in 1..cfg::RENDER_SLOTS { + state.owners[slot] = 100 + slot as u64; + state.released |= 1u64 << slot; + state.killed |= 1u64 << slot; + state.killed_at[slot] = 1000 - slot as u64; + } + // One pair released without a kill. + state.killed &= !(1u64 << 7); + } + let first = pool.acquire_render(1, None, u64::MAX, || Ok(())).unwrap(); + assert_eq!(first.slot(), 7); + let second = pool.acquire_render(2, None, u64::MAX, || Ok(())).unwrap(); + assert_eq!(usize::from(second.slot()), cfg::RENDER_SLOTS - 1); + assert_eq!(pool.state.lock().killed & (1u64 << second.slot()), 0); + } + + #[test] + fn a_full_queue_table_reuses_killed_pairs_before_unowned_slots() { + let pool = Pool::new(MAX_EXECUTION_CONTEXTS).unwrap(); + { + let mut state = pool.state.lock(); + for slot in 1..8 { + state.owners[slot] = 100 + slot as u64; + state.released |= 1u64 << slot; + state.killed |= 1u64 << slot; + state.killed_at[slot] = 1000 - slot as u64; + } + } + // With room in the queue table a new owner builds a pair in an unowned slot. + let built = pool.acquire_render(1, None, u64::MAX, || Ok(())).unwrap(); + assert!(usize::from(built.slot()) >= 8); + // Without room it takes the killed pair released longest ago. + pool.note_build_room(false); + let reused = pool.acquire_render(2, None, u64::MAX, || Ok(())).unwrap(); + assert_eq!(reused.slot(), 7); + assert_eq!(pool.state.lock().killed & (1u64 << 7), 0); + // It claims no sibling: neither another killed pair nor an unowned slot. + let state = pool.state.lock(); + for slot in 1..7 { + assert_eq!(state.owners[slot], 100 + slot as u64); + } + assert_eq!(state.owners.iter().filter(|owner| **owner == 2).count(), 1); + } } impl Pool { diff --git a/drivers/gpu/drm/asahi/g17/queue/compute.rs b/drivers/gpu/drm/asahi/g17/queue/compute.rs index 3d5e12be64842..68fd03c42b42b 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute.rs @@ -566,6 +566,9 @@ pub(crate) struct Queue { replay_error: Option, replay_prepared: bool, exit: Option, + /// Since when the kill of this failed queue's context waits for other contexts' compute, + /// and whether that wait was reported. + kill_wait: Option<(u64, bool)>, installed: bool, /// Bit `p` set once a work slot of priority class `p` named this queue. outer_started: u8, @@ -691,6 +694,7 @@ impl Queue { replay_error: None, replay_prepared: false, exit: None, + kill_wait: None, installed: false, outer_started: 0, published_config: None, @@ -732,6 +736,14 @@ impl Queue { } } + /// The firmware failed a command of this quarantined queue, and the queue still waits for + /// its retirement witness: commands published after the failed one, which the firmware + /// never runs, or the failed command itself, whose ring entry it does not consume (a ray + /// traversal's, on Neo). Only a kill of the queue's context gives them back. + pub(crate) fn fault_pending(&self) -> bool { + self.fault_stamp.is_some() && self.quarantined && self.awaiting_witness && self.exit.is_none() + } + /// The stamp that proves retirement: the stored one, or a newer reported fault's. fn witness_stamp(&self) -> Result { let stored = self.graph.stamp()?; diff --git a/drivers/gpu/drm/asahi/g17/queue/compute/exit.rs b/drivers/gpu/drm/asahi/g17/queue/compute/exit.rs index d7386e335e167..ae812dd41fc12 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute/exit.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute/exit.rs @@ -83,11 +83,12 @@ impl Transaction { } impl Queue { - /// A command with ray traversal scratch is never killed. On Neo, killing a + /// A running command with ray traversal scratch is never killed. On Neo, killing a /// traversal that stopped making progress either crashed the GPU firmware /// when the queue's pool was then released, or stalled every later render /// when it was retained; left alone, the rest of the GPU kept working. Such - /// a queue stays quarantined and retained until its commands retire. + /// a queue stays quarantined and retained until its commands retire. A + /// traversal the firmware reported failed no longer runs (see `fault_pending`). fn active_traversal(&self) -> bool { self.active.iter().any(|active| { matches!( @@ -101,7 +102,7 @@ impl Queue { && self.released && self.active.len != 0 && self.exit.is_none() - && !self.active_traversal() + && (!self.active_traversal() || self.fault_pending()) && (self.quarantined || self.retired_by_teardown) && age >= if self.quarantined { @@ -124,6 +125,23 @@ impl Queue { .and_then(|active| active.ticket.map(|ticket| ticket.owner)) }) } + /// The kill of this queue's failed context waits; returns how long it has waited, and + /// whether this is the first time it crossed `slow`. + pub(crate) fn note_kill_wait(&mut self, now: u64, slow: u64) -> (u64, bool) { + let (since, warned) = self.kill_wait.get_or_insert((now, false)); + let waited = now.saturating_sub(*since); + let crossed = waited >= slow && !*warned; + *warned |= crossed; + (waited, crossed) + } + /// How long the kill of this queue's failed context has been waiting, if it is. + pub(crate) fn kill_waiting(&self, now: u64) -> Option { + self.kill_wait.map(|(since, _)| now.saturating_sub(since)) + } + /// The kill was published: how long it waited, if it did. + pub(crate) fn take_kill_wait(&mut self, now: u64) -> u64 { + self.kill_wait.take().map_or(0, |(since, _)| now.saturating_sub(since)) + } pub(crate) fn record_exit(&mut self, cookie: u64, now: u64) { self.exit = Some(Transaction { cookie, @@ -162,6 +180,7 @@ impl Queue { return false; } exit.phase = Phase::Acknowledged; + crate::g17::trace::teardown(4, cookie, u64::from(self.qid()), 0); true } pub(crate) fn settle_exit( diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index 696a1486eb0ec..bcf10f97fa2d7 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -1614,7 +1614,43 @@ impl Pair { let counters = self.free_list.counters()?; let stamps = host.stamps(self.qids())?; let (_, state) = host.epoch()?; - Ok(retirement::drained(self.ordinal, stamps, counters, state)) + Ok(retirement::drained(self.ordinal, stamps, counters, state) + || (state == 0 && self.executed_unstamped() && self.context_killed())) + } + + /// The firmware acknowledged a kill of this pair's context (scheduler page): it no longer + /// walks that context's work list, so the commands it never unlinked can be released. + fn context_killed(&self) -> bool { + crate::g17::queue::compute::exit::State::read(&self.memory.context).is_ok_and(|state| state.killed != 0) + } + + /// The pair's context may be killed although its commands were not retired: they all ran + /// (`executed_unstamped`), so the kill leaves nothing queued for the firmware to run. + /// Its memory stays until the kill is acknowledged. + pub(crate) fn executed_for_kill(&self, context: &Arc) -> bool { + Arc::ptr_eq(context, &self.memory.context) && self.executed_unstamped() + } + + /// Every command of this quarantined pair ran, its ring entries were consumed and its + /// free-list blocks returned, yet the firmware never stores the stamp of the last one. + /// On Neo this is the render work of a context whose compute command failed: it runs + /// about a second after the failure, but its stamp never arrives and it stays linked in + /// the firmware's job list, so neither the stamp proof nor the abandoned-group proof + /// ever holds and the pair would stay quarantined, keeping its context, until the + /// processors stop. + fn executed_unstamped(&self) -> bool { + self.quarantined + // Only for a closed owner: a live owner would go on publishing into a context + // whose work list still links the unstamped node. + && self.owner.is_none() + && self.prepared.is_none() + && self.active.len() != 0 + && self.free_list.counters().is_ok_and(|counters| counters[0] == counters[1]) + && self.active.iter().all(|active| { + active.outer_published + && observation::snapshot(&self.memory, active.ticket.ordinal) + .is_ok_and(retirement::Snapshot::executed_and_consumed) + }) } /// Account one witnessed tracker, preserving all completion ownership. @@ -1628,6 +1664,10 @@ impl Pair { let qids = self.qids(); let qos = self.qos_owners(); let buffer = self.buffer_id(); + // Commands the firmware never unlinked are released only after it acknowledged the + // kill of their context, on every path (see `firmware_drained`). + let unstamped = self.executed_unstamped(); + let killed = unstamped && self.context_killed(); let Some(active) = self.active.iter_mut().find(|a| !a.flist_completed) else { return Ok(false); }; @@ -1638,6 +1678,12 @@ impl Pair { }; let result = match retirement::poll(&mut active.ticket, &mut observer)? { retirement::Poll::Complete(ticks) => Ok(ticks), + // Its commands ran without a stamp (see `executed_unstamped`): they already failed + // with the quarantine, and the client sees them cancelled. + retirement::Poll::Pending | retirement::Poll::Retry if killed => { + crate::g17::trace::teardown(2, u64::from(self.slot), active.ticket.ordinal, 0); + Err(ECANCELED) + } retirement::Poll::Pending | retirement::Poll::Retry => { core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire); let snapshot = observer.snapshot(active.ticket.ordinal)?; diff --git a/drivers/gpu/drm/asahi/g17/queue/render/retirement.rs b/drivers/gpu/drm/asahi/g17/queue/render/retirement.rs index 00e4997ef3661..7996970ee369f 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render/retirement.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render/retirement.rs @@ -127,6 +127,12 @@ impl Snapshot { pub(crate) fn untouched(self) -> bool { self.pass == [0, 0] } + + /// The command ran (tiler start and fragment end written) and the firmware consumed both + /// queues' ring entries. + pub(crate) fn executed_and_consumed(self) -> bool { + self.executed() && self.cursors.iter().all(|cursors| cursors.done == cursors.write) + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -355,3 +361,25 @@ impl Tracker { .filter_map(Option::as_mut) } } + +#[kunit_tests(asahi_g17_render_retirement)] +mod executed_tests { + use super::*; + + fn snapshot(done: u32, write: u32, pass: [u64; 2]) -> Snapshot { + Snapshot { + cursors: [Cursors { done, read: done, write }; 2], + job_list_empty: false, + pass, + } + } + + #[test] + fn executed_and_consumed_needs_both_passes_and_consumed_rings() { + assert!(snapshot(3, 3, [1, 2]).executed_and_consumed()); + assert!(!snapshot(3, 3, [0, 0]).executed_and_consumed()); + assert!(!snapshot(3, 3, [1, 0]).executed_and_consumed()); + assert!(!snapshot(3, 3, [0, 2]).executed_and_consumed()); + assert!(!snapshot(2, 3, [1, 2]).executed_and_consumed()); + } +} diff --git a/drivers/gpu/drm/asahi/g17/runtime.rs b/drivers/gpu/drm/asahi/g17/runtime.rs index e04295d576606..9d7564600455e 100644 --- a/drivers/gpu/drm/asahi/g17/runtime.rs +++ b/drivers/gpu/drm/asahi/g17/runtime.rs @@ -541,6 +541,7 @@ impl Registry { .qids .reserve_for(identity, DataMaster::Compute, Some(FIRST_COMPUTE_QID), queues) .map_err(|error| self.compute_admission_error(error))?; + self.note_build_room(); let reservation = ComputeReservation { slot, id, owner }; self.compute[slot] = Some(ComputeEntry { reservation, @@ -584,6 +585,7 @@ impl Registry { self.reserved(reservation)?; self.qids.cancel_unpublished(reservation.id)?; self.compute[reservation.slot] = None; + self.note_build_room(); Ok(()) } @@ -601,6 +603,7 @@ impl Registry { if unpublished { self.qids.cancel_unpublished(entry.reservation.id)?; self.compute[slot] = None; + self.note_build_room(); } Ok(()) } @@ -2373,6 +2376,10 @@ impl Registry { writeln!(f, "qos_ids used={} total=126", self.qos_ids.used())?; writeln!(f, "metrics_ids used={} total={}", self.metrics.used(), super::fw::buffer::METRICS_SLOTS)?; self.render.write_capacity(f)?; + let now = now_ns(); + let waits = || self.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()) + .filter_map(|queue| queue.kill_waiting(now)); + compute_exit::write_kill_waits(f, waits().count(), waits().max().unwrap_or(0))?; writeln!(f, "compute_slots used={} total={} reusable={} building={}", self.compute.iter().flatten().count(), self.compute.len(), self.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()).filter(|queue| queue.reusable()).count(), diff --git a/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs b/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs index bf9d023038a51..791c1fccae6ae 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs @@ -12,9 +12,57 @@ use crate::g17::{ timeout::LIVENESS_NS, Firmware, MSG_CONTROL_NOTIFY, }; -use core::sync::atomic::{fence, Ordering}; +use core::sync::atomic::{fence, AtomicU32, AtomicU64, Ordering}; use kernel::{prelude::*, sync::Arc}; +/// Waits of failed contexts' kills for a moment without other contexts' compute in flight, +/// by bucket (under 10 ms, 100 ms, 1 s, 5 s, and longer) and the longest. +static KILL_WAITS: [AtomicU32; 5] = [const { AtomicU32::new(0) }; 5]; +static KILL_WAIT_MAX_NS: AtomicU64 = AtomicU64::new(0); +const KILL_WAIT_BOUNDS_NS: [u64; 4] = [10_000_000, 100_000_000, 1_000_000_000, 5_000_000_000]; +/// A failed context's kill waiting this long is reported, once per kill and at most once per +/// this interval across kills. +const SLOW_KILL_WAIT_NS: u64 = 5_000_000_000; +static SLOW_KILL_WARNED_NS: AtomicU64 = AtomicU64::new(0); +/// Failed contexts' kills published beside other contexts' compute under GPU memory pressure, +/// and when the last one was: at most one per [`PRESSED_KILL_INTERVAL_NS`], so the memory of +/// one is freed before another is considered. +static PRESSED_KILLS: AtomicU32 = AtomicU32::new(0); +static PRESSED_KILL_NS: AtomicU64 = AtomicU64::new(0); +const PRESSED_KILL_INTERVAL_NS: u64 = 250_000_000; + +/// GPU memory is within an eighth of the device limit. A failed context holds its memory +/// until its kill is acknowledged, so kills waiting for other contexts' compute to pause can +/// push the device to its limit, and then every client's allocations fail: on Neo, +/// gnome-shell crashed on such an ENOMEM. A kill beside foreign compute risks one GPU +/// recovery instead. +fn memory_pressed() -> bool { + let ledger = &crate::memacct::LEDGER; + let limit = ledger.limit.load(Ordering::Relaxed); + limit != 0 + && ledger + .user_committed + .load(Ordering::Relaxed) + .saturating_add(ledger.kernel_resident.load(Ordering::Relaxed)) + > limit - limit / 8 +} + +fn record_kill_wait(waited: u64) { + let bucket = KILL_WAIT_BOUNDS_NS.iter().position(|bound| waited < *bound).unwrap_or(4); + KILL_WAITS[bucket].fetch_add(1, Ordering::Relaxed); + KILL_WAIT_MAX_NS.fetch_max(waited, Ordering::Relaxed); +} + +pub(in crate::g17) fn write_kill_waits(f: &mut kernel::fmt::Formatter<'_>, waiting: usize, longest: u64) + -> kernel::fmt::Result +{ + let count = |bucket: usize| KILL_WAITS[bucket].load(Ordering::Relaxed); + writeln!(f, "failed_kill_waits under_10ms={} under_100ms={} under_1s={} under_5s={} longer={} max_ms={} memory_pressed={}", + count(0), count(1), count(2), count(3), count(4), + KILL_WAIT_MAX_NS.load(Ordering::Relaxed) / 1_000_000, PRESSED_KILLS.load(Ordering::Relaxed))?; + writeln!(f, "failed_kills waiting={} longest_wait_ms={}", waiting, longest / 1_000_000) +} + impl super::Registry { /// ACK storage belongs to the already-retained transaction, so a full event /// batch cannot overflow a separate cache. No allocation or wait is needed. @@ -53,16 +101,22 @@ impl Firmware { let _ = queue.announce_exit(now, || self.primary.notify(MSG_CONTROL_NOTIFY)); } } + // Under memory pressure one kill skips the wait, at most one per interval. + let last = PRESSED_KILL_NS.load(Ordering::Relaxed); + let mut pressed = + (last == 0 || now.saturating_sub(last) >= PRESSED_KILL_INTERVAL_NS) && memory_pressed(); for index in 0..self.queues.teardown.len() { let Some((context, age)) = self.queues.teardown.closed(index, now) else { continue; }; + // A failed context's executed pairs keep their pool leases until the kill is + // acknowledged (`Pair::firmware_drained`), so its kill cannot wait for them. if !self.queues.render.teardown_idle(&context)? - || !self.queues.render.teardown_pools_released(&context) + || (!self.compute_failed(&context) && !self.queues.render.teardown_pools_released(&context)) { continue; } - if let Err(error) = self.publish_compute_exit(&context, age, now) { + if let Err(error) = self.publish_compute_exit(&context, age, now, &mut pressed) { dev_err!( self.primary.state.shared.dev.as_ref(), "Scheduler {} exit request failed: {:?}\n", @@ -74,7 +128,7 @@ impl Firmware { Ok(()) } - fn publish_compute_exit(&mut self, context: &Arc, age: u64, now: u64) -> Result { + fn publish_compute_exit(&mut self, context: &Arc, age: u64, now: u64, pressed: &mut bool) -> Result { let Some(index) = self.queues.compute.iter().position(|entry| { entry .as_ref() @@ -83,7 +137,36 @@ impl Firmware { }) else { return Ok(()); }; - if self.render_control_backpressured() || self.kill_unsafe(context, now) { + let failed = self.compute_failed(context); + if self.render_control_backpressured() { + return Ok(()); + } + let beside = failed && self.foreign_compute_running(context, now); + if failed { + if beside && !*pressed { + let queue = self.queues.compute[index] + .as_mut() + .and_then(|entry| entry.queue.as_deref_mut()); + if let Some(mut queue) = queue { + let (waited, crossed) = queue.note_kill_wait(now, SLOW_KILL_WAIT_NS); + let last = SLOW_KILL_WARNED_NS.load(Ordering::Relaxed); + if crossed + && (last == 0 || now.saturating_sub(last) >= SLOW_KILL_WAIT_NS) + && SLOW_KILL_WARNED_NS + .compare_exchange(last, now.max(1), Ordering::Relaxed, Ordering::Relaxed) + .is_ok() + { + dev_warn!( + self.primary.state.shared.dev.as_ref(), + "Scheduler {} kill has waited {} ms for other contexts' compute to pause\n", + context.id(), + waited / 1_000_000 + ); + } + } + return Ok(()); + } + } else if self.kill_unsafe(context, now) { return Ok(()); } let (owner, target) = { @@ -122,7 +205,7 @@ impl Firmware { && !self .queues .admission - .hold_render_slots_for_exit_kill(owner, hold) + .hold_render_slots_for_exit_kill(owner, hold, self.queues.render.executed_slots(context) & hold) { return Ok(()); } @@ -131,13 +214,23 @@ impl Firmware { self.queues.admission.release_exit_kill_hold(hold); return if error == EAGAIN { Ok(()) } else { Err(error) }; } - // Publication is irrevocable even if its doorbell fails. Keep both - // the killed graph and the held render identities until processor stop. + // Publication is irrevocable even if its doorbell fails. Keep the killed + // graph until processor stop, and the held render identities until this + // context's scheduler release is consumed or processors stop. let mut queue = self.queues.compute[index] .as_mut() .and_then(|entry| entry.queue.as_deref_mut()) .ok_or(EIO)?; + if failed { + record_kill_wait(queue.take_kill_wait(now)); + } + if beside { + *pressed = false; + PRESSED_KILLS.fetch_add(1, Ordering::Relaxed); + PRESSED_KILL_NS.store(now.max(1), Ordering::Relaxed); + } queue.record_exit(cookie, now); + crate::g17::trace::teardown(3, cookie, u64::from(context.id()), hold | u64::from(beside)); fence(Ordering::SeqCst); if let Err(error) = queue.announce_exit(now, || self.primary.notify(MSG_CONTROL_NOTIFY)) { dev_warn!( @@ -150,12 +243,39 @@ impl Firmware { Ok(()) } + /// Another context's compute command is in flight. On Neo, kills of failed contexts beside + /// other clients' compute occasionally made the firmware recover the GPU (twice in ten + /// minutes of a five-lane load with 67 faults), even with nothing freed before the kill's + /// acknowledgement; such a kill takes the next moment without foreign compute instead. + /// Render work is not waited for. A GPU that retired nothing for [`LIVENESS_NS`] has + /// stopped. + fn foreign_compute_running(&self, context: &Arc, now: u64) -> bool { + let stalled = crate::g17::recovery::since_retired(now).is_none_or(|since| since >= LIVENESS_NS); + !stalled + && self.queues.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()).any(|queue| { + queue.in_flight() && !queue.context().is_some_and(|bound| Arc::ptr_eq(bound, context)) + }) + } + + /// A compute command of `context` failed and the firmware holds commands behind it that it + /// never runs. `publish_compute_exit` is only reached once `teardown_idle` found all of + /// the context's render pairs idle; the pairs behind such a failure become idle when + /// their commands have executed (`Pair::firmware_drained`), so by then nothing of the + /// context runs any more. + fn compute_failed(&self, context: &Arc) -> bool { + self.queues.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()).any(|queue| { + queue.context().is_some_and(|bound| Arc::ptr_eq(bound, context)) && queue.fault_pending() + }) + } + /// A kill while other contexts' commands run makes the firmware restart the /// GPU, and repeated ones crash it; on an otherwise idle GPU it is safe. A /// closed context's commands left alone retire like any others once the GPU /// reaches them, so its queues stay quarantined and retained until then. Only /// a GPU that retired nothing for [`LIVENESS_NS`] has stopped, and then the - /// kill proceeds. + /// kill proceeds. A context whose compute failed ([`Self::compute_failed`]) + /// is not left alone, since its held commands never retire: its kill waits + /// only for other contexts' compute ([`Self::foreign_compute_running`]). fn kill_unsafe(&self, context: &Arc, now: u64) -> bool { let stalled = crate::g17::recovery::since_retired(now).is_none_or(|since| since >= LIVENESS_NS); if stalled { diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index b5649f059f5d2..22cd1f18876ca 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -76,6 +76,9 @@ pub(super) struct RenderState { pools: pools::Pools, grow: grow::Grow, retired: KVec, + /// Graphs of released contexts the firmware may still walk (see `detach_context`): kept + /// until the firmware stops, when this state is dropped. + kept_graphs: KVec, next_pm_generation: u64, } @@ -102,6 +105,7 @@ impl RenderState { pools: pools::Pools::new()?, grow: grow::Grow::new()?, retired: KVec::with_capacity(128, GFP_KERNEL)?, + kept_graphs: KVec::new(), next_pm_generation: 1, }) } @@ -300,6 +304,7 @@ impl Registry { if let Some(pair) = entry.pair.as_ref() { self.buffers.unreserve_owner(pair.buffer_id())?; } + self.note_build_room(); } return Ok(Some(UnusedRenderSpare { _entry: entry, @@ -357,11 +362,25 @@ impl Registry { .any(|(slot, entry)| entry.is_some() && self.admission.owns_slot(owner, slot)) } + /// Tell admission whether another render pair can still be built. + pub(super) fn note_build_room(&self) { + self.admission.note_build_room(self.qids.render_pair_fits()); + } + pub(super) fn ensure_render_ids(&mut self, slot: u8, owner: u64) -> Result { if owner == 0 || slot == 0 || usize::from(slot) >= RENDER_SLOTS { return Err(EINVAL); } - if self.render.entries[usize::from(slot)].is_none() { + if self.render.entries[usize::from(slot)].is_some() { + return Ok(()); + } + let built = self.build_render_ids(slot, owner); + self.note_build_room(); + built + } + + fn build_render_ids(&mut self, slot: u8, owner: u64) -> Result { + { let identity = self.identities.reserve(2)?.start; let tiling = self.qids.reserve(identity, DataMaster::Tiling, None)?; let fragment = match self.qids.reserve(identity + 1, DataMaster::Fragment, None) { @@ -405,6 +424,13 @@ impl Registry { /// ensure_render_ids cancels a first ID if the second cannot be reserved; /// validation, identity overflow, and cancellation errors still propagate. fn prepare_render_ids(&mut self, slot: u8, owner: u64, optional: bool) -> Result { + // A full queue table refuses a sibling without scanning it on every preparation. + if optional + && self.render.entries.get(usize::from(slot)).is_some_and(Option::is_none) + && !self.qids.render_pair_fits() + { + return Ok(false); + } match self.ensure_render_ids(slot, owner) { Ok(()) => Ok(true), Err(error) if optional && error == ENOSPC => Ok(false), diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/exit.rs b/drivers/gpu/drm/asahi/g17/runtime/render/exit.rs index c47110cb76fad..25e1335c5ba13 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/exit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/exit.rs @@ -26,6 +26,19 @@ impl RenderState { (own, foreign, slots) } + /// Slots of `context`'s pairs whose commands all ran but stay unretired until its kill. + pub(in crate::g17::runtime) fn executed_slots(&self, context: &Arc) -> u64 { + let mut slots = 0; + for (slot, entry) in self.entries.iter().enumerate() { + if entry.as_ref().and_then(|entry| entry.pair.as_ref()) + .is_some_and(|pair| pair.executed_for_kill(context)) + { + slots |= 1u64 << slot; + } + } + slots + } + pub(in crate::g17::runtime) fn qids_for_slot(&self, slot: u8) -> u128 { self.entries .get(usize::from(slot)) diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs index 1ef6742b689a7..e6379178138d1 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs @@ -131,7 +131,8 @@ impl Backend { .reclaim_unused_render_spare(Some(self.owner), true)?; let ready = firmware.queues.has_render_ids(self.owner); drop(state); - if reclaimed.is_none() && !ready { + // A full queue table makes admission reuse a free killed pair instead. + if reclaimed.is_none() && !ready && !pool.killed_pair_free() { self.wait_render_capacity(&mut observed, started, sleep)?; } drop(reclaimed); diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs b/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs index e54b1f4638f58..18063f0cd6a14 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs @@ -38,7 +38,7 @@ impl RenderState { return Ok(false); } if let Some(pair) = entry.pair.as_ref() { - if !pair.idle_for_context(context)? { + if !pair.idle_for_context(context)? && !pair.executed_for_kill(context) { return Ok(false); } } @@ -123,6 +123,7 @@ impl RenderState { if !out.has_room(count, 1) { return Err(EBUSY); } + let linked = context.firmware_may_link(); for entry in self.entries.iter_mut().flatten() { if !entry .context @@ -140,7 +141,14 @@ impl RenderState { let (mut parked, retired) = KBox::into_inner(pair).park(); // This park follows the consumed release of `context`. parked.release_consumed(context.scheduler_va()); - *target = Some(retired); + 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"); + keep_graph(&mut self.kept_graphs, retired); + } else { + *target = Some(retired); + } entry.parked = Some(parked); // Detached backing retains these references until the worker runs. entry.context = None; @@ -155,8 +163,12 @@ impl RenderState { if let Some(id) = self.retired[index].buffer_id() { buffers.unreserve_owner(id)?; } - let target = out.binding_slot()?; - *target = Some(self.retired.remove(index).map_err(|_| EIO)?); + let retired = self.retired.remove(index).map_err(|_| EIO)?; + if linked { + keep_graph(&mut self.kept_graphs, retired); + } else { + *out.binding_slot()? = Some(retired); + } } Ok(()) } @@ -181,3 +193,18 @@ impl Registry { Ok(()) } } + +/// Keeps `graph` until the firmware stops; if even that cannot be recorded, it is leaked +/// rather than freed. +fn keep_graph( + kept: &mut KVec, + graph: crate::g17::queue::render::RetiredBinding, +) { + if kept.reserve(1, GFP_KERNEL).is_ok() { + if let Err(error) = kept.push_within_capacity(graph) { + core::mem::forget(error.0); + } + } else { + core::mem::forget(graph); + } +} diff --git a/drivers/gpu/drm/asahi/g17/runtime/teardown.rs b/drivers/gpu/drm/asahi/g17/runtime/teardown.rs index 16bd5828265a4..0222ae4c478c8 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/teardown.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/teardown.rs @@ -100,6 +100,8 @@ impl ReclaimBatch { struct Service<'a> { render: &'a mut super::render::RenderState, + admission: &'a crate::g17::queue::admission::Pool, + objects: &'a Arc, compute: &'a [Option], buffers: &'a mut BufferIds, init: &'a InitData, @@ -137,8 +139,15 @@ impl Host for Service<'_> { self.primary.notify(MSG_CONTROL_NOTIFY) } fn detach(&mut self, context: &Arc) -> Result { + let linked = context.retain_linked_work(self.objects); + let (_, _, slots) = self.render.exit_scope(context); + crate::g17::trace::teardown(5, u64::from(context.id()), slots, u64::from(linked)); self.render .detach_context(context, self.buffers, self.reclaim)?; + // The firmware consumed this context's scheduler release after its kill (if any) + // was acknowledged and its pairs went idle: it no longer names these pairs, so + // their kill hold ends here rather than at the next processor stop. + self.admission.release_killed_slots(slots); // detach_context preflights this slot together with every graph slot. // Retain it before Pending removes its reference under the device lock. self.reclaim.context(context) @@ -173,6 +182,7 @@ impl Firmware { self.queues.render.observe_teardown_releases(&self.init)?; let recovery = self.recovery.pending() || self.init.recovery_state()? != 0; let Registry { + admission, render, compute, buffers, @@ -181,6 +191,8 @@ impl Firmware { } = &mut self.queues; let mut service = Service { render, + admission, + objects: &self.pool, compute, buffers, init: &self.init, diff --git a/drivers/gpu/drm/asahi/g17/trace.c b/drivers/gpu/drm/asahi/g17/trace.c index 23666be389b96..9e93291f5114e 100644 --- a/drivers/gpu/drm/asahi/g17/trace.c +++ b/drivers/gpu/drm/asahi/g17/trace.c @@ -54,3 +54,9 @@ void asahi_g17_trace_event(u8 phase) if (trace_asahi_g17_event_enabled()) trace_asahi_g17_event(phase, __arch_counter_get_cntvct()); } + +void asahi_g17_trace_teardown(u8 kind, u64 a, u64 b, u64 c) +{ + if (trace_asahi_g17_teardown_enabled()) + trace_asahi_g17_teardown(kind, a, b, c); +} diff --git a/drivers/gpu/drm/asahi/g17/trace.h b/drivers/gpu/drm/asahi/g17/trace.h index 7e5a50e774b59..d1529a2775fcc 100644 --- a/drivers/gpu/drm/asahi/g17/trace.h +++ b/drivers/gpu/drm/asahi/g17/trace.h @@ -125,6 +125,25 @@ TRACE_EVENT(asahi_g17_message, __entry->counter) ); +/* Failed-context teardown and render slot reuse (kind: see g17/trace.rs teardown()). */ +TRACE_EVENT(asahi_g17_teardown, + TP_PROTO(u8 kind, u64 a, u64 b, u64 c), + TP_ARGS(kind, a, b, c), + TP_STRUCT__entry( + __field(u8, kind) + __field(u64, a) + __field(u64, b) + __field(u64, c) + ), + TP_fast_assign( + __entry->kind = kind; + __entry->a = a; + __entry->b = b; + __entry->c = c; + ), + TP_printk("kind=%u a=%#llx b=%#llx c=%#llx", __entry->kind, __entry->a, __entry->b, __entry->c) +); + TRACE_EVENT(asahi_g17_event, TP_PROTO(u8 phase, u64 counter), TP_ARGS(phase, counter), diff --git a/drivers/gpu/drm/asahi/g17/trace.rs b/drivers/gpu/drm/asahi/g17/trace.rs index b76b46425d2ce..f498de82e87c6 100644 --- a/drivers/gpu/drm/asahi/g17/trace.rs +++ b/drivers/gpu/drm/asahi/g17/trace.rs @@ -16,6 +16,7 @@ unsafe extern "C" { fn asahi_g17_trace_doorbell_enabled() -> bool; fn asahi_g17_trace_doorbell(engine: u8, power: u32); fn asahi_g17_trace_event(phase: u8); + fn asahi_g17_trace_teardown(kind: u8, a: u64, b: u64, c: u64); } pub(super) const RENDER: u8 = 0; @@ -74,6 +75,15 @@ pub(super) fn message(kind: u8, value: u64) { unsafe { asahi_g17_trace_message(kind, value) }; } +/// Failed-context teardown: 1 compute fault (qid, stamp), 2 unstamped render retire (slot, +/// ordinal), 3 kill published (cookie, context, held slots | 1 if it skipped the wait for other +/// contexts' compute under memory pressure; slot 0 is never held), 4 kill acknowledged (cookie, qid), +/// 5 detach (context, slots, linked), 6 render slot to a new owner (slot, old, new | killed << 63). +pub(crate) fn teardown(kind: u8, a: u64, b: u64, c: u64) { + // SAFETY: Plain values only. + unsafe { asahi_g17_trace_teardown(kind, a, b, c) }; +} + /// Event worker pass boundaries: 0 begin, 1 fences signalled, 2 end. pub(super) fn event(phase: u8) { // SAFETY: Plain values only. diff --git a/drivers/gpu/drm/asahi/g17/trace_api.h b/drivers/gpu/drm/asahi/g17/trace_api.h index d09f529a163f0..0049aa0079634 100644 --- a/drivers/gpu/drm/asahi/g17/trace_api.h +++ b/drivers/gpu/drm/asahi/g17/trace_api.h @@ -20,5 +20,6 @@ bool asahi_g17_trace_doorbell_enabled(void); void asahi_g17_trace_doorbell(u8 engine, u32 power); void asahi_g17_trace_message(u8 kind, u64 value); void asahi_g17_trace_event(u8 phase); +void asahi_g17_trace_teardown(u8 kind, u64 a, u64 b, u64 c); #endif /* _ASAHI_G17_TRACE_API_H */