diff --git a/drivers/gpu/drm/asahi/g17/completion.rs b/drivers/gpu/drm/asahi/g17/completion.rs index 4f5b87bc853dc..276bdc8e81a96 100644 --- a/drivers/gpu/drm/asahi/g17/completion.rs +++ b/drivers/gpu/drm/asahi/g17/completion.rs @@ -570,7 +570,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(); @@ -589,7 +591,9 @@ impl Completion { guard.quarantine(); } if self.spared() { - self.status.report_failure(error); + if error != ECANCELED { + self.status.report_failure(error); + } } else if error != ECANCELED { self.status.record(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 0bc1d5024566e..43b0bbfdbbd94 100644 --- a/drivers/gpu/drm/asahi/g17/dependency.rs +++ b/drivers/gpu/drm/asahi/g17/dependency.rs @@ -89,3 +89,19 @@ impl Frontier { self.insert(sequence, dependency) } } + +#[kunit_tests(asahi_g17_dependency)] +mod tests { + use super::*; + + #[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 0297c619b4f93..bb726ef2cf68a 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) } @@ -268,6 +281,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(); diff --git a/drivers/gpu/drm/asahi/g17/queue/admission.rs b/drivers/gpu/drm/asahi/g17/queue/admission.rs index ebdc79d2b2dd3..c9421419f9327 100644 --- a/drivers/gpu/drm/asahi/g17/queue/admission.rs +++ b/drivers/gpu/drm/asahi/g17/queue/admission.rs @@ -403,6 +403,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, @@ -434,17 +444,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 { .. }) { @@ -785,10 +800,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/queue/mod.rs b/drivers/gpu/drm/asahi/g17/queue/mod.rs index 6a9b15789339f..dfd6fa30c74bd 100644 --- a/drivers/gpu/drm/asahi/g17/queue/mod.rs +++ b/drivers/gpu/drm/asahi/g17/queue/mod.rs @@ -255,8 +255,9 @@ impl Queue { }) }); let failure_count = failures.iter().flatten().count(); - let mut dependencies = - KVec::with_capacity(inputs.len() + count + timestamp_count + failure_count, GFP_KERNEL)?; + let mut dependencies = KVec::with_capacity( + inputs.len() + count + timestamp_count + failure_count, GFP_KERNEL, + )?; let mut checked_inputs = KVec::with_capacity(inputs.len() + timestamp_count + failure_count, GFP_KERNEL)?; let mut firmware = KVec::with_capacity(count, GFP_KERNEL)?; diff --git a/drivers/gpu/drm/asahi/g17/runtime.rs b/drivers/gpu/drm/asahi/g17/runtime.rs index 6f4a5355d6eb3..515f81cf8a5a5 100644 --- a/drivers/gpu/drm/asahi/g17/runtime.rs +++ b/drivers/gpu/drm/asahi/g17/runtime.rs @@ -31,7 +31,6 @@ use kernel::{prelude::*, sync::Arc}; const QIDS: usize = 128; const FIRST_COMPUTE_QID: u8 = 4; -const RECOVERY_SETTLE_POLLS: usize = 200; const WORK_RING_WAIT_NS: u64 = super::job::TIMEOUT_MS as u64 * 1_000_000; /// A reservation remains identifiable while its backing is built without the @@ -1287,9 +1286,8 @@ impl Backend { admission.check()?; let required = super::fw::compute_pool::required_blocks(self.shared.active_cores, scs_pages)?; use kernel::time::{delay::fsleep, Delta}; - const SPARED_WAIT_MS: usize = 200; let mut deferred = DeferredBatch::publication()?; - for _ in 0..SPARED_WAIT_MS { + loop { admission.check()?; let pending = (|| { let mut state = self.shared.state.lock(); @@ -1303,18 +1301,20 @@ impl Backend { let pending = pending?; if !pending { match self.build_compute(required, admission) { - Err(error) if error == EAGAIN => continue, + Err(error) if error == EAGAIN => { + fsleep(Delta::from_millis(1)); + continue; + } result => return result, } } - // Other owners continue submitting while this owner gives its - // queue-local drain witness time to arrive. + // Other owners continue submitting while this owner gives its queue-local drain + // witness time to arrive. 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(); fsleep(Delta::from_millis(1)); } - // No new command has been accepted. Keep the spared queue and its - // retained work intact until a later admission observes its witness. - admission.check()?; - Err(EBUSY) } /// Called after taking this packet's publication claim. All waits and work @@ -1326,7 +1326,6 @@ impl Backend { ) -> Result { packet.check_dependencies()?; let super::command::Validated::Compute { scratch, .. } = &packet.command else { return Err(EINVAL); }; - let mut recovery_polls = 0; let mut state_delay = 20i64; let deadline = now_ns().saturating_add(WORK_RING_WAIT_NS); let check = || { @@ -1376,10 +1375,7 @@ impl Backend { } let recovery_state = super::recovery::Memory::recovery_state(&firmware.init)?; if firmware.recovery.pending() || recovery_state != 0 { - if recovery_polls == RECOVERY_SETTLE_POLLS { - return Err(EBUSY); - } - recovery_polls += 1; + admission.extend_for_recovery(); drop(state); drop(lease); self.shared.queue_events(); @@ -1405,10 +1401,7 @@ impl Backend { && (firmware.recovery.pending() || super::recovery::Memory::recovery_state(&firmware.init)? != 0) { - if recovery_polls == RECOVERY_SETTLE_POLLS { - return Err(EBUSY); - } - recovery_polls += 1; + admission.extend_for_recovery(); drop(state); drop(lease); self.shared.queue_events(); @@ -1609,9 +1602,9 @@ impl Registry { } impl Backend { - pub(super) fn wait_capacity(&self, started: u64) -> Result { + pub(super) fn wait_capacity(&self, deadline_ns: u64) -> Result { let check = || self.admission_open(); - let admission = Admission::new(started.saturating_add(WORK_RING_WAIT_NS), &check); + let admission = Admission::new(deadline_ns, &check); self.wait_compute_capacity(&admission) } diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs index b77fc13b7ed2f..035d484ed42f3 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs @@ -59,10 +59,19 @@ impl Backend { (*state).as_deref().ok_or(ENODEV)?.queues.admission.clone() }; 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()); + 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); } @@ -97,11 +106,13 @@ impl Backend { .reclaim_unused_render_spare(Some(self.owner), false)? }; if reclaimed.is_none() { - self.wait_render_capacity(started, sleep)?; + self.wait_render_capacity(&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), } }; @@ -131,7 +142,7 @@ impl Backend { let ready = firmware.queues.has_render_ids(self.owner); drop(state); if reclaimed.is_none() && !ready { - self.wait_render_capacity(started, sleep)?; + self.wait_render_capacity(&mut capacity, sleep)?; } drop(reclaimed); } @@ -140,31 +151,62 @@ impl Backend { } } + /// 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)) + } + + /// Whether the wait `clock` measures gives up: after `bound_ns` counted from the later of + /// its first poll and the last poll that saw a recovery pending or this owner's spared pair + /// draining (so the usual bound applies afresh once a recovery ends), and after + /// RECOVERY_WAIT_MS from its first poll in any case. + fn wait_expired(&self, clock: &mut WaitClock, bound_ns: u64) -> Result { + let now = super::super::now_ns(); + let first = *clock.first.get_or_insert(now); + clock.calm = clock.calm.max(first); + if self.waiting_for_recovery()? { + clock.calm = now; + } + Ok(now.saturating_sub(first) >= recovery_wait_ns() + || now.saturating_sub(clock.calm) >= bound_ns) + } + /// A pair is borrowed, still settling, or waiting for the release of the /// scheduler it last ran for to be consumed and the pair parked. The event /// worker observes both, so kick it on every poll; time out only after the /// same bound as a capacity wait. - fn wait_render_busy(&self, since: &mut Option, sleep: bool) -> Result { + fn wait_render_busy(&self, clock: &mut WaitClock, sleep: bool) -> Result { if !sleep { return Err(FALLBACK); } - let now = super::super::now_ns(); - if now.saturating_sub(*since.get_or_insert(now)) >= super::super::WORK_RING_WAIT_NS { + if self.wait_expired(clock, super::super::WORK_RING_WAIT_NS)? { return Err(ETIMEDOUT); } self.shared.queue_events(); - fsleep(Delta::from_micros(200)); + // Past the usual bound this waits for a recovery: poll gently. + let first = clock.first.unwrap_or(0); + let long = super::super::now_ns().saturating_sub(first) >= super::super::WORK_RING_WAIT_NS; + fsleep(if long { Delta::from_millis(1) } else { Delta::from_micros(200) }); Ok(()) } /// A closing owner may still hold its pair until exact retirement. Wait /// outside both the device mutex and preparation gate so the event worker /// and recovery can return capacity. No packet is firmware-visible here. - fn wait_render_capacity(&self, started: u64, sleep: bool) -> Result { + fn wait_render_capacity(&self, clock: &mut WaitClock, sleep: bool) -> Result { if !sleep { return Err(FALLBACK); } - self.wait_capacity(started) + if self.wait_expired(clock, super::super::WORK_RING_WAIT_NS)? { + return Err(EBUSY); + } + // One reclaim-and-sleep pass; the clock above bounds the wait. + self.wait_capacity(super::super::now_ns().saturating_add(super::super::WORK_RING_WAIT_NS)) } fn render_with_lease( @@ -188,11 +230,11 @@ impl Backend { let admission = self.acquire_render_with_ids(packet, early, sleep)?; let slot = admission.slot(); packet.completion.attach_render_lease(admission)?; - let mut state_polls = 0; + let mut state_clock = WaitClock::default(); let mut state_delay = 20i64; - let mut busy_since = None; - let mut recovery_polls = 0; - let capacity_started = super::super::now_ns(); + let mut busy = WaitClock::default(); + let mut recovery_since = None; + let mut capacity = WaitClock::default(); loop { if packet.cancel_requested() { return Err(ECANCELED); @@ -223,10 +265,10 @@ impl Backend { if !sleep { return Err(FALLBACK); } - if state_polls == 1000 { + // Work states free as earlier, possibly spared, work retires. + if self.wait_expired(&mut state_clock, 1_000_000_000)? { return Err(ETIMEDOUT); } - state_polls += 1; fsleep(Delta::from_micros(state_delay)); state_delay = (state_delay * 2).min(1000); continue; @@ -237,40 +279,34 @@ impl Backend { } match self.ensure_render(packet, slot, sleep) { Err(error) if error == ENOSPC => { - self.wait_render_capacity(capacity_started, sleep)?; + self.wait_render_capacity(&mut capacity, sleep)?; continue; } Err(error) if error == EAGAIN => continue, Err(error) if error == EBUSY => { - self.wait_render_busy(&mut busy_since, sleep)?; + self.wait_render_busy(&mut busy, sleep)?; continue; } result => result?, } match self.prepare_render(slot, packet, deferred, sleep) { Err(error) if !packet.is_published() && error == ENOSPC => { - self.wait_render_capacity(capacity_started, sleep)?; + self.wait_render_capacity(&mut capacity, sleep)?; continue; } Err(error) if !packet.is_published() && [EAGAIN, EBUSY].contains(&error) => { - let recovering = { - let state = self.shared.state.lock(); - let firmware = (*state).as_deref().ok_or(ENODEV)?; - firmware.recovery.pending() - || crate::g17::recovery::Memory::recovery_state(&firmware.init)? != 0 - }; - if recovering { + if self.waiting_for_recovery()? { if !sleep { return Err(FALLBACK); } - if recovery_polls == super::super::RECOVERY_SETTLE_POLLS { + let since = *recovery_since.get_or_insert(super::super::now_ns()); + if super::super::now_ns().saturating_sub(since) >= recovery_wait_ns() { return Err(ETIMEDOUT); } - recovery_polls += 1; self.shared.queue_events(); fsleep(Delta::from_millis(1)); } else if error == EBUSY { - self.wait_render_busy(&mut busy_since, sleep)?; + self.wait_render_busy(&mut busy, sleep)?; } else if !sleep { return Err(FALLBACK); } @@ -505,3 +541,14 @@ 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_expired). +#[derive(Default)] +struct WaitClock { + first: Option, + calm: u64, +} diff --git a/drivers/gpu/drm/asahi/g17/timeout.rs b/drivers/gpu/drm/asahi/g17/timeout.rs index d951bd164d6e9..8215be8c83009 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 66396f89cc333..e4db0063eb937 100644 --- a/include/uapi/drm/asahi_drm.h +++ b/include/uapi/drm/asahi_drm.h @@ -391,7 +391,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.