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, 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/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); 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); } }