drm/asahi: G17 teardown review follow-ups (cookie order, kept reservations, kept-graph count)

drivers/gpu/drm/asahi/g17/object.rs

@@ -499,7 +499,9 @@ struct CacheLease {
 pub(crate) struct PooledObject {
     object: core::mem::ManuallyDrop<KernelObject>,
     lease: Option<CacheLease>,
-    keep: Option<kernel::sync::Arc<Pool>>,
+    /// The pool keeping this object until firmware stop, and whether `reserve_kept` took its
+    /// list entry.
+    keep: Option<(kernel::sync::Arc<Pool>, 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<Pool>) {
         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<CacheLease>) {
+    fn keep_object(&self, object: KernelObject, lease: Option<CacheLease>, 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,

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)
     }

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);

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()) {

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<Option<usize>> {
         if self.recovery.pending() {
             return Ok(None);

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<crate::g17::queue::render::RetiredBinding>,
     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);
     }
 }