From 94e719a0cec2a65502ce5413d2ee9a105c1cc8d9 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:32:11 +1100 Subject: [PATCH 1/7] drm/asahi: g17: never replay a render group a recovery blamed settle_blamed_render() keeps the firmware-blamed queue's oldest group spared when an earlier recovery spared it, so that its failure stays non-sticky. fail_render_packet() then tries replay_abandoned(), which counted the group as innocent because it was not classified guilty. A group aborted by a recovery carries the same abandonment witness as an innocent group the firmware dropped, so the blamed work was published again, up to MAX_REPLAYS times, and each replay could set off another recovery. The class cannot carry the blame on its own: the earlier recovery's spare has to stand, and quarantine() can later extend a spared class to the rest of the pair. Record the blame on the completion instead, apart from the class and never cleared, set it in settle_blamed_render() whatever class the group keeps, and make the replay decision refuse any set that holds a blamed group. The error class is unchanged, so blamed work an earlier recovery spared still fails without a sticky device loss. KUnit covers a blamed group at the front, behind an innocent group and at the replay limit, where an innocent front alone would be replayed. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/completion.rs | 14 ++++++++++++ drivers/gpu/drm/asahi/g17/queue/render.rs | 3 +++ drivers/gpu/drm/asahi/g17/replay.rs | 22 +++++++++++++++---- .../drm/asahi/g17/runtime/render/settle.rs | 4 +++- 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/completion.rs b/drivers/gpu/drm/asahi/g17/completion.rs index 36c46a3823bdf..4f5b87bc853dc 100644 --- a/drivers/gpu/drm/asahi/g17/completion.rs +++ b/drivers/gpu/drm/asahi/g17/completion.rs @@ -264,6 +264,9 @@ pub(crate) struct Completion { timeout_error: AtomicI32, replays: AtomicU8, replay_timeout: AtomicBool, + /// A recovery blamed this command. Unlike `class`, which a later + /// classification can change, this never clears. + blamed: AtomicBool, firmware_consumer: AtomicBool, class: AtomicU8, } @@ -318,6 +321,7 @@ impl Completion { timeout_error: AtomicI32::new(0), replays: AtomicU8::new(0), replay_timeout: AtomicBool::new(false), + blamed: AtomicBool::new(false), firmware_consumer: AtomicBool::new(false), class: AtomicU8::new(0), }), @@ -471,6 +475,16 @@ impl Completion { self.class.load(Ordering::Acquire) == 2 } + /// A recovery blamed this command, whatever class it keeps: it may have caused the fault, + /// so it is never published again. + pub(crate) fn note_blamed(&self) { + self.blamed.store(true, Ordering::Release); + } + + pub(crate) fn blamed(&self) -> bool { + self.blamed.load(Ordering::Acquire) + } + /// Installed before the first firmware-visible prefix. Cancellation takes /// the same lock after claiming the signal, so it cannot miss a late lease. pub(crate) fn attach_render_lease( diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index e1d2c64f6c349..019bfe6348cfe 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -1223,10 +1223,12 @@ impl Pair { let groups = self.active.iter().map(|active| { let innocent = active.outer_published && !active.failed && !active.packet.completion.guilty(); + let blamed = active.packet.completion.blamed(); let exhausted = active.packet.completion.replay_count() >= crate::g17::completion::MAX_REPLAYS; // decide() stops at the first group that is not abandoned, an error included. let abandoned = innocent + && !blamed && (|| -> Result { let mut observer = Observer { memory, @@ -1249,6 +1251,7 @@ impl Pair { }); Group { innocent, + blamed, abandoned, exhausted, } diff --git a/drivers/gpu/drm/asahi/g17/replay.rs b/drivers/gpu/drm/asahi/g17/replay.rs index 563c3d3f40080..f79f9f3c091ec 100644 --- a/drivers/gpu/drm/asahi/g17/replay.rs +++ b/drivers/gpu/drm/asahi/g17/replay.rs @@ -7,8 +7,11 @@ /// One retained group, front first. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct Group { - /// Published, not failed and not blamed (guilty) by a recovery. + /// Published, not failed and not classified guilty by a recovery. pub(crate) innocent: bool, + /// A recovery blamed it, whatever class it kept (an earlier recovery may have spared it): + /// it may have caused the fault. + pub(crate) blamed: bool, /// Carries the strict abandonment witness. pub(crate) abandoned: bool, /// Already republished the maximum number of times. @@ -31,7 +34,7 @@ pub(crate) fn decide(groups: impl Iterator) -> Decision { let mut front_exhausted = None; let mut successor_exhausted = false; for group in groups { - if !group.innocent { + if !group.innocent || group.blamed { return Decision::Refuse; } if !group.abandoned { @@ -53,9 +56,10 @@ pub(crate) fn decide(groups: impl Iterator) -> Decision { mod tests { use super::*; fn group(innocent: bool, abandoned: bool, exhausted: bool) -> Group { - Group { innocent, abandoned, exhausted } + Group { innocent, blamed: false, abandoned, exhausted } } - const OK: Group = Group { innocent: true, abandoned: true, exhausted: false }; + const OK: Group = Group { innocent: true, blamed: false, abandoned: true, exhausted: false }; + const BLAMED: Group = Group { innocent: true, blamed: true, abandoned: true, exhausted: false }; #[test] fn every_group_abandoned_and_innocent_is_replayed() { assert_eq!(decide([OK].into_iter()), Decision::Replay); @@ -71,6 +75,16 @@ mod tests { assert_eq!(decide([OK, group(false, true, false)].into_iter()), Decision::Refuse); } #[test] + fn blamed_work_is_never_replayed_even_when_spared() { + // An earlier recovery spared it, so it still counts as innocent; the blame refuses it. + assert_eq!(decide([BLAMED].into_iter()), Decision::Refuse); + assert_eq!(decide([BLAMED, OK].into_iter()), Decision::Refuse); + assert_eq!(decide([OK, BLAMED].into_iter()), Decision::Refuse); + // Also once the replay limit is reached, where an innocent front alone is replayed. + let exhausted = Group { exhausted: true, ..BLAMED }; + assert_eq!(decide([exhausted].into_iter()), Decision::Refuse); + } + #[test] fn a_live_group_waits_and_stops_the_walk() { assert_eq!(decide([OK, group(true, false, false)].into_iter()), Decision::Wait); // Nothing after the live group is looked at, guilty or not. diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/settle.rs b/drivers/gpu/drm/asahi/g17/runtime/render/settle.rs index d93e701b8b5cb..24c724cb32470 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/settle.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/settle.rs @@ -127,7 +127,9 @@ impl crate::g17::Firmware { if let Some(packet) = packet { // Unclassified work counts as innocent, and a blamed group must never be replayed. // Work an earlier recovery spared is collateral here (classify() blames before its - // collateral check) and stays spared. + // collateral check) and stays spared, so its failure stays non-sticky; the blame + // itself is recorded apart from the class, and refuses any replay of it. + packet.completion.note_blamed(); if !packet.completion.spared() { packet.completion.classify(crate::g17::recovery::Class::Guilty); } From 1054f6763014fe4b4032a08ae7ccce721be4d385 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:32:58 +1100 Subject: [PATCH 2/7] drm/asahi: g17: fix the render timeout head and replay documentation Pair::timeout_head() had two first lines of documentation, an older one and its replacement; keep one. replay_abandoned() said the firmware drops an innocent pair's queued groups unexecuted after another VM's recovery on 25G83. The same was seen on the A18 Pro's 26A428 firmware when the replay was first added (b9fd334a9c96, "drm/asahi: g17: replay render groups firmware retired without executing"), so name both. Also note what the single witness sample of the timeout and settlement paths can cost: a sample can straddle a firmware update of the fragment end record, and a group that did execute then runs a second time. The behaviour is unchanged. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/queue/render.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index 019bfe6348cfe..1ad0809c1d6f2 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -966,9 +966,9 @@ impl Pair { .all(|qid| masks[usize::from(qid) / 64] & (1u64 << (qid % 64)) != 0) }) } - /// The oldest published group, for cross-VM timeout attribution (crate::g17::timeout). - /// The oldest published group still running. A quarantined pair's work holds the GPU until - /// the firmware drains it, so it stays visible to other VMs' watchdogs, as compute does. + /// The oldest published group still running, for cross-VM timeout attribution + /// (crate::g17::timeout). A quarantined pair's work holds the GPU until the firmware drains + /// it, so it stays visible to other VMs' watchdogs, as compute does. pub(crate) fn timeout_head(&self) -> Option { self.active .iter() @@ -1199,13 +1199,16 @@ impl Pair { /// Every retained group was retired and unlinked by firmware without /// executing (job list empty, stamps cover it, no recovery, fragment end - /// never written): after another VM's recovery the 25G83 firmware drops an - /// innocent pair's queued groups this way. Nothing of them reached their - /// attachments, so the same commands are published again, in order, instead - /// of failing their owners. Only the whole set is replayed; a replayed prefix - /// would run after a live successor. `grace` asks for the strict witness - /// twice ABANDON_GRACE_NS apart (worker polling); the render timeout, long - /// after publication, takes it once as its retirement does. + /// never written): after another VM's recovery the firmware drops an + /// innocent pair's queued groups this way (seen on 25G83 and 26A428). + /// Nothing of them reached their attachments, so the same commands are + /// published again, in order, instead of failing their owners. Only the + /// whole set is replayed; a replayed prefix would run after a live + /// successor. `grace` asks for the strict witness twice ABANDON_GRACE_NS + /// apart (worker polling); the render timeout, long after publication, + /// takes it once as its retirement does. A single sample can still + /// straddle a firmware update of the fragment end record; the cost is + /// that a group which did execute runs a second time (blending twice). fn replay_abandoned( &mut self, host: &mut impl Host, From 496a59f8960191dc511b92328b13ccc7e31ad1ff Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:34:09 +1100 Subject: [PATCH 3/7] drm/asahi: g17: do not spare a timeout behind work that already failed classify() assumes the culprit is the oldest unfinished work. That does not hold when an innocent frame F was published before the culprit C and stalls behind it: F times out first and, being oldest, fails as its own fault. F's quarantined pair keeps its timeout head until the firmware drains it, so C then defers MAX_RENEWALS times, about 16 s, and is classified Spared: its failure is not sticky and its client is not torn down. Before render timeouts had a grace, C failed after about 2 s. Every compositor has a render head, so on the A18 Pro this covers most render and compute hangs, and a quarantined pair that never drains does the same to every later timeout. Mark a head whose queue has already failed (a quarantined render pair, or a quarantined compute queue still awaiting its witness), and split classify() into blocker(), which prefers a live blocker to a failed one, and decide(). A timeout behind another VM's older work still defers for the whole grace, so that a failed culprit's teardown can free the GPU. Once the grace runs out it is spared only behind a live blocker, which its own watchdog still settles; behind failed work only it is its own. This also applies to compute, which weighs render heads and its own failed queues the same way: compute work still blocked after the grace behind queues that already failed now fails as its own instead of being spared, as a render did before the grace existed. KUnit covers a failed older blocker (defer, then own, at any renewal count past the grace), a pair that never drains, a live blocker next to failed ones, and the blocker filter. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/queue/compute.rs | 1 + drivers/gpu/drm/asahi/g17/queue/render.rs | 3 +- drivers/gpu/drm/asahi/g17/timeout.rs | 72 +++++++++++++++++++--- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/compute.rs b/drivers/gpu/drm/asahi/g17/queue/compute.rs index 9ccdd703106b7..ce6b376dcd9e1 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute.rs @@ -621,6 +621,7 @@ impl Queue { vm: Arc::as_ptr(active.packet.completion.status()) as usize, published: active.published, finished: active.observed.is_some(), + failed: self.quarantined, }) } diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index 1ad0809c1d6f2..4fa4e0f6a6c83 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -968,7 +968,7 @@ impl Pair { } /// The oldest published group still running, for cross-VM timeout attribution /// (crate::g17::timeout). A quarantined pair's work holds the GPU until the firmware drains - /// it, so it stays visible to other VMs' watchdogs, as compute does. + /// it, so it stays visible to other VMs' watchdogs, as compute does, marked as failed. pub(crate) fn timeout_head(&self) -> Option { self.active .iter() @@ -978,6 +978,7 @@ impl Pair { vm: Arc::as_ptr(active.packet.completion.status()) as usize, published: active.published, finished: false, + failed: self.quarantined, }) } diff --git a/drivers/gpu/drm/asahi/g17/timeout.rs b/drivers/gpu/drm/asahi/g17/timeout.rs index 5dc04ef5d09a0..99e163c455ff8 100644 --- a/drivers/gpu/drm/asahi/g17/timeout.rs +++ b/drivers/gpu/drm/asahi/g17/timeout.rs @@ -58,6 +58,9 @@ pub(crate) struct Head { pub(crate) vm: usize, pub(crate) published: u64, pub(crate) finished: bool, + /// Its queue already failed (quarantined) and holds the GPU only until the firmware drains + /// it. No later watchdog of its own will settle it. + pub(crate) failed: bool, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -67,18 +70,34 @@ pub(crate) enum Decision { Spare, } -pub(crate) fn classify(own: Head, renewals: u8, others: impl Iterator) -> Decision { - if own.finished || !others.into_iter().any(|other| { - other.vm != own.vm && !other.finished && other.published < own.published - }) { - Decision::Own - } else if renewals < MAX_RENEWALS { - Decision::Defer - } else { - Decision::Spare +/// Another VM's older unfinished work that can block `own`: live work before work whose queue +/// already failed, the oldest of each first. +pub(crate) fn blocker(own: Head, others: impl Iterator) -> Option { + others + .filter(|other| other.vm != own.vm && !other.finished && other.published < own.published) + .min_by_key(|other| (other.failed, other.published)) +} + +/// Work behind another VM's older unfinished work is renewed up to MAX_RENEWALS times, so that +/// the blocker's own failure and teardown can free the GPU. After that it is spared only behind +/// a live blocker, which its own watchdog still settles. A blocker that already failed waits only +/// for the firmware to drain it and no longer excuses later work: otherwise an innocent frame +/// published before the culprit, which timed out first, would spare the culprit, and a pair that +/// never drains would spare every later timeout. +pub(crate) fn decide(own: Head, renewals: u8, blocker: Option) -> Decision { + match blocker { + _ if own.finished => Decision::Own, + None => Decision::Own, + Some(_) if renewals < MAX_RENEWALS => Decision::Defer, + Some(blocker) if blocker.failed => Decision::Own, + Some(_) => Decision::Spare, } } +pub(crate) fn classify(own: Head, renewals: u8, others: impl Iterator) -> Decision { + decide(own, renewals, blocker(own, others)) +} + #[cfg_attr(not(test), kernel::macros::kunit_tests(asahi_g17_execution_allowance))] mod tests { use super::*; @@ -132,7 +151,10 @@ mod tests { assert!(background_stalled(Some(since), u64::MAX)); } fn head(vm: usize, published: u64, finished: bool) -> Head { - Head { vm, published, finished } + Head { vm, published, finished, failed: false } + } + fn failed(vm: usize, published: u64) -> Head { + Head { vm, published, finished: false, failed: true } } #[test] fn ray_query_execution_is_bounded_at_ten_seconds() { @@ -190,6 +212,36 @@ mod tests { assert_eq!(classify(head(7, 100, false), 0, [head(5, 101, false)].into_iter()), Decision::Own); } #[test] + fn a_blocker_that_already_failed_defers_then_never_spares() { + // An innocent frame published before the culprit timed out first and failed; its + // quarantined pair keeps its head until the firmware drains it. + let culprit = head(7, 101, false); + for count in 0..MAX_RENEWALS { + assert_eq!(classify(culprit, count, [failed(5, 100)].into_iter()), Decision::Defer); + } + assert_eq!(classify(culprit, MAX_RENEWALS, [failed(5, 100)].into_iter()), Decision::Own); + assert_eq!(classify(culprit, u8::MAX, [failed(5, 100)].into_iter()), Decision::Own); + // A pair that never drains does not spare any later timeout either. + let drained_never = [failed(5, 100), failed(5, 102)]; + for own in [head(7, 105, false), head(9, 200, false)] { + assert_eq!(classify(own, MAX_RENEWALS, drained_never.into_iter()), Decision::Own); + } + } + #[test] + fn a_live_blocker_still_spares_beside_a_failed_one() { + let own = head(9, 103, false); + let others = [failed(5, 100), head(7, 101, false), failed(8, 102)]; + assert_eq!(blocker(own, others.into_iter()), Some(head(7, 101, false))); + assert_eq!(classify(own, MAX_RENEWALS, others.into_iter()), Decision::Spare); + // Failed or not, a blocker must be older, unfinished and of another VM. + let late = [failed(5, 104), failed(9, 100), Head { finished: true, ..failed(5, 100) }]; + assert_eq!(blocker(own, late.into_iter()), None); + assert_eq!(classify(own, 0, late.into_iter()), Decision::Own); + // Among failed blockers the oldest is reported. + let both_failed = [failed(8, 102), failed(5, 100)]; + assert_eq!(blocker(own, both_failed.into_iter()), Some(failed(5, 100))); + } + #[test] fn same_vm_finished_and_later_work_do_not_block() { let own = head(5, 101, false); assert_eq!(classify(own, 0, [head(5, 100, false), head(7, 100, true), head(7, 101, false), head(7, 102, false)].into_iter()), Decision::Own); From fc1a16b4722ce9a9eb6b77fc0af4163873cacc9e Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:35:57 +1100 Subject: [PATCH 4/7] drm/asahi: g17: name the blocking work in the watchdog lines The render and compute watchdog lines said a timeout was "behind earlier work of another VM" without naming that work, and the render line also lacked the publication= field the compute line has. Neither showed whether a decision rested on a live blocker or on one that had already failed, which is what the decision now depends on. Return the blocker with the decision (timeout::attribute()) and print the VM and publication of both the timed-out work and its blocker, and whether the blocker had already failed. The line is printed at the first deferral, as before, and once the grace has run out, now also when that ends in Own behind failed work. VMs were compared by the address of their status, which cannot be logged. Compare them by mmu::Vm::id() instead, which a G17 build now compiles too: the status is created with its VM, a context takes it from its VM and a completion from its context, so the two identify the same VMs. The new line reads: Render watchdog owner=12 vm=5 publication=345 renewals=0 decision=Defer behind earlier work of another VM: blocker_vm=7 blocker_publication=340 blocker_failed=0 KUnit checks which verdicts name a blocker. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/queue/compute.rs | 2 +- drivers/gpu/drm/asahi/g17/queue/render.rs | 2 +- drivers/gpu/drm/asahi/g17/runtime/backend.rs | 14 +++-- .../drm/asahi/g17/runtime/compute_control.rs | 10 ++-- drivers/gpu/drm/asahi/g17/runtime/render.rs | 12 ++--- drivers/gpu/drm/asahi/g17/timeout.rs | 51 +++++++++++++++++-- drivers/gpu/drm/asahi/mmu.rs | 4 +- 7 files changed, 71 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/compute.rs b/drivers/gpu/drm/asahi/g17/queue/compute.rs index ce6b376dcd9e1..b64e11a889d80 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute.rs @@ -618,7 +618,7 @@ impl Queue { return None; } self.active.front().map(|active| crate::g17::timeout::Head { - vm: Arc::as_ptr(active.packet.completion.status()) as usize, + vm: active.packet.context.vm().id(), published: active.published, finished: active.observed.is_some(), failed: self.quarantined, diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index 4fa4e0f6a6c83..ea048e573dfe0 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -975,7 +975,7 @@ impl Pair { .filter(|active| active.outer_published) .find(|active| active.drained_result.is_none()) .map(|active| crate::g17::timeout::Head { - vm: Arc::as_ptr(active.packet.completion.status()) as usize, + vm: active.packet.context.vm().id(), published: active.published, finished: false, failed: self.quarantined, diff --git a/drivers/gpu/drm/asahi/g17/runtime/backend.rs b/drivers/gpu/drm/asahi/g17/runtime/backend.rs index 1451b8a75ccc5..7ee3352f301d8 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/backend.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/backend.rs @@ -241,17 +241,21 @@ impl job::Backend for Backend { { return true; } - let decision = firmware.queues.render_timeout_decision(packet, renewals); + let verdict = firmware.queues.render_timeout_decision(packet, renewals); + let decision = verdict.map_or(Decision::Own, |verdict| verdict.decision); // As compute does: work still blocked behind another VM's older unfinished work after the // whole grace is innocent, so its failure must stay non-sticky. The pair's quarantine // extends the class to the pair's other groups. if decision == Decision::Spare { packet.completion.classify(crate::g17::recovery::Class::Spared); } - if decision != Decision::Own && (renewals == 0 || decision == Decision::Spare) { - dev_warn!(self.shared.dev.as_ref(), - "Render watchdog owner={} renewals={} decision={:?} behind earlier work of another VM\n", - self.owner, renewals, decision); + if let Some(verdict) = verdict { + if let Some(blocker) = verdict.reported_blocker(renewals) { + dev_warn!(self.shared.dev.as_ref(), + "Render watchdog owner={} vm={} publication={} renewals={} decision={:?} behind earlier work of another VM: blocker_vm={} blocker_publication={} blocker_failed={}\n", + self.owner, verdict.own.vm, verdict.own.published, renewals, decision, + blocker.vm, blocker.published, u8::from(blocker.failed)); + } } decision == Decision::Defer } diff --git a/drivers/gpu/drm/asahi/g17/runtime/compute_control.rs b/drivers/gpu/drm/asahi/g17/runtime/compute_control.rs index 21dd1515cd59b..c407d9576e9c4 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/compute_control.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/compute_control.rs @@ -32,15 +32,17 @@ impl Backend { .filter(|other| other.owner() != Some(self.owner)) .filter_map(|other| other.timeout_head()); let others = Iterator::chain(computes, firmware.queues.render_timeout_heads()); - let decision = timeout::classify(own, renewals, others); + let verdict = timeout::attribute(own, renewals, others); + let decision = verdict.decision; if decision == Decision::Spare { firmware.queues.compute(self.owner).ok_or(EIO)? .classify(&|_| Class::Spared); } - if decision != Decision::Own && (renewals == 0 || decision == Decision::Spare) { + if let Some(blocker) = verdict.reported_blocker(renewals) { dev_warn!(self.shared.dev.as_ref(), - "Compute watchdog owner={} publication={} renewals={} decision={:?} behind earlier work of another VM\n", - self.owner, own.published, renewals, decision); + "Compute watchdog owner={} vm={} publication={} renewals={} decision={:?} behind earlier work of another VM: blocker_vm={} blocker_publication={} blocker_failed={}\n", + self.owner, own.vm, own.published, renewals, decision, + blocker.vm, blocker.published, u8::from(blocker.failed)); } Ok(decision) })(); diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index 876585fb244e4..691339ce7b04a 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -978,7 +978,7 @@ impl Registry { &self, packet: &Arc, renewals: u8, - ) -> crate::g17::timeout::Decision { + ) -> Option { let own = self .render .entries @@ -993,14 +993,14 @@ impl Registry { .flatten() .filter_map(|entry| entry.queue.as_deref()) .filter_map(|queue| queue.timeout_head()); - match own { - Some(own) => crate::g17::timeout::classify( + // Without a head of its own the timeout is its own. + own.map(|own| { + crate::g17::timeout::attribute( own, renewals, Iterator::chain(self.render_timeout_heads(), computes), - ), - None => crate::g17::timeout::Decision::Own, - } + ) + }) } } diff --git a/drivers/gpu/drm/asahi/g17/timeout.rs b/drivers/gpu/drm/asahi/g17/timeout.rs index 99e163c455ff8..d951bd164d6e9 100644 --- a/drivers/gpu/drm/asahi/g17/timeout.rs +++ b/drivers/gpu/drm/asahi/g17/timeout.rs @@ -55,7 +55,8 @@ pub(crate) fn renew_execution(published: bool, enabled: bool, renewals: &mut u8) #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct Head { - pub(crate) vm: usize, + /// The owning VM's [`crate::mmu::Vm::id`], one per VM status; the watchdog lines print it. + pub(crate) vm: u64, pub(crate) published: u64, pub(crate) finished: bool, /// Its queue already failed (quarantined) and holds the GPU only until the firmware drains @@ -94,8 +95,28 @@ pub(crate) fn decide(own: Head, renewals: u8, blocker: Option) -> Decision } } -pub(crate) fn classify(own: Head, renewals: u8, others: impl Iterator) -> Decision { - decide(own, renewals, blocker(own, others)) +/// A watchdog's decision with the heads it rests on, for its log line. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct Verdict { + pub(crate) decision: Decision, + pub(crate) own: Head, + pub(crate) blocker: Option, +} + +impl Verdict { + /// The blocker a watchdog line names: at the first deferral and once the grace has run out. + pub(crate) fn reported_blocker(&self, renewals: u8) -> Option { + self.blocker.filter(|_| renewals == 0 || self.decision != Decision::Defer) + } +} + +pub(crate) fn attribute(own: Head, renewals: u8, others: impl Iterator) -> Verdict { + let blocker = blocker(own, others); + Verdict { + decision: decide(own, renewals, blocker), + own, + blocker, + } } #[cfg_attr(not(test), kernel::macros::kunit_tests(asahi_g17_execution_allowance))] @@ -150,10 +171,13 @@ mod tests { } assert!(background_stalled(Some(since), u64::MAX)); } - fn head(vm: usize, published: u64, finished: bool) -> Head { + fn classify(own: Head, renewals: u8, others: impl Iterator) -> Decision { + attribute(own, renewals, others).decision + } + fn head(vm: u64, published: u64, finished: bool) -> Head { Head { vm, published, finished, failed: false } } - fn failed(vm: usize, published: u64) -> Head { + fn failed(vm: u64, published: u64) -> Head { Head { vm, published, finished: false, failed: true } } #[test] @@ -242,6 +266,23 @@ mod tests { assert_eq!(blocker(own, both_failed.into_iter()), Some(failed(5, 100))); } #[test] + fn the_watchdog_line_names_the_blocker_at_the_first_deferral_and_the_end() { + let own = head(9, 103, false); + let verdict = attribute(own, 0, [failed(5, 100)].into_iter()); + let expected = Verdict { decision: Decision::Defer, own, blocker: Some(failed(5, 100)) }; + assert_eq!(verdict, expected); + assert_eq!(verdict.reported_blocker(0), Some(failed(5, 100))); + assert_eq!(attribute(own, 1, [failed(5, 100)].into_iter()).reported_blocker(1), None); + let last = attribute(own, MAX_RENEWALS, [failed(5, 100)].into_iter()); + assert_eq!(last.decision, Decision::Own); + assert_eq!(last.reported_blocker(MAX_RENEWALS), Some(failed(5, 100))); + let spared = attribute(own, MAX_RENEWALS, [head(5, 100, false)].into_iter()); + assert_eq!(spared.decision, Decision::Spare); + assert_eq!(spared.reported_blocker(MAX_RENEWALS), Some(head(5, 100, false))); + // Nothing blocks: nothing to report. + assert_eq!(attribute(own, 0, [].into_iter()).reported_blocker(0), None); + } + #[test] fn same_vm_finished_and_later_work_do_not_block() { let own = head(5, 101, false); assert_eq!(classify(own, 0, [head(5, 100, false), head(7, 100, true), head(7, 101, false), head(7, 102, false)].into_iter()), Decision::Own); diff --git a/drivers/gpu/drm/asahi/mmu.rs b/drivers/gpu/drm/asahi/mmu.rs index 7fa207c9ff5e4..3ecfad6be6c65 100644 --- a/drivers/gpu/drm/asahi/mmu.rs +++ b/drivers/gpu/drm/asahi/mmu.rs @@ -2055,8 +2055,8 @@ impl Vm { Ok(self) } - /// The identifier this VM was created with (G15 keeps per-VM state keyed by it). - #[cfg(CONFIG_DRM_ASAHI_G16)] + /// The identifier this VM was created with (G15 keeps per-VM state keyed by it; G17 timeout + /// attribution compares and logs VMs by it). pub(crate) fn id(&self) -> u64 { self.id } From 894ec76dc89219caae27f9d6845521337b16e8bf Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:36:19 +1100 Subject: [PATCH 5/7] drm/asahi: g17: treat a torn render sample in the timeout fallback as no proof When fail_after_observation() cannot replay a pair's dropped groups it falls back to retiring an abandoned spared front, with its consecutive abandoned spared successors. That loop took retirement::abandoned() with "?", so a snapshot whose cursors straddled a firmware update returned its error through timeout_render() to stop_after_error() and lose_device_quietly(), and every client lost the device. replay_abandoned() already reads that error as no proof, and the ordinary retirement poll retries it. Do the same here, for the front and its successors: the loop stops, groups already retired stay retired, and if none was the pair is quarantined as on any other timeout. Errors from reading the snapshot, the stamps or the recovery state still propagate, as they do in replay_abandoned(). Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/queue/render.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index ea048e573dfe0..ef43880e8791f 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -1491,8 +1491,14 @@ impl Pair { let snapshot = observer.snapshot(active.ticket.ordinal)?; let stamps = observer.stamps()?; let (_, state) = observer.host.epoch()?; + // A torn sample is no proof, as in replay_abandoned(): the group stays retained + // (quarantined with the pair if nothing was retired) instead of the error + // stopping the runtime for every client. if !active.outer_published - || !retirement::abandoned(&active.ticket, snapshot, stamps, state, false)? + || !matches!( + retirement::abandoned(&active.ticket, snapshot, stamps, state, false), + Ok(true) + ) { break; } From 7d167e382f40542b25ffceb596d0982ee002b7ca Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:37:10 +1100 Subject: [PATCH 6/7] drm/asahi: g17: fail the dropped successors of an exhausted render replay When the front of a pair's dropped groups has already been republished MAX_REPLAYS times, replay_abandoned() failed that front with EIO and returned, leaving its successors active. They were then replayed on their own, after the failed front, and consumed output it never wrote, while the commit that added the multi-group replay says that a failed replay fails the parked successors with it. Fail them with the front instead. replay::decide() already allows the limit only on the front and every group carries the abandonment witness, so each successor is retired with the same accounting and error as the front and none is parked. Log the limit as such rather than as a replay. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/queue/render.rs | 18 +++++++++++++++--- drivers/gpu/drm/asahi/g17/replay.rs | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index ef43880e8791f..41824c135bfb6 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -1283,7 +1283,18 @@ impl Pair { return Ok(false); } } - if !grace || crate::debug::debug_enabled(crate::debug::DebugFlags::VerboseFaults) { + // decide() lets only the front be exhausted. An exhausted front fails, and its + // successors, which consume its output, fail with it rather than run without it. + let mut failing = self.active.front().is_some_and(|front| { + front.packet.completion.replay_count() >= crate::g17::completion::MAX_REPLAYS + }); + if failing { + pr_warn!( + "G17: render slot {}: {} groups retired without execution; replay limit reached, failing them\n", + self.slot, + self.active.len() + ); + } else if !grace || crate::debug::debug_enabled(crate::debug::DebugFlags::VerboseFaults) { pr_warn!( "G17: render slot {}: {} groups retired without execution; replaying\n", self.slot, @@ -1308,7 +1319,8 @@ impl Pair { } let ordinal = front.ticket.ordinal; let packet = front.packet.clone(); - if !packet.completion.begin_replay() { + failing = failing || !packet.completion.begin_replay(); + if failing { if !packet.completion.spared() { packet.completion.status().record(EIO); if self.terminal.is_none() { @@ -1318,7 +1330,7 @@ impl Pair { defer(Deferred::Retired(packet.completion.clone(), Err(EIO)))?; self.memory.retire(ordinal)?; self.active.pop().ok_or(EIO)?; - return Ok(true); + continue; } self.memory.retire(ordinal)?; self.active.pop().ok_or(EIO)?; diff --git a/drivers/gpu/drm/asahi/g17/replay.rs b/drivers/gpu/drm/asahi/g17/replay.rs index f79f9f3c091ec..94daa5ec0dfeb 100644 --- a/drivers/gpu/drm/asahi/g17/replay.rs +++ b/drivers/gpu/drm/asahi/g17/replay.rs @@ -28,8 +28,8 @@ pub(crate) enum Decision { } /// Groups are read in order and the first one that decides stops the walk, so a caller can -/// observe the abandonment lazily. An exhausted front is replayed: the replay fails it alone. -/// An exhausted successor would break the order. +/// observe the abandonment lazily. An exhausted front is replayed: the replay fails it, and its +/// successors, which consume its output, with it. An exhausted successor would break the order. pub(crate) fn decide(groups: impl Iterator) -> Decision { let mut front_exhausted = None; let mut successor_exhausted = false; From 5ee97891f61f552c8c1d677db50725c413fb3ed7 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 24 Oct 2026 12:39:38 +1100 Subject: [PATCH 7/7] drm/asahi: g17: retire finished renders before attributing a render timeout The compute watchdog runs scan_compute() before it classifies a timeout, so a head that finished but whose completion the event worker has not processed yet no longer counts as a blocker. The render watchdog classified without such a pass, and render heads never report themselves finished, so a finished but unprocessed group of another VM could defer a render timeout, and, if its completion stayed unprocessed for the whole grace, spare it. Add scan_renders(), which retires the finished front groups of every pair the registry holds, with the in-lock retirement, collection and terminal settlement that the timeout and VM settlement paths already use, and run it before the render timeout is attributed. Pairs lent out for preparation, growth or observation, and pairs holding a deferred timeout or error, are left to the event worker, which settles them in its own order; nothing is retired while a recovery is pending. A packet the pass retires has its retirement queued, which both timeout paths already treat as settled. The collected callbacks run after the device mutex is released, as in compute, and if no collector can be allocated the attribution goes ahead without the pass, as before. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/asahi/g17/runtime/backend.rs | 63 ++++++++++++-------- drivers/gpu/drm/asahi/g17/runtime/render.rs | 38 ++++++++++++ 2 files changed, 76 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/backend.rs b/drivers/gpu/drm/asahi/g17/runtime/backend.rs index 7ee3352f301d8..7d690470cd7b5 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/backend.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/backend.rs @@ -231,33 +231,46 @@ impl job::Backend for Backend { return self.defer_compute_timeout(packet, renewals); } use crate::g17::timeout::{self, Decision}; - let mut state = self.shared.state.lock(); - let Some(firmware) = (*state).as_deref_mut() else { - return false; - }; - // Borrowed-graph and queued-retirement deferrals share the renewal bound. - if timeout::transaction_grace(renewals) - && firmware.queues.defer_render_timeout(packet, ETIMEDOUT) - { - return true; - } - let verdict = firmware.queues.render_timeout_decision(packet, renewals); - let decision = verdict.map_or(Decision::Own, |verdict| verdict.decision); - // As compute does: work still blocked behind another VM's older unfinished work after the - // whole grace is innocent, so its failure must stay non-sticky. The pair's quarantine - // extends the class to the pair's other groups. - if decision == Decision::Spare { - packet.completion.classify(crate::g17::recovery::Class::Spared); - } - if let Some(verdict) = verdict { - if let Some(blocker) = verdict.reported_blocker(renewals) { - dev_warn!(self.shared.dev.as_ref(), - "Render watchdog owner={} vm={} publication={} renewals={} decision={:?} behind earlier work of another VM: blocker_vm={} blocker_publication={} blocker_failed={}\n", - self.owner, verdict.own.vm, verdict.own.published, renewals, decision, - blocker.vm, blocker.published, u8::from(blocker.failed)); + // As for compute, a final retirement pass precedes the attribution, and all completion + // callbacks run after releasing the device mutex. Without a collector the attribution + // goes ahead without that pass, as it did before it existed. + let mut deferred = DeferredBatch::worker().ok(); + let result = (|| -> Result { + let mut state = self.shared.state.lock(); + let Some(firmware) = (*state).as_deref_mut() else { + return Ok(false); + }; + if let Some(deferred) = deferred.as_mut() { + firmware.scan_renders(deferred)?; + } + // Borrowed-graph and queued-retirement deferrals share the renewal bound. + if timeout::transaction_grace(renewals) + && firmware.queues.defer_render_timeout(packet, ETIMEDOUT) + { + return Ok(true); } + let verdict = firmware.queues.render_timeout_decision(packet, renewals); + let decision = verdict.map_or(Decision::Own, |verdict| verdict.decision); + // As compute does: work still blocked behind another VM's older unfinished work + // after the whole grace is innocent, so its failure must stay non-sticky. The pair's + // quarantine extends the class to the pair's other groups. + if decision == Decision::Spare { + packet.completion.classify(crate::g17::recovery::Class::Spared); + } + if let Some(verdict) = verdict { + if let Some(blocker) = verdict.reported_blocker(renewals) { + dev_warn!(self.shared.dev.as_ref(), + "Render watchdog owner={} vm={} publication={} renewals={} decision={:?} behind earlier work of another VM: blocker_vm={} blocker_publication={} blocker_failed={}\n", + self.owner, verdict.own.vm, verdict.own.published, renewals, decision, + blocker.vm, blocker.published, u8::from(blocker.failed)); + } + } + Ok(decision == Decision::Defer) + })(); + if let Some(deferred) = deferred.as_mut() { + deferred.finish(); } - decision == Decision::Defer + matches!(result, Ok(true)) } fn try_early(&self, packet: &Arc) -> bool { diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index 691339ce7b04a..12b37c5883bc7 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -683,6 +683,44 @@ impl crate::g17::Firmware { Ok(()) } + /// A final retirement pass before a render watchdog attributes its timeout, as compute runs + /// scan_compute() first: a head that finished but whose completion the event worker has not + /// processed yet must not defer another VM's timeout. Pairs lent out, or holding a deferred + /// timeout or error, stay with the worker, which observes and settles them in its order. + pub(in crate::g17::runtime) fn scan_renders(&mut self, deferred: &mut DeferredBatch) -> Result { + if self.recovery.pending() + || self + .primary + .state + .shared + .crashed + .load(core::sync::atomic::Ordering::Acquire) + { + return Ok(()); + } + for slot in 1..self.queues.render.entries.len() { + if !self.queues.render.entries[slot] + .as_ref() + .is_some_and(|entry| { + entry + .pair + .as_ref() + .is_some_and(|pair| pair.published_in_flight()) + && entry.preparing.is_none() + && !entry.growing + && !entry.pending_timeout + && entry.pending_error.is_none() + }) + { + continue; + } + self.retire_render_slot(slot, None, deferred)?; + self.queues.render.collect_retired(slot)?; + self.settle_render_terminal(slot, deferred)?; + } + Ok(()) + } + pub(in crate::g17) fn settle_render_vm( &mut self, status: &Arc,