From d6d0c93e679fdb31a290b6d3ffa1fbd334411836 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 27 Oct 2026 02:19:20 -0400 Subject: [PATCH 1/3] drm/asahi: take a G17 render-only kill's cookie before its admission 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. --- drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs b/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs index c8e6de387d91c..cd7f2999d0312 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs @@ -374,6 +374,9 @@ impl Firmware { refuse_render_kill(&mut self.queues.teardown, index, RENDER_KILL_OWNER); return Ok(()); }; + // As for a failed context's kill: the cookie first, so that running out of cookies + // leaves no admission hold behind. + let cookie = exit::next_cookie()?; if !self .queues .admission @@ -382,7 +385,6 @@ impl Firmware { refuse_render_kill(&mut self.queues.teardown, index, RENDER_KILL_HOLD); return Ok(()); } - let cookie = exit::next_cookie()?; let record = ControlRecord::ContextKill(ContextKill::new(context.scheduler_va(), cookie)); if let Err(error) = Rings::publish_control(&self.init, &record) { self.queues.admission.release_exit_kill_hold(hold); From 4e96b0a44f45bde2f870f812a4057c0324923d9c Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 27 Oct 2026 02:19:20 -0400 Subject: [PATCH 2/3] drm/asahi: keep a G17 kept object's list reservation with the object 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. --- drivers/gpu/drm/asahi/g17/object.rs | 33 +++++++++++++++++++---------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/object.rs b/drivers/gpu/drm/asahi/g17/object.rs index 7cc13dbba705e..87fdb0e8a62ad 100644 --- a/drivers/gpu/drm/asahi/g17/object.rs +++ b/drivers/gpu/drm/asahi/g17/object.rs @@ -499,7 +499,9 @@ struct CacheLease { pub(crate) struct PooledObject { object: core::mem::ManuallyDrop, lease: Option, - keep: Option>, + /// The pool keeping this object until firmware stop, and whether `reserve_kept` took its + /// list entry. + keep: Option<(kernel::sync::Arc, bool)>, } impl core::ops::Deref for PooledObject { @@ -520,8 +522,8 @@ impl PooledObject { /// 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()); + let reserved = pool.reserve_kept(); + self.keep = Some((pool.clone(), reserved)); } } } @@ -531,8 +533,8 @@ impl Drop for PooledObject { // 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()); + if let Some((pool, reserved)) = self.keep.take() { + pool.keep_object(object, self.lease.take(), reserved); return; } match self.lease.take() { @@ -687,22 +689,25 @@ 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) { + /// Room for one more kept object, taken by the matching `keep_object`. Returns whether + /// it was reserved; after a failed allocation that drop leaks the object instead. + fn reserve_kept(&self) -> bool { 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; + return false; } inner.kept_reserved = pending; + true } - fn keep_object(&self, object: KernelObject, lease: Option) { + fn keep_object(&self, object: KernelObject, lease: Option, reserved: bool) { let mut inner = self.inner.lock(); - inner.kept_reserved = inner.kept_reserved.saturating_sub(1); + if reserved { + 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; @@ -717,6 +722,12 @@ impl Pool { if inner.kept.len() == KEPT_WARN { pr_warn!("G17 keeps {} firmware-linked objects until the GPU firmware restarts\n", KEPT_WARN); } + if !reserved { + // Another kept object's reservation must not be used: leak this one instead. + pr_err!("G17 could not record a firmware-linked object; leaking it\n"); + core::mem::forget(object); + return; + } // retain_until_stop reserved the entry; this never allocates. match inner.kept.push_within_capacity(object) { Ok(()) => inner.kept_bytes += size, From 34cf6456523e31a08a1f9c4ecb1184533bc9dfbb Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 27 Oct 2026 02:19:20 -0400 Subject: [PATCH 3/3] drm/asahi: count kept G17 render graphs and fix two stale comments 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. --- drivers/gpu/drm/asahi/g17/queue/admission.rs | 3 ++- drivers/gpu/drm/asahi/g17/runtime/render.rs | 2 ++ drivers/gpu/drm/asahi/g17/runtime/render/observe.rs | 4 ++-- drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs | 5 +++++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/admission.rs b/drivers/gpu/drm/asahi/g17/queue/admission.rs index 5c657444be01f..ed73323e34e6f 100644 --- a/drivers/gpu/drm/asahi/g17/queue/admission.rs +++ b/drivers/gpu/drm/asahi/g17/queue/admission.rs @@ -940,7 +940,8 @@ 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. + /// Successful publication retains these bits until the owner's scheduler release is + /// consumed (`release_killed_slots` at detach), or until both processors stop. 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) } diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index b62f677b014f0..4e7eaa2d8d246 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -1145,6 +1145,8 @@ impl Registry { impl RenderState { pub(super) fn write_capacity(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result { writeln!(f, "retired_render_bindings used={} total={}", self.retired.len(), self.retired.capacity())?; + writeln!(f, "firmware_linked_render_graphs kept={} leaked={}", self.kept_graphs.len(), + teardown::LEAKED_GRAPHS.load(core::sync::atomic::Ordering::Relaxed))?; self.pools.write_capacity(f)?; for (slot, entry) in self.entries.iter().enumerate() { if let Some(context) = entry.as_ref().and_then(|entry| entry.context.as_ref()) { diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs b/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs index b499ae5132c18..51682b43728c0 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs @@ -8,8 +8,8 @@ use core::sync::atomic::Ordering; use kernel::{prelude::*, sync::Arc}; impl crate::g17::Firmware { - /// The first slot from `start` whose pair has news to observe: published work in flight, both - /// queues' notifications seen, and an observation the pair accepts. + /// The first slot from `start` whose pair has news to observe: published work in flight, the + /// fragment queue's notification seen (`completion_noted`), and an observation the pair accepts. fn next_render_observation(&self, start: usize) -> Result> { if self.recovery.pending() { return Ok(None); diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs b/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs index 18063f0cd6a14..8f3ee6f3b15bd 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/teardown.rs @@ -196,15 +196,20 @@ impl Registry { /// 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); + 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) { + LEAKED_GRAPHS.fetch_add(1, core::sync::atomic::Ordering::Relaxed); core::mem::forget(error.0); } } else { + LEAKED_GRAPHS.fetch_add(1, core::sync::atomic::Ordering::Relaxed); core::mem::forget(graph); } }