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..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>; @@ -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(|_| ()); @@ -119,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 387ce9ba2fb3c..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(()) } } @@ -195,4 +201,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/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/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/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)); } } 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: