From 925df2acdf61c4e4b21e98c35f0ffaa358eb9cd5 Mon Sep 17 00:00:00 2001 From: Jonathan Goyvaerts Date: Wed, 28 Oct 2026 11:47:53 -0300 Subject: [PATCH] drm/asahi: g17: stop cancelling behind a failed command once it has retired A queue remembered the earliest failed ordinal of each engine for good, and every later command whose prefix covered it was cancelled. GL orders each submission after every earlier one, so once one command of a queue failed, every later command of that queue was cancelled too. With cancellation no longer reported as a lost device, the client never noticed: after a CS2 GPU hang, one Hyprland render that sampled the game's failed buffer was cancelled, then every later Hyprland render (about 47 a second, all fences ECANCELED), and the screen kept the game's last frame until Hyprland or the machine restarted. New buffers did not help; the queue itself was poisoned. The record exists so that nothing runs before a failed producer the firmware may still own or replay. That holds only while the command keeps its VM pin: complete() and fail_unpublished() release it, fail_while_owned() keeps it until release_after_retirement(). Track the failed commands that still hold their pin and stop counting each one once it has released it. A failure that cannot be tracked for lack of memory keeps cancelling for good, as before. Signed-off-by: Jonathan Goyvaerts Assisted-by: LLM --- drivers/gpu/drm/asahi/g17/completion.rs | 5 ++ drivers/gpu/drm/asahi/g17/dependency.rs | 107 ++++++++++++++++++++++++ drivers/gpu/drm/asahi/g17/job.rs | 9 +- drivers/gpu/drm/asahi/g17/queue/mod.rs | 28 +++---- 4 files changed, 133 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/completion.rs b/drivers/gpu/drm/asahi/g17/completion.rs index 02e7bbc5c33dc..362cf507e1575 100644 --- a/drivers/gpu/drm/asahi/g17/completion.rs +++ b/drivers/gpu/drm/asahi/g17/completion.rs @@ -606,6 +606,11 @@ impl Completion { drop(lease); } + /// Whether the command still holds its VM pin: firmware may still own or replay it. + pub(crate) fn retains_vm(&self) -> bool { + self.vm_job.lock().is_some() + } + fn release_healthy(&self) { if let Some(sources) = &self.sources { sources.release(); } drop(self.render.lock().take()); diff --git a/drivers/gpu/drm/asahi/g17/dependency.rs b/drivers/gpu/drm/asahi/g17/dependency.rs index 0bc1d5024566e..387ce9ba2fb3c 100644 --- a/drivers/gpu/drm/asahi/g17/dependency.rs +++ b/drivers/gpu/drm/asahi/g17/dependency.rs @@ -25,6 +25,48 @@ pub(crate) fn replay_status(status: i32) -> Result { else { Ok(()) } } +/// Whether a failed command may still run or write: it holds its VM pin until it has physically +/// retired, or was never published. +pub(crate) trait Retention { + fn retained(&self) -> bool; +} + +/// The failed ordinals of one engine that later commands must not bypass. A command whose prefix +/// covers one is cancelled, so none runs before a failed producer the firmware may still replay. +/// Once that producer has retired nothing of it can run or write any more, so it stops counting: +/// otherwise one cancelled command cancelled every later command of its queue for good. +pub(crate) struct FailureRecord { + retained: KVec<(u64, T)>, + /// A failure that could not be tracked: it keeps cancelling for good, as all failures once did. + untracked: Option<(u64, T)>, +} + +impl FailureRecord { + pub(crate) const fn new() -> Self { + Self { retained: KVec::new(), untracked: None } + } + + /// A finished frontier entry, at pruning. + pub(crate) fn note(&mut self, sequence: u64, status: i32, entry: &T) { + if status >= 0 || !entry.retained() { + return; + } + if self.retained.push((sequence, entry.clone()), GFP_KERNEL).is_err() + && self.untracked.as_ref().is_none_or(|(first, _)| sequence < *first) + { + self.untracked = Some((sequence, entry.clone())); + } + } + + /// The earliest failure later prefixes must not bypass. + pub(crate) fn earliest(&mut self) -> Option<(u64, &T)> { + self.retained.retain(|(_, entry)| entry.retained()); + Iterator::chain(self.retained.iter(), self.untracked.iter()) + .min_by_key(|(sequence, _)| *sequence) + .map(|(sequence, entry)| (*sequence, entry)) + } +} + pub(crate) struct Frontier { dependencies: [KickDependency; KICK_BARRIERS_MAX], sequences: [u64; KICK_BARRIERS_MAX], @@ -89,3 +131,68 @@ impl Frontier { self.insert(sequence, dependency) } } + +#[kunit_tests(asahi_g17_dependency)] +mod tests { + use super::*; + + #[derive(Clone)] + struct Probe(kernel::sync::Arc); + + impl Probe { + fn new(retained: bool) -> Self { + Self(kernel::sync::Arc::new(core::sync::atomic::AtomicBool::new(retained), GFP_KERNEL).unwrap()) + } + fn retire(&self) { + self.0.store(false, core::sync::atomic::Ordering::Relaxed); + } + } + + impl Retention for Probe { + fn retained(&self) -> bool { + self.0.load(core::sync::atomic::Ordering::Relaxed) + } + } + + fn earliest(record: &mut FailureRecord) -> Option { + record.earliest().map(|(sequence, _)| sequence) + } + + #[test] + fn a_cancelled_command_does_not_poison_its_queue() { + // Cancelled or failed before publication: it released its pin and can never run. + let mut record = FailureRecord::new(); + record.note(5, ECANCELED.to_errno(), &Probe::new(false)); + assert_eq!(earliest(&mut record), None); + } + + #[test] + fn a_retained_failure_cancels_until_it_retires() { + let mut record = FailureRecord::new(); + let failed = Probe::new(true); + record.note(5, EIO.to_errno(), &failed); + assert_eq!(earliest(&mut record), Some(5)); + failed.retire(); + assert_eq!(earliest(&mut record), None); + } + + #[test] + fn a_later_retained_failure_survives_an_earlier_retirement() { + let mut record = FailureRecord::new(); + let (first, second) = (Probe::new(true), Probe::new(true)); + record.note(5, EIO.to_errno(), &first); + record.note(9, ECANCELED.to_errno(), &second); + assert_eq!(earliest(&mut record), Some(5)); + first.retire(); + assert_eq!(earliest(&mut record), Some(9)); + second.retire(); + assert_eq!(earliest(&mut record), None); + } + + #[test] + fn successes_are_not_recorded() { + let mut record = FailureRecord::new(); + record.note(5, 1, &Probe::new(true)); + assert_eq!(earliest(&mut record), None); + } +} diff --git a/drivers/gpu/drm/asahi/g17/job.rs b/drivers/gpu/drm/asahi/g17/job.rs index ad9adcecf0f81..0c940f74213b0 100644 --- a/drivers/gpu/drm/asahi/g17/job.rs +++ b/drivers/gpu/drm/asahi/g17/job.rs @@ -401,6 +401,13 @@ pub(crate) struct Fences { pub(crate) ready: Fence, pub(crate) completed: Fence, pub(crate) publication: Option>, + pub(crate) owner: Arc, +} + +impl super::dependency::Retention for Fences { + fn retained(&self) -> bool { + self.owner.retains_vm() + } } impl Fences { @@ -479,6 +486,6 @@ impl Scheduler { self.backend.count_backlog(&packet); } job.push(); - Ok(Fences { ready, completed, publication: host_publication }) + Ok(Fences { ready, completed, publication: host_publication, owner: packet.completion.clone() }) } } diff --git a/drivers/gpu/drm/asahi/g17/queue/mod.rs b/drivers/gpu/drm/asahi/g17/queue/mod.rs index d22c0f617f374..f6637f2858c26 100644 --- a/drivers/gpu/drm/asahi/g17/queue/mod.rs +++ b/drivers/gpu/drm/asahi/g17/queue/mod.rs @@ -10,6 +10,7 @@ use super::{ command::{Parser, UscWindow, Validated}, completion::{Completion, Destinations, PreparedPublication}, context::Context, + dependency::FailureRecord, fence::Outputs, job::{self, Backend, Engine, Fences, Order, Packet, Scheduler}, validation::Proof, @@ -98,7 +99,7 @@ pub(crate) struct Queue { /// Finished frontier entries pruned during this ioctl. Their last references /// can tear a whole completion down; that waits until the ioctl has published. finished: KVec, - failures: [Option<(u64, Fence)>; 2], + failures: [FailureRecord; 2], // Closing the userspace queue releases admission even if firmware still // retains one of its execution contexts or installed physical graphs. _slot: admission::QueueSlot, @@ -137,7 +138,7 @@ impl Queue { sequence: [0; 2], frontiers: core::array::from_fn(|_| KVec::new()), finished: KVec::new(), - failures: [None, None], + failures: [FailureRecord::new(), FailureRecord::new()], _slot: slot, }) } @@ -172,11 +173,7 @@ impl Queue { if let Some(finished) = finished.as_mut() { let _ = finished.push_within_capacity(fences.clone()); } - // One earliest failed ordinal covers every later prefix without - // retaining its packet, mappings, or every failed fence. - if status < 0 && failure.as_ref().is_none_or(|(first, _)| *sequence < *first) { - *failure = Some((*sequence, fences.completed.clone())); - } + failure.note(*sequence, status, fences); false }); } @@ -224,9 +221,9 @@ impl Queue { !Order::contains(order.wait_through[1], *sequence) || fences.ready.raw() != fences.completed.raw() || fences.status() > 0) - && self.failures.iter().enumerate().all(|(index, failure)| - failure.as_ref().is_none_or(|(sequence, _)| - !Order::contains(order.wait_through[index], *sequence))) + && self.failures.iter_mut().enumerate().all(|(index, failure)| + failure.earliest().is_none_or(|(sequence, _)| + !Order::contains(order.wait_through[index], sequence))) { self.backend.input_sources(inputs) } else { None }; if engine == Engine::Compute { super::trace::inputs(self.context.id(), order.sequence, 1, inputs.len(), sources.as_deref()); @@ -284,11 +281,12 @@ impl Queue { .iter() .filter(|(_, fences)| fences.ready.raw() == fences.completed.raw()) .count(); - let failures: [Option<&Fence>; 2] = core::array::from_fn(|index| { - self.failures[index].as_ref().and_then(|(sequence, fence)| { - Order::contains(order.wait_through[index], *sequence).then_some(fence) - }) - }); + let mut failures: [Option; 2] = [None, None]; + for (index, failure) in self.failures.iter_mut().enumerate() { + failures[index] = failure.earliest().and_then(|(sequence, fences)| { + Order::contains(order.wait_through[index], sequence).then(|| fences.completed.clone()) + }); + } let failure_count = failures.iter().flatten().count(); let mut dependencies = KVec::with_capacity(inputs.len() + count + timestamp_count + failure_count, GFP_KERNEL)?;