From 62109cc021f51b246875bd513471fa2a1bf56bf1 Mon Sep 17 00:00:00 2001 From: Jonathan Goyvaerts Date: Tue, 27 Oct 2026 23:56:08 -0300 Subject: [PATCH 1/2] drm/asahi: g17: keep innocent clients alive across another client's recovery A recovery for one client's hang still killed the others: a CS2 reason-0 recovery cancelled 42 commands and Hyprland got ECANCELED from fail_unpublished(), which Honeykrisp treats as a lost device, so the desktop froze until a reboot. - ECANCELED is not reported as this client's failure from fail_unpublished(), the ordered output publication, or fail_while_owned() for spared work. Work the firmware listed as interrupted keeps reporting its failure. - Publication waits for a pending recovery, or for this owner's spared pair to drain, for up to RECOVERY_WAIT_MS (26 s) instead of the usual 2 s: the compute admission deadline extends (Admission:: extend_for_recovery) in ensure_compute, in the recovery waits of compute_packet including the input-source wait, and in Gate::enter_until, whose gate only a recovery closes; the render waits keep a WaitClock whose usual bound starts afresh once the recovery ends, and render admission extends its deadline while a recovery is pending. A spared quarantined pair waits until the caller's deadline; a guilty pair's publication still fails after a second. - Commands with failed inputs or predecessors stay cancelled: a failed fence does not prove its producer retired. The compute deadline host tests move to the longer recovery budget, and a mutant now checks that removing the extension fails them. Link: https://github.com/aurora-silicon/linux/pull/433 Co-developed-by: Ryan Murray Signed-off-by: Ryan Murray Signed-off-by: Jonathan Goyvaerts Assisted-by: LLM --- drivers/gpu/drm/asahi/g17/completion.rs | 8 +- .../drm/asahi/g17/completion/publication.rs | 4 +- drivers/gpu/drm/asahi/g17/dependency.rs | 11 ++ drivers/gpu/drm/asahi/g17/preparation.rs | 51 +++++-- drivers/gpu/drm/asahi/g17/queue/admission.rs | 38 +++-- drivers/gpu/drm/asahi/g17/runtime.rs | 19 ++- .../drm/asahi/g17/runtime/render/submit.rs | 130 +++++++++++++++--- drivers/gpu/drm/asahi/g17/timeout.rs | 7 + include/uapi/drm/asahi_drm.h | 5 +- tools/asahi/g17-admission-deadline-tests.py | 2 +- tools/asahi/g17-compute-constructor-tests.py | 4 +- tools/asahi/g17-compute-deadline-tests.py | 20 +-- .../g17-compute-growth-deadline-tests.py | 6 +- 13 files changed, 248 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/completion.rs b/drivers/gpu/drm/asahi/g17/completion.rs index 7d887f17d22c6..90f6d77780bff 100644 --- a/drivers/gpu/drm/asahi/g17/completion.rs +++ b/drivers/gpu/drm/asahi/g17/completion.rs @@ -694,7 +694,9 @@ impl Completion { return; } self.status.record_if_device_loss(error); - if self.accepted.load(Ordering::Acquire) { + // As in complete(): a cancellation is not this client's device loss. A guilty VM + // already carries its own sticky error. + if self.accepted.load(Ordering::Acquire) && error != ECANCELED { self.status.report_failure(error); } self.release_healthy(); @@ -713,7 +715,9 @@ impl Completion { guard.quarantine(); } if self.spared() { - self.status.report_failure(error); + if error != ECANCELED || self.interrupted() { + self.status.report_failure(error); + } } else { self.report(error); } diff --git a/drivers/gpu/drm/asahi/g17/completion/publication.rs b/drivers/gpu/drm/asahi/g17/completion/publication.rs index a1e05a9408552..fc4b4f8739015 100644 --- a/drivers/gpu/drm/asahi/g17/completion/publication.rs +++ b/drivers/gpu/drm/asahi/g17/completion/publication.rs @@ -96,7 +96,9 @@ impl Publication { if let Err(error) = result { // Inherited failure is visible to the client without inventing a // permanent admission failure in this command's VM. - self.status.report_failure(error); + if error != ECANCELED { + self.status.report_failure(error); + } } self.output.publish(result); let result = result.map(|_| ()); diff --git a/drivers/gpu/drm/asahi/g17/dependency.rs b/drivers/gpu/drm/asahi/g17/dependency.rs index 387ce9ba2fb3c..b7cb74a59644f 100644 --- a/drivers/gpu/drm/asahi/g17/dependency.rs +++ b/drivers/gpu/drm/asahi/g17/dependency.rs @@ -195,4 +195,15 @@ mod tests { record.note(5, 1, &Probe::new(true)); assert_eq!(earliest(&mut record), None); } + + #[test] + fn failed_predecessor_cannot_be_bypassed() { + let mut frontier = Frontier::new(); + for error in [ECANCELED, EIO, ETIMEDOUT] { + assert_eq!(frontier.include(Some(5), 5, error.to_errno(), 0), Err(ECANCELED)); + } + assert_eq!(frontier.include(Some(5), 5, 1, 0), Ok(())); + assert_eq!(frontier.include(Some(5), 6, EIO.to_errno(), 0), Ok(())); + assert_eq!(frontier.count, 0); + } } diff --git a/drivers/gpu/drm/asahi/g17/preparation.rs b/drivers/gpu/drm/asahi/g17/preparation.rs index 2aa3213115076..c7d1761699a24 100644 --- a/drivers/gpu/drm/asahi/g17/preparation.rs +++ b/drivers/gpu/drm/asahi/g17/preparation.rs @@ -6,6 +6,7 @@ //! the device mutex, then drops that mutex before waiting for builders. A builder's lease is //! stored after its tentative mappings so those mappings are destroyed before the join completes. +use core::cell::Cell; use core::sync::atomic::{AtomicU64, Ordering}; use kernel::{ new_condvar, new_mutex, @@ -24,25 +25,37 @@ const EPOCH: u64 = CLOSED << 3; /// packet cancellation, logical close, VM status and device loss even when no /// capacity/recovery event arrives. Wakes and nested constructors never renew it. pub(super) struct Admission<'a> { - deadline: u64, + deadline: Cell, + started: u64, check: &'a dyn Fn() -> Result, } impl<'a> Admission<'a> { pub(super) fn new(deadline: u64, check: &'a dyn Fn() -> Result) -> Self { - Self { deadline, check } + Self { deadline: Cell::new(deadline), started: super::now_ns(), check } + } + + /// A recovery is pending or this owner's spared work drains: wait as long as the + /// watchdog may take to settle it (RECOVERY_WAIT_MS from the start) instead of failing. + pub(super) fn extend_for_recovery(&self) { + let limit = self + .started + .saturating_add(u64::from(super::timeout::RECOVERY_WAIT_MS) * 1_000_000); + if self.deadline.get() < limit { + self.deadline.set(limit); + } } pub(super) fn check(&self) -> Result { (self.check)()?; - if super::now_ns() >= self.deadline { return Err(EBUSY); } + if super::now_ns() >= self.deadline.get() { return Err(EBUSY); } Ok(()) } /// Ignore condvar remaining-time reports: only the monotonic deadline counts. pub(super) fn slice_ms(&self) -> Result { self.check()?; - let remaining = self.deadline.saturating_sub(super::now_ns()); + let remaining = self.deadline.get().saturating_sub(super::now_ns()); if remaining == 0 { return Err(EBUSY); } Ok(remaining.div_ceil(1_000_000).min(10) as u32) } @@ -137,19 +150,20 @@ impl Gate { } } - /// Packet admission may wait for recovery only within its original budget. + /// Only a recovery closes the gate: packet admission waits for it up to + /// RECOVERY_WAIT_MS from the admission's start (Admission::extend_for_recovery). /// Called with no device mutex or existing preparation lease. pub(super) fn enter_until(self: &Arc, admission: &Admission<'_>) -> Result { admission.check()?; match self.try_enter() { - Err(error) if error == EAGAIN => {} + Err(error) if error == EAGAIN => admission.extend_for_recovery(), result => return result, } let mut wait = self.wait.lock(); loop { admission.check()?; match self.try_enter() { - Err(error) if error == EAGAIN => {} + Err(error) if error == EAGAIN => admission.extend_for_recovery(), result => return result, } let _ = self.changed.wait_timeout( @@ -322,6 +336,20 @@ mod growth_gate_tests { mod compute_deadline_tests { use super::*; + #[test] + fn a_recovery_extends_the_deadline_once_from_the_start() { + let check = || Ok(()); + let admission = Admission::new(super::super::now_ns(), &check); + assert_eq!(admission.check(), Err(EBUSY)); + admission.extend_for_recovery(); + assert_eq!(admission.check(), Ok(())); + let limit = admission.started + + u64::from(super::super::timeout::RECOVERY_WAIT_MS) * 1_000_000; + assert_eq!(admission.deadline.get(), limit); + admission.extend_for_recovery(); + assert_eq!(admission.deadline.get(), limit); + } + #[test] fn spent_budget_refuses_even_an_open_gate() { let gate = Gate::new().unwrap(); @@ -332,13 +360,16 @@ mod compute_deadline_tests { } #[test] - fn closed_gate_expires_without_any_recovery_wake() { + fn closed_gate_waits_past_the_budget_for_recovery() { let gate = Gate::new().unwrap(); gate.close(); let start = super::super::now_ns(); - let check = || Ok(()); + // Cancel well after the 20 ms budget: only the recovery extension keeps it waiting. + let check = || { + if super::super::now_ns() - start >= 60_000_000 { Err(ECANCELED) } else { Ok(()) } + }; let admission = Admission::new(start + 20_000_000, &check); - assert!(matches!(gate.enter_until(&admission), Err(error) if error == EBUSY)); + assert!(matches!(gate.enter_until(&admission), Err(error) if error == ECANCELED)); assert!(super::super::now_ns() - start < 500_000_000); gate.wait_drained(); assert!(gate.reopen()); diff --git a/drivers/gpu/drm/asahi/g17/queue/admission.rs b/drivers/gpu/drm/asahi/g17/queue/admission.rs index ed73323e34e6f..f09e629aa047e 100644 --- a/drivers/gpu/drm/asahi/g17/queue/admission.rs +++ b/drivers/gpu/drm/asahi/g17/queue/admission.rs @@ -494,6 +494,16 @@ impl Pool { } } } + /// The owner's pair is quarantined behind another VM's recovery (spared) and drains. + pub(crate) fn spared_quarantined(&self, owner: u64) -> bool { + let state = self.state.lock(); + state + .owners + .iter() + .position(|id| *id == owner) + .is_some_and(|slot| state.quarantined & state.spared & (1u64 << slot) != 0) + } + pub(crate) fn acquire_render( self: &Arc, owner: u64, @@ -525,17 +535,22 @@ impl Pool { .position(|id| *id == owner) .is_some_and(|slot| state.quarantined & (1u64 << slot) != 0) { - if now >= quarantine_deadline { - // Behind an innocent pair's quarantine (another VM wedged the GPU) only this - // publication fails, not the VM: it recovers once the pair drains. - let spared = state - .owners - .iter() - .position(|id| *id == owner) - .is_some_and(|slot| state.spared & (1u64 << slot) != 0); - return Err(if spared { EBUSY } else { EIO }); + // Behind an innocent pair's quarantine (another VM wedged the GPU) wait for the + // pair to drain until the caller's deadline, which the caller extends while it + // drains (RECOVERY_WAIT_MS); a guilty pair's publication fails after a second. + let spared = state + .owners + .iter() + .position(|id| *id == owner) + .is_some_and(|slot| state.spared & (1u64 << slot) != 0); + if now >= quarantine_deadline && !spared { + return Err(EIO); } - let slice = remaining_ms.min(wait_slice_ms(quarantine_deadline, now)?); + let slice = if spared { + remaining_ms + } else { + remaining_ms.min(wait_slice_ms(quarantine_deadline, now)?) + }; if matches!(self.released.wait_interruptible_timeout( &mut state, kernel::time::msecs_to_jiffies(slice) ), CondVarTimeoutResult::Signal { .. }) { @@ -1001,10 +1016,13 @@ mod fault_capacity_tests { let queue = pool.reserve_queue().unwrap(); let lease = pool.acquire_render(1, None, u64::MAX, || Ok(())).unwrap(); let bit = 1u64 << lease.slot(); + assert!(!pool.spared_quarantined(1)); lease.quarantine(true); assert_ne!(pool.state.lock().spared & bit, 0); + assert!(pool.spared_quarantined(1)); lease.quarantine(false); assert_eq!(pool.state.lock().spared & bit, 0); + assert!(!pool.spared_quarantined(1)); // Once guilty, a later spared failure does not make it spared again. lease.quarantine(true); assert_eq!(pool.state.lock().spared & bit, 0); diff --git a/drivers/gpu/drm/asahi/g17/runtime.rs b/drivers/gpu/drm/asahi/g17/runtime.rs index a6edfd0a80b38..d391fbb3001bb 100644 --- a/drivers/gpu/drm/asahi/g17/runtime.rs +++ b/drivers/gpu/drm/asahi/g17/runtime.rs @@ -1756,8 +1756,10 @@ impl Backend { } } // Other owners continue submitting while this owner waits for its - // queue-local drain witness. No new command has been accepted: the - // spared queue and its retained work stay intact past the deadline. + // queue-local drain witness. No new command has been accepted; if the + // admission deadline passes, the spared queue and its retained work stay + // intact until a later admission observes its witness. + admission.extend_for_recovery(); self.shared.queue_events(); self.wait_release(&self.shared.progress, observed, admission)?; } @@ -2090,6 +2092,13 @@ impl Backend { admission.check()?; let observed = progress.observe(); if !self.sources_published(packet)? { + let recovering = self.shared.state.lock().as_deref().is_some_and(|firmware| { + firmware.recovery.pending() + || super::recovery::Memory::recovery_state(&firmware.init).unwrap_or(0) != 0 + }); + if recovering { + admission.extend_for_recovery(); + } self.shared.queue_events(); self.wait_release(progress, observed, &admission)?; continue; @@ -2131,6 +2140,7 @@ impl Backend { } let recovery_state = super::recovery::Memory::recovery_state(&firmware.init)?; if firmware.recovery.pending() || recovery_state != 0 { + admission.extend_for_recovery(); drop(state); drop(lease); self.shared.queue_events(); @@ -2162,6 +2172,11 @@ impl Backend { { // Recovery or backpressure: nothing is firmware-visible. Other // queues keep submitting and the event worker retires meanwhile. + if firmware.recovery.pending() + || super::recovery::Memory::recovery_state(&firmware.init)? != 0 + { + admission.extend_for_recovery(); + } drop(state); drop(lease); self.shared.queue_events(); diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs index f1a433ddb0db4..970035547dfee 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs @@ -54,13 +54,22 @@ impl Backend { ) -> Result<(RenderLease, bool)> { let pool = &self.admission; let started = super::super::now_ns(); - let deadline_ns = started.saturating_add(u64::from(crate::g17::job::TIMEOUT_MS) * 1_000_000); + let mut deadline_ns = + started.saturating_add(u64::from(crate::g17::job::TIMEOUT_MS) * 1_000_000); + let recovery_deadline_ns = started.saturating_add(recovery_wait_ns()); // A capacity refusal samples the render domain and retries once; only // a refusal after that sample waits, so no release can be missed. let mut observed = None; + let mut capacity = WaitClock::default(); loop { self.admission_open()?; - if super::super::now_ns() >= deadline_ns { return Err(EBUSY); } + if super::super::now_ns() >= deadline_ns { + if deadline_ns < recovery_deadline_ns && self.waiting_for_recovery()? { + deadline_ns = recovery_deadline_ns; + } else { + return Err(EBUSY); + } + } if packet.cancel_requested() { return Err(ECANCELED); } @@ -95,11 +104,13 @@ impl Backend { .reclaim_unused_render_spare(Some(self.owner), false)? }; if reclaimed.is_none() { - self.wait_render_capacity(&mut observed, started, sleep)?; + self.wait_render_capacity(&mut observed, &mut capacity, sleep)?; } drop(reclaimed); continue; } + // The deadline check above decides whether a recovery extends the wait. + Err(error) if error == EBUSY => continue, Err(error) => return Err(error), } }; @@ -133,7 +144,7 @@ impl Backend { drop(state); // A full queue table makes admission reuse a free killed pair instead. if reclaimed.is_none() && !ready && !pool.killed_pair_free() { - self.wait_render_capacity(&mut observed, started, sleep)?; + self.wait_render_capacity(&mut observed, &mut capacity, sleep)?; } drop(reclaimed); } @@ -221,13 +232,29 @@ impl Backend { domain.block(observed, remaining.div_ceil(1_000_000).min(10) as u32, false) } + /// A recovery is pending, or this owner's pair is spared and draining after one: an + /// innocent publication waits up to RECOVERY_WAIT_MS for it instead of failing. + fn waiting_for_recovery(&self) -> Result { + let state = self.shared.state.lock(); + let firmware = (*state).as_deref().ok_or(ENODEV)?; + Ok(firmware.recovery.pending() + || crate::g17::recovery::Memory::recovery_state(&firmware.init)? != 0 + || firmware.queues.admission.spared_quarantined(self.owner)) + } + + /// The deadline of the wait `clock` measures (WaitClock::deadline). + fn wait_deadline(&self, clock: &mut WaitClock, bound_ns: u64) -> Result { + let recovering = self.waiting_for_recovery()?; + Ok(clock.deadline(super::super::now_ns(), recovering, bound_ns)) + } + /// A pair is borrowed, still settling, waiting for firmware room or for the /// release of the scheduler it last ran for. Every pair return, retirement /// and teardown step signals the render domain, and each event pass does /// while a publisher waits; the deadline is only the failure bound. fn wait_render_busy( &self, - since: &mut Option, + clock: &mut WaitClock, observed: &mut Option, sleep: bool, packet: &Packet, @@ -235,8 +262,7 @@ impl Backend { if !sleep { return Err(FALLBACK); } - let now = super::super::now_ns(); - let deadline = since.get_or_insert(now).saturating_add(super::super::WORK_RING_WAIT_NS); + let deadline = self.wait_deadline(clock, super::super::WORK_RING_WAIT_NS)?; // Sample, then retry once: only a refusal after the sample waits. let Some(sample) = observed.take() else { *observed = Some(self.shared.renders.observe()); @@ -249,7 +275,12 @@ impl Backend { /// Render capacity is short: return what idle owners hold, then block until /// a pair, lease or owner returns. No packet is firmware-visible here, and /// neither the device mutex nor a preparation lease is held while waiting. - fn wait_render_capacity(&self, observed: &mut Option, started: u64, sleep: bool) -> Result { + fn wait_render_capacity( + &self, + observed: &mut Option, + clock: &mut WaitClock, + sleep: bool, + ) -> Result { if !sleep { return Err(FALLBACK); } @@ -263,8 +294,8 @@ impl Backend { }; self.shared.queue_events(); self.admission_open()?; - let remaining = started - .saturating_add(super::super::WORK_RING_WAIT_NS) + let remaining = self + .wait_deadline(clock, super::super::WORK_RING_WAIT_NS)? .saturating_sub(super::super::now_ns()); if remaining == 0 { return Err(EBUSY); @@ -367,9 +398,10 @@ impl Backend { return result; } } - let mut busy_since = None; - let capacity_started = super::super::now_ns(); - let deadline_ns = capacity_started.saturating_add(super::super::WORK_RING_WAIT_NS); + let mut busy = WaitClock::default(); + let mut capacity = WaitClock::default(); + let mut state_clock = WaitClock::default(); + let mut recovery_since = None; let mut seen = None; let mut states = None; loop { @@ -407,8 +439,11 @@ impl Backend { let work = packet.completion.work_state()?; match states.take() { None => states = Some(work.returns().observe()), + // Work states free as earlier, possibly spared, work retires. Some(sample) => { - self.wait_domain(work.returns(), sample, deadline_ns, packet)? + let deadline = self + .wait_deadline(&mut state_clock, super::super::WORK_RING_WAIT_NS)?; + self.wait_domain(work.returns(), sample, deadline, packet)? } } continue; @@ -426,19 +461,19 @@ impl Backend { }; match ensured { Err(error) if error == ENOSPC => { - self.wait_render_capacity(&mut seen, capacity_started, sleep)?; + self.wait_render_capacity(&mut seen, &mut capacity, sleep)?; continue; } Err(error) if error == EAGAIN => continue, Err(error) if error == EBUSY => { - self.wait_render_busy(&mut busy_since, &mut seen, sleep, packet)?; + self.wait_render_busy(&mut busy, &mut seen, sleep, packet)?; continue; } result => result?, } match self.prepare_render(slot, packet, deferred, sleep) { Err(error) if !packet.is_published() && error == ENOSPC => { - self.wait_render_capacity(&mut seen, capacity_started, sleep)?; + self.wait_render_capacity(&mut seen, &mut capacity, sleep)?; continue; } Err(error) if !packet.is_published() && [EAGAIN, EBUSY].contains(&error) => { @@ -458,10 +493,13 @@ impl Backend { // Recovery closes the preparation gate before touching // any queue; the next pass blocks in enter() until it // reopens. The worker raises the closure. + let since = *recovery_since.get_or_insert(super::super::now_ns()); self.shared.queue_events(); - self.shared.preparations.wait_closure(gate, deadline_ns)?; + self.shared + .preparations + .wait_closure(gate, since.saturating_add(recovery_wait_ns()))?; } else if error == EBUSY { - self.wait_render_busy(&mut busy_since, &mut seen, sleep, packet)?; + self.wait_render_busy(&mut busy, &mut seen, sleep, packet)?; } else if !sleep { return Err(FALLBACK); } @@ -699,3 +737,57 @@ impl crate::g17::Firmware { result } } + +fn recovery_wait_ns() -> u64 { + u64::from(crate::g17::timeout::RECOVERY_WAIT_MS) * 1_000_000 +} + +/// One bounded wait (Backend::wait_deadline). +#[derive(Default)] +struct WaitClock { + first: Option, + calm: u64, +} + +impl WaitClock { + /// `bound_ns` after the later of the first call and the last call that saw a recovery + /// pending or this owner's spared pair draining (so the usual bound applies afresh once a + /// recovery ends), and at most RECOVERY_WAIT_MS after the first call. + fn deadline(&mut self, now: u64, recovering: bool, bound_ns: u64) -> u64 { + let first = *self.first.get_or_insert(now); + self.calm = if recovering { now } else { self.calm.max(first) }; + first + .saturating_add(recovery_wait_ns()) + .min(self.calm.saturating_add(bound_ns)) + } +} + +#[kunit_tests(asahi_g17_wait_clock)] +mod tests { + use super::*; + + const BOUND: u64 = 2_000_000_000; + + #[test] + fn a_calm_wait_keeps_its_first_deadline() { + let mut clock = WaitClock::default(); + assert_eq!(clock.deadline(100, false, BOUND), 100 + BOUND); + assert_eq!(clock.deadline(500, false, BOUND), 100 + BOUND); + } + + #[test] + fn the_bound_starts_afresh_once_a_recovery_ends() { + let mut clock = WaitClock::default(); + clock.deadline(100, false, BOUND); + assert_eq!(clock.deadline(1_000, true, BOUND), 1_000 + BOUND); + assert_eq!(clock.deadline(2_000, false, BOUND), 1_000 + BOUND); + } + + #[test] + fn a_recovery_cannot_extend_past_the_recovery_wait() { + let mut clock = WaitClock::default(); + clock.deadline(100, true, BOUND); + let limit = 100 + recovery_wait_ns(); + assert_eq!(clock.deadline(limit - 1, true, BOUND), limit); + } +} diff --git a/drivers/gpu/drm/asahi/g17/timeout.rs b/drivers/gpu/drm/asahi/g17/timeout.rs index b83ea5d07f406..ce003e6b18196 100644 --- a/drivers/gpu/drm/asahi/g17/timeout.rs +++ b/drivers/gpu/drm/asahi/g17/timeout.rs @@ -18,6 +18,13 @@ pub(crate) fn transaction_grace(renewals: u8) -> bool { renewals < MAX_RENEWALS pub(crate) const POLL_MS: u32 = 2000; const EXECUTION_RENEWALS: u8 = 4; +/// How long an innocent client's publication waits for a recovery to settle and its +/// spared work to drain, instead of failing with EBUSY: the watchdog defers work behind +/// another VM at most this long before it settles it (`classify`; ray work gets the +/// execution renewals on top). +pub(crate) const RECOVERY_WAIT_MS: u32 = + POLL_MS * (1 + EXECUTION_RENEWALS as u32 + MAX_RENEWALS as u32); + /// A background control/replay poll has no scheduler fence to expire it. /// Its timestamp belongs to the exact transaction and is never renewed by work. pub(crate) fn background_stalled(since: Option, now: u64) -> bool { diff --git a/include/uapi/drm/asahi_drm.h b/include/uapi/drm/asahi_drm.h index dc34315ec8c82..418522205ee6f 100644 --- a/include/uapi/drm/asahi_drm.h +++ b/include/uapi/drm/asahi_drm.h @@ -445,7 +445,10 @@ enum drm_asahi_feature { * The driver publishes an accepted job's failure before signaling its fence. * Thus userspace must query after a successful DRM syncobj wait/poll before * treating that fence as successful execution. Syncobj signal alone is not - * an execution-success indication. + * an execution-success indication. A job cancelled (ECANCELED) because + * another VM's recovery discarded it, an input did not signal in time, or + * its queue was closed, is not a failure of this VM: its fence carries the + * error, error stays 0, and its output is undefined. * The feature bit is the availability contract; older kernels leave it zero. * Kernel errors for invalid requests: EINVAL flags/pad/size, ENOENT foreign or * missing vm_id, EFAULT inaccessible pointer. No output on invalid request. diff --git a/tools/asahi/g17-admission-deadline-tests.py b/tools/asahi/g17-admission-deadline-tests.py index b58715f6fcc58..d6e63e08d1188 100644 --- a/tools/asahi/g17-admission-deadline-tests.py +++ b/tools/asahi/g17-admission-deadline-tests.py @@ -91,7 +91,7 @@ def fixture(): assert!(NOW.load(Ordering::SeqCst)<=1_010_000_000,"quarantine budget extended"); reset(mode);let pool=new_pool(false,2);pool.state.lock().spared=2; assert!(matches!(pool.acquire_render(1,None,2_000_000_000,||Ok(())),Err(EBUSY))); - assert!(NOW.load(Ordering::SeqCst)<=1_010_000_000,"spared quarantine budget extended"); + assert!((2_000_000_000..=2_010_000_000).contains(&NOW.load(Ordering::SeqCst)),"a spared quarantine waits for the caller's deadline"); } reset(2);let pool=new_pool(true,0); assert!(matches!(pool.acquire_render(1,Some(Request(&())),2_000_000_000,||Ok(())),Err(ERESTARTSYS))); diff --git a/tools/asahi/g17-compute-constructor-tests.py b/tools/asahi/g17-compute-constructor-tests.py index 8c0c4dc772885..14b736f50436b 100644 --- a/tools/asahi/g17-compute-constructor-tests.py +++ b/tools/asahi/g17-compute-constructor-tests.py @@ -27,14 +27,16 @@ def fixture(): runtime = (ROOT / 'runtime.rs').read_text() preparation = (ROOT / 'preparation.rs').read_text() admission = (item(preparation, "pub(super) struct Admission<'a>") + '\n' + - item(preparation, "impl<'a> Admission<'a>")).replace('pub(super) ', '').replace('super::now_ns()', 'now_ns()') + item(preparation, "impl<'a> Admission<'a>")).replace('pub(super) ', '').replace('super::now_ns()', 'now_ns()').replace('super::timeout::', '') methods = '\n'.join(item(runtime, sig) for sig in [' fn shared_compute(', ' fn compute_scheduler(']) methods = methods.replace('super::object::Allocator', 'Allocator').replace('super::preparation::Lease', 'Lease').replace('super::initdata::', '') return r''' #![allow(dead_code)] +use std::cell::Cell; use std::sync::{Mutex,MutexGuard,atomic::{AtomicBool,AtomicU64,Ordering}}; type Result = std::result::Result; const EIO:i32=-5;const EAGAIN:i32=-11;const ENODEV:i32=-19;const EBUSY:i32=-16;const ECANCELED:i32=-125; +const RECOVERY_WAIT_MS:u32=26_000; static NOW:AtomicU64=AtomicU64::new(0);static CANCEL:AtomicBool=AtomicBool::new(false);static ALLOCATE:AtomicBool=AtomicBool::new(false); fn now_ns()->u64{NOW.load(Ordering::SeqCst)} mod kernel{pub mod time{pub fn msecs_to_jiffies(ms:u32)->u32{ms}}} diff --git a/tools/asahi/g17-compute-deadline-tests.py b/tools/asahi/g17-compute-deadline-tests.py index 98c8751773f9b..0b021d1b8f810 100644 --- a/tools/asahi/g17-compute-deadline-tests.py +++ b/tools/asahi/g17-compute-deadline-tests.py @@ -37,19 +37,21 @@ def fixture(): packet = packet.replace(old, new) ensure = ensure.replace(old, new) admission = (item(preparation, "pub(super) struct Admission<'a>") + '\n' + - item(preparation, "impl<'a> Admission<'a>")).replace('pub(super) ', '').replace('super::now_ns()', 'now_ns()') + item(preparation, "impl<'a> Admission<'a>")).replace('pub(super) ', '').replace('super::now_ns()', 'now_ns()').replace('super::timeout::', '') gate = item(preparation, 'pub(super) fn enter_until(').replace('pub(super) ', '') return r''' #![allow(dead_code,unused_variables,irrefutable_let_patterns)] +use std::cell::Cell; use std::sync::{Arc,Mutex,MutexGuard,atomic::{AtomicBool,AtomicU64,AtomicUsize,Ordering}}; type Result = std::result::Result; const EINVAL:i32=-22;const ECANCELED:i32=-125;const EIO:i32=-5;const ENODEV:i32=-19;const EBUSY:i32=-16;const EAGAIN:i32=-11; const RECOVERY_SETTLE_POLLS:usize=200;const WORK_RING_WAIT_NS:u64=2_000_000_000; +const RECOVERY_WAIT_MS:u32=26_000;const RECOVERY_WAIT_NS:u64=RECOVERY_WAIT_MS as u64*1_000_000; static NOW:AtomicU64=AtomicU64::new(0);static CALLS:AtomicUsize=AtomicUsize::new(0); static MODE:AtomicUsize=AtomicUsize::new(0);static CANCEL:AtomicU64=AtomicU64::new(u64::MAX); static STATUS:AtomicU64=AtomicU64::new(u64::MAX);static CLOSED:AtomicBool=AtomicBool::new(false); fn now_ns()->u64{NOW.load(Ordering::SeqCst)} -fn advance(ns:u64){NOW.fetch_add(ns,Ordering::SeqCst);assert!(now_ns()<=6_000_000_000,"admission budget exceeded");} +fn advance(ns:u64){NOW.fetch_add(ns,Ordering::SeqCst);assert!(now_ns()<=RECOVERY_WAIT_NS+4_000_000_000,"admission budget exceeded");} mod kernel{pub mod time{ pub fn msecs_to_jiffies(ms:u32)->u32{ms} pub struct Delta(pub u64);impl Delta{pub fn from_millis(ms:u64)->Self{Self(ms*1_000_000)}pub fn from_micros(us:i64)->Self{Self(us as u64*1_000)}} @@ -59,7 +61,7 @@ def fixture(): struct Wait; impl Wait{fn lock(&self){}} struct Changed; -impl Changed{fn wait_timeout(&self,_:&mut (),_:u32){advance(1_000_000);assert!(now_ns()<=WORK_RING_WAIT_NS,"gate wake storm must remain bounded");}} +impl Changed{fn wait_timeout(&self,_:&mut (),_:u32){advance(1_000_000);assert!(now_ns()<=RECOVERY_WAIT_NS,"gate wake storm must remain bounded");}} struct Gate{wait:Wait,changed:Changed} struct Lease; impl Lease{fn is_current(&self)->bool{true}} @@ -120,7 +122,8 @@ def fixture(): MODE.store(mode,Ordering::SeqCst);NOW.store(0,Ordering::SeqCst);CALLS.store(0,Ordering::SeqCst); let r=b.compute_packet(&p,&mut DeferredBatch); assert_eq!(r,Err(EBUSY),"one admission budget covers binding theft/recovery/publication"); - assert!(now_ns()<=WORK_RING_WAIT_NS+10_000_000,"admission budget exceeded"); + let budget=if mode==1{RECOVERY_WAIT_NS}else{WORK_RING_WAIT_NS}; + assert!(now_ns()>=budget&&now_ns()<=budget+10_000_000,"admission budget exceeded"); if mode==0{assert_eq!(CALLS.load(Ordering::SeqCst),4);assert_eq!(now_ns(),WORK_RING_WAIT_NS);} println!("PASS mode={} result={:?} calls={} elapsed_ms={}",mode,r,CALLS.load(Ordering::SeqCst),now_ns()/1_000_000); } @@ -147,8 +150,8 @@ def fixture(): NOW.store(0,Ordering::SeqCst);CLOSED.store(true,Ordering::SeqCst); let check=||{if cancel&&now_ns()>=10_000_000{Err(ECANCELED)}else{Ok(())}}; let admission=Admission::new(20_000_000,&check); - assert!(matches!(b.shared.preparations.enter_until(&admission),Err(e) if e==if cancel{ECANCELED}else{EBUSY}),"gate wake storm must check original deadline and cancellation"); - assert_eq!(now_ns(),if cancel{10_000_000}else{20_000_000}); + assert!(matches!(b.shared.preparations.enter_until(&admission),Err(e) if e==if cancel{ECANCELED}else{EBUSY}),"gate wake storm must check the recovery deadline and cancellation"); + assert_eq!(now_ns(),if cancel{10_000_000}else{RECOVERY_WAIT_NS}); } println!("PASS spent budgets, nested capacity cancellation/status, gate recovery wake storms"); } @@ -160,12 +163,13 @@ def main(): mutants = [ ('fixed', source, ''), ('renew-on-binding-loss', source.replace('let admission = Admission::new(deadline, &check);\n let progress = &self.shared.progress;\n loop {', 'let progress = &self.shared.progress;\n loop {\n let admission = Admission::new(now_ns().saturating_add(WORK_RING_WAIT_NS), &check);'), 'admission budget exceeded'), - ('ignore-deadline', source.replace('if now_ns() >= self.deadline', 'if false'), 'admission budget exceeded'), + ('ignore-deadline', source.replace('if now_ns() >= self.deadline.get()', 'if false'), 'admission budget exceeded'), ('render-work-state', source.replace('acquire(crate::g17::job::Engine::Compute)', 'acquire(crate::g17::job::Engine::Render)'), 'compute admission takes a compute work state'), ('omit-gate-check', source.replace(' admission.check()?;\n match self.try_enter()', ' match self.try_enter()').replace('admission.slice_ms()?', '10'), 'gate wake storm'), + ('no-recovery-extension', source.replace('admission.extend_for_recovery();', ''), 'admission budget exceeded'), ] # Bind the old unbounded mutant with the review's eventual cancellation. - mutants[1] = (mutants[1][0], mutants[1][1].replace('for mode in 0..4 {', 'CANCEL.store(10_000_000_000,Ordering::SeqCst);\n for mode in 0..4 {'), mutants[1][2]) + mutants[1] = (mutants[1][0], mutants[1][1].replace('for mode in 0..4 {', 'CANCEL.store(40_000_000_000,Ordering::SeqCst);\n for mode in 0..4 {'), mutants[1][2]) with tempfile.TemporaryDirectory(prefix='g17-compute-deadline-') as tmp: for name, code, expected in mutants: if name != 'fixed': diff --git a/tools/asahi/g17-compute-growth-deadline-tests.py b/tools/asahi/g17-compute-growth-deadline-tests.py index 3c652a96df759..f2c148e5ec46a 100644 --- a/tools/asahi/g17-compute-growth-deadline-tests.py +++ b/tools/asahi/g17-compute-growth-deadline-tests.py @@ -27,15 +27,17 @@ def fixture(): runtime = (ROOT / 'runtime.rs').read_text() prep = (ROOT / 'preparation.rs').read_text() admission = (item(prep, "pub(super) struct Admission<'a>") + '\n' + - item(prep, "impl<'a> Admission<'a>")).replace('pub(super) ', '').replace('super::now_ns()', 'now_ns()') + item(prep, "impl<'a> Admission<'a>")).replace('pub(super) ', '').replace('super::now_ns()', 'now_ns()').replace('super::timeout::', '') methods = '\n'.join(item(runtime, sig) for sig in [' fn grow_compute(', ' fn finish_uninstalled_compute(']) methods = methods.replace('super::object::Allocator', 'Allocator').replace('super::freelist::Pages', 'Pages') return r''' #![allow(dead_code)] +use std::cell::Cell; use std::sync::{Mutex,MutexGuard,atomic::{AtomicBool,AtomicU64,Ordering,fence}}; type Result = std::result::Result; type KBox = Box; const EIO:i32=-5;const EAGAIN:i32=-11;const ENODEV:i32=-19;const EBUSY:i32=-16;const ECANCELED:i32=-125; +const RECOVERY_WAIT_MS:u32=26_000; static NOW:AtomicU64=AtomicU64::new(0);static ALIAS:AtomicBool=AtomicBool::new(false);static PAGES:AtomicBool=AtomicBool::new(false); static RESERVED:AtomicBool=AtomicBool::new(false);static ABORTED:AtomicBool=AtomicBool::new(false); fn now_ns()->u64{NOW.load(Ordering::SeqCst)} @@ -112,7 +114,7 @@ def fixture(): def main(): source = fixture() mutants = [('fixed', source, ''), - ('ignore-shared-deadline', source.replace('if now_ns() >= self.deadline', 'if false').replace('if remaining == 0 { return Err(EBUSY); }', ''), 'growth admission exceeded'), + ('ignore-shared-deadline', source.replace('if now_ns() >= self.deadline.get()', 'if false').replace('if remaining == 0 { return Err(EBUSY); }', ''), 'growth admission exceeded'), ('keep-growth-alias', source.replace(' drop(alias.take());', ''), 'growth aliases must precede')] with tempfile.TemporaryDirectory(prefix='g17-compute-growth-') as tmp: for name, code, expected in mutants: From 432f851c8e3195e77f1c334574130f1743e23407 Mon Sep 17 00:00:00 2001 From: Jonathan Goyvaerts Date: Wed, 28 Oct 2026 09:44:12 -0300 Subject: [PATCH 2/2] drm/asahi: g17: cancel consumers of a failed producer whatever it failed with A command whose input fence, ordered predecessor or compute source failed inherits the failure through inherited_error(). Only EIO, ENODEV and ETIMEDOUT became ECANCELED; any other error passed through unchanged and was reported as the consumer's own failure, which userspace treats as a lost device. A GPU recovery fails the render groups it dropped and cannot replay with ENODATA. One such recovery (render slot 10, 11 groups of a muvm GPU worker) took down Xwayland, whose next command consumed one of the worker's buffers. Xwayland's own commands then failed, so its buffers kept that failure in their fences, and 85 minutes later a newly started muvm GPU worker that imported them failed with ENODATA too, without any recovery. Cancel the consumer for every producer failure. ECANCELED is not reported as a device loss and does not poison the consumer's VM; its fence still carries the error, as the UAPI documents. An interrupted fence wait is not a producer failure and keeps its own error. G15 reports every input failure as a client failure anyway, so it keeps inherited_error() unchanged. Signed-off-by: Jonathan Goyvaerts Assisted-by: LLM --- drivers/gpu/drm/asahi/g17/completion/publication.rs | 8 +++----- drivers/gpu/drm/asahi/g17/dependency.rs | 8 +++++++- drivers/gpu/drm/asahi/g17/fence.rs | 2 +- drivers/gpu/drm/asahi/g17/job.rs | 5 +++-- drivers/gpu/drm/asahi/g17/queue/compute.rs | 4 +--- drivers/gpu/drm/asahi/g17/recovery/safety.rs | 1 + 6 files changed, 16 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/completion/publication.rs b/drivers/gpu/drm/asahi/g17/completion/publication.rs index fc4b4f8739015..8e16a14f05488 100644 --- a/drivers/gpu/drm/asahi/g17/completion/publication.rs +++ b/drivers/gpu/drm/asahi/g17/completion/publication.rs @@ -8,7 +8,7 @@ //! nor waits for another command; the last prerequisite publishes a ready result. use super::Output; -use crate::g17::{dependency::inherited_error, status::VmStatus}; +use crate::g17::{dependency::PRODUCER_FAILED, status::VmStatus}; use kernel::{new_mutex, prelude::*, sync::{Arc, Mutex}}; type Links = Option>; @@ -121,10 +121,8 @@ impl Publication { let ready = { let mut state = self.state.lock(); debug_assert!(state.remaining != 0); - if let Err(error) = result { - if state.predecessor_error.is_none() { - state.predecessor_error = Some(inherited_error(error)); - } + if result.is_err() && state.predecessor_error.is_none() { + state.predecessor_error = Some(PRODUCER_FAILED); } state.remaining -= 1; state.claim() diff --git a/drivers/gpu/drm/asahi/g17/dependency.rs b/drivers/gpu/drm/asahi/g17/dependency.rs index b7cb74a59644f..5a0ed611b1dde 100644 --- a/drivers/gpu/drm/asahi/g17/dependency.rs +++ b/drivers/gpu/drm/asahi/g17/dependency.rs @@ -9,6 +9,7 @@ use super::{ use kernel::prelude::*; /// A failed producer does not establish a fault in its consumer's VM. +#[cfg(CONFIG_DRM_ASAHI_G15)] pub(crate) fn inherited_error(error: Error) -> Error { if error == EIO || error == ENODEV || error == ETIMEDOUT { ECANCELED @@ -17,11 +18,16 @@ pub(crate) fn inherited_error(error: Error) -> Error { } } +/// What a G17 consumer of a failed producer fails with, whatever the producer failed with: its +/// client does not see a cancellation as a lost device. A producer a recovery dropped fails with +/// ENODATA, and passing that on made every consumer of the producer's buffers lose its device too. +pub(crate) const PRODUCER_FAILED: Error = ECANCELED; + /// A replay's original host inputs must still be signalled. Treat an unmet /// input as a failed replay, never as a wait under the device mutex. pub(crate) fn replay_status(status: i32) -> Result { if status == 0 { Err(EBUSY) } - else if status < 0 { Err(inherited_error(Error::from_errno(status))) } + else if status < 0 { Err(PRODUCER_FAILED) } else { Ok(()) } } diff --git a/drivers/gpu/drm/asahi/g17/fence.rs b/drivers/gpu/drm/asahi/g17/fence.rs index 71107371640bd..0b8819bdbd396 100644 --- a/drivers/gpu/drm/asahi/g17/fence.rs +++ b/drivers/gpu/drm/asahi/g17/fence.rs @@ -180,7 +180,7 @@ impl Submission { self.remaining.load(Ordering::Acquire) == 0 || self.retired.load(Ordering::Acquire); let error = self.error.load(Ordering::Acquire); if error != 0 { - return Err(super::dependency::inherited_error(Error::from_errno(error))); + return Err(super::dependency::PRODUCER_FAILED); } Ok(complete) } diff --git a/drivers/gpu/drm/asahi/g17/job.rs b/drivers/gpu/drm/asahi/g17/job.rs index 711dd59b0ecc2..7e43c163ac4a7 100644 --- a/drivers/gpu/drm/asahi/g17/job.rs +++ b/drivers/gpu/drm/asahi/g17/job.rs @@ -177,8 +177,9 @@ impl Packet { kernel::time::msecs_to_jiffies(10) as isize, ) }; + // An interrupted wait is not a producer failure. if waited < 0 { - return Err(super::dependency::inherited_error(Error::from_errno(waited as i32))); + return Err(Error::from_errno(waited as i32)); } if waited > 0 { break; } } @@ -257,7 +258,7 @@ pub(crate) fn fence_status(fence: &Fence) -> i32 { fn check_status(fence: &Fence) -> Result { let status = fence_status(fence); if status < 0 { - Err(super::dependency::inherited_error(Error::from_errno(status))) + Err(super::dependency::PRODUCER_FAILED) } else { Ok(()) } diff --git a/drivers/gpu/drm/asahi/g17/queue/compute.rs b/drivers/gpu/drm/asahi/g17/queue/compute.rs index 368e95f144c24..55994066825f8 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute.rs @@ -2056,9 +2056,7 @@ impl Queue { let ticket = active.ticket.ok_or(EIO)?; let state = crate::g17::job::fence_status(&active.packet.completion.fence()); if state < 0 { - return Err(crate::g17::dependency::inherited_error(Error::from_errno( - state, - ))); + return Err(crate::g17::dependency::PRODUCER_FAILED); } if state > 0 { return Ok(None); diff --git a/drivers/gpu/drm/asahi/g17/recovery/safety.rs b/drivers/gpu/drm/asahi/g17/recovery/safety.rs index 082416f0cb9a6..d71d3cc81553f 100644 --- a/drivers/gpu/drm/asahi/g17/recovery/safety.rs +++ b/drivers/gpu/drm/asahi/g17/recovery/safety.rs @@ -126,5 +126,6 @@ mod tests { assert_eq!(replay_status(1), Ok(())); assert_eq!(replay_status(EIO.to_errno()), Err(ECANCELED)); assert_eq!(replay_status(ETIMEDOUT.to_errno()), Err(ECANCELED)); + assert_eq!(replay_status(ENODATA.to_errno()), Err(ECANCELED)); } }