diff --git a/icd_shim/src/proxy.rs b/icd_shim/src/proxy.rs index d17abd7..edabaa9 100644 --- a/icd_shim/src/proxy.rs +++ b/icd_shim/src/proxy.rs @@ -2833,6 +2833,22 @@ unsafe fn gn_acquire_common( VK_TIMEOUT }; } + // The slot is ours, but the panel may still be scanning its buffer out: the + // compositor hands a buffer back at the flip event, which leaves when its + // replacement is published, before that replacement is on screen. Wait for + // the release the daemon forwards for it (none from an older daemon: no + // wait) before the guest may render into it. A non-blocking acquire is not + // made to block. + if timeout != 0 { + let budget = deadline.map_or(crate::scanout::RELEASE_WAIT, |dl| { + dl.saturating_duration_since(Instant::now()) + .min(crate::scanout::RELEASE_WAIT) + }); + crate::scanout::wait_release( + unsafe { (*(*sc).slots.as_ptr().add(idx as usize)).ino }, + budget, + ); + } if sem != 0 || fence != 0 { let gq = unsafe { (*gd).queues }; if gq.is_null() { diff --git a/icd_shim/src/scanout.rs b/icd_shim/src/scanout.rs index e2d39ac..4b4f86f 100644 --- a/icd_shim/src/scanout.rs +++ b/icd_shim/src/scanout.rs @@ -17,9 +17,9 @@ //! pre-existing behaviour rather than a broken one. use gndrm_uapi::wire::{ - scanout_ahb_format, FrameCapNote, FrameCapSubscribe, LsfgConfig, ScanoutAllocReq, - ScanoutAllocResp, ScanoutLayout, VsyncNote, CAP_FRAME_CAP, CAP_PRESENT_ID, CAP_VSYNC_NOTE, - FPS_UNCAPPED, SCANOUT_LAYOUT_ENV, SCANOUT_REQ_AHB_HANDLE, + scanout_ahb_format, FrameCapNote, FrameCapSubscribe, LsfgConfig, ReleaseFence, ScanoutAllocReq, + ScanoutAllocResp, ScanoutLayout, VsyncNote, CAP_FRAME_CAP, CAP_PRESENT_ID, CAP_RELEASE_FENCE, + CAP_VSYNC_NOTE, FPS_UNCAPPED, SCANOUT_LAYOUT_ENV, SCANOUT_REQ_AHB_HANDLE, }; use std::ffi::c_void; use std::os::fd::RawFd; @@ -427,7 +427,7 @@ fn connect(path: &str) -> RawFd { if !send_frame( fd, &FrameCapSubscribe { - caps: CAP_FRAME_CAP | CAP_VSYNC_NOTE | CAP_PRESENT_ID, + caps: CAP_FRAME_CAP | CAP_VSYNC_NOTE | CAP_PRESENT_ID | CAP_RELEASE_FENCE, } .encode(), ) { @@ -559,6 +559,14 @@ fn exchange_req(fd: RawFd, req: ScanoutAllocReq) -> Result if frames < MAX_BURST_FRAMES { continue; } + } else if let Some(note) = ReleaseFence::decode(buf) { + // A buffer's release, pushed while this exchange waits: + // kept for the next acquire of that buffer. + record_release(note, recv_fd); + if frames < MAX_BURST_FRAMES { + continue; + } + return Err(Failure::Dead); } else if gndrm_uapi::wire::FrameAcquired::decode(buf).is_some() { // The acquire counter shares this channel and can arrive // while a reply is pending; it is not the reply. @@ -593,6 +601,7 @@ fn exchange_req(fd: RawFd, req: ScanoutAllocReq) -> Result } } note_pool_fd_kind(recv_fd, r.ino); + note_owned(r.ino); return Ok(ScanoutBuf { fd: recv_fd, ino: r.ino, @@ -693,36 +702,313 @@ pub(crate) fn poll_notes() { if *ch < 0 { return; } + // Every frame already queued, not just one: release notes arrive twice per + // frame, and one read per call would fall behind them. + for _ in 0..MAX_BURST_FRAMES { + let Some((buf, len, recv_fd)) = recv_push(*ch) else { + return; + }; + dispatch_push(&buf[..len], recv_fd); + } +} + +/// One pushed frame and the descriptor riding with it (`-1` for none), or +/// `None` when nothing is queued (or the peer closed). Non-blocking. +fn recv_push(ch: RawFd) -> Option<([u8; PUSH_LEN], usize, RawFd)> { // One frame of the largest push. SEQPACKET preserves boundaries but truncates - // on a short buffer, so this must cover every push: `FrameCapNote`, - // `VsyncNote` and `LsfgConfig`. + // on a short buffer, so this must cover every push. let mut buf = [0u8; PUSH_LEN]; - // SAFETY: `*ch` is the connection this module owns and `buf` is one full - // frame; MSG_DONTWAIT leaves a frame that has not arrived for the next - // call, and a read of any other length is a peer that closed. - let n = unsafe { libc::recv(*ch, buf.as_mut_ptr().cast(), buf.len(), libc::MSG_DONTWAIT) }; - let Ok(len) = usize::try_from(n) else { - return; + let mut iov = libc::iovec { + iov_base: buf.as_mut_ptr().cast(), + iov_len: buf.len(), + }; + let mut cbuf = [0u64; 8]; + let mut recv_fd: RawFd = -1; + // SAFETY: `ch` is the connection this module owns; the message points at + // live stack buffers; MSG_DONTWAIT leaves a frame that has not arrived for + // the next call. + let n = unsafe { + let mut msg: libc::msghdr = core::mem::zeroed(); + msg.msg_iov = &mut iov; + msg.msg_iovlen = 1; + msg.msg_control = cbuf.as_mut_ptr().cast(); + msg.msg_controllen = core::mem::size_of_val(&cbuf) as _; + let n = libc::recvmsg(ch, &mut msg, libc::MSG_DONTWAIT | libc::MSG_CMSG_CLOEXEC); + if n > 0 { + let mut cmsg = libc::CMSG_FIRSTHDR(&msg); + while !cmsg.is_null() { + if (*cmsg).cmsg_level == libc::SOL_SOCKET && (*cmsg).cmsg_type == libc::SCM_RIGHTS { + let fd = core::ptr::read_unaligned(libc::CMSG_DATA(cmsg).cast::()); + if recv_fd >= 0 { + libc::close(recv_fd); + } + recv_fd = fd; + } + cmsg = libc::CMSG_NXTHDR(&msg, cmsg); + } + } + n }; - if len == 0 { + let len = usize::try_from(n).ok().filter(|&l| l > 0)?; + Some((buf, len, recv_fd)) +} + +/// Apply one pushed frame. `recv_fd` (owned) is kept only by a release note. +fn dispatch_push(buf: &[u8], recv_fd: RawFd) { + if let Some(note) = ReleaseFence::decode(buf) { + record_release(note, recv_fd); return; } + if recv_fd >= 0 { + // SAFETY: a descriptor that rode with a frame that carries none. + unsafe { libc::close(recv_fd) }; + } // Dispatch on the magic: every push type shares the socket, in any order. - if let Some(note) = FrameCapNote::decode(&buf[..len]) { + if let Some(note) = FrameCapNote::decode(buf) { apply_note(note); - } else if let Some(note) = VsyncNote::decode(&buf[..len]) { + } else if let Some(note) = VsyncNote::decode(buf) { apply_vsync(note); - } else if let Some(config) = LsfgConfig::decode(&buf[..len]) { + } else if let Some(config) = LsfgConfig::decode(buf) { if !config.is_request() { crate::lsfg::apply(config); } - } else if let Some(done) = gndrm_uapi::wire::PresentDone::decode(&buf[..len]) { + } else if let Some(done) = gndrm_uapi::wire::PresentDone::decode(buf) { // The panel latched the frame this id was marked for. crate::present::record(done.id, done.latch_ns); - } else if let Some(acq) = gndrm_uapi::wire::FrameAcquired::decode(&buf[..len]) { - // Sent from the acquire path; nothing to apply here. Accepted so the - // log does not report it as an unknown frame. - let _ = acq; + } + // Anything else (the shim's own acquire counter, a frame a newer daemon + // sends) is ignored. +} + +// ------------------------------------------------------------ release fences +// +// The daemon sends gamescope's page-flip event when it publishes a frame, and +// gamescope then hands the replaced buffer back to this shim while the panel +// may still be scanning it out; a guest that renders into it then tears. A +// daemon that knows this shim reads them (`CAP_RELEASE_FENCE`) says so before +// the flip event leaves (a pending note) and forwards the buffer's release fence +// when SurfaceFlinger reports it. The acquire that next hands the buffer to the +// guest waits for both, bounded. An older daemon sends neither, and acquire +// behaves as before. + +/// One scan-out buffer's release state. +struct Release { + ino: u64, + /// Pending notes not yet followed by their fence. + outstanding: u32, + /// The newest fence received, `-1` for none (or "no fence: already free"). + fd: RawFd, +} + +/// Release state by scan-out inode, for buffers this process was handed. +static RELEASES: Mutex> = Mutex::new(Vec::new()); + +/// The most buffers tracked; the oldest entry is dropped beyond it. +const MAX_RELEASES: usize = 64; + +/// Scan-out inodes this process received from the daemon. Notes for any other +/// buffer (another client's, or the compositor's own) are dropped on arrival. +static OWNED: Mutex> = Mutex::new(Vec::new()); + +/// The most inodes remembered as owned; the oldest is forgotten beyond it. +const MAX_OWNED: usize = 256; + +/// Bound of one acquire-side wait, a 30 Hz frame. A release signals within a +/// vsync or two; this only matters when a note was lost. +pub(crate) const RELEASE_WAIT: std::time::Duration = std::time::Duration::from_millis(33); + +/// Release fences waited at acquire, and waits that ran out of budget. +pub(crate) static RELEASE_WAITS: AtomicUsize = AtomicUsize::new(0); +pub(crate) static RELEASE_TIMEOUTS: AtomicUsize = AtomicUsize::new(0); + +fn note_owned(ino: u64) { + let mut o = OWNED.lock().unwrap_or_else(|e| e.into_inner()); + if o.contains(&ino) { + return; + } + if o.len() >= MAX_OWNED { + o.remove(0); + } + o.push(ino); +} + +fn owned(ino: u64) -> bool { + OWNED + .lock() + .unwrap_or_else(|e| e.into_inner()) + .contains(&ino) +} + +fn close_fd(fd: RawFd) { + if fd >= 0 { + // SAFETY: a descriptor the caller owns and gives up here. + unsafe { libc::close(fd) }; + } +} + +/// Take in one release note; `fd` (owned) is the fence that rode with it. +fn record_release(note: ReleaseFence, fd: RawFd) { + if note.ino == 0 || !owned(note.ino) { + close_fd(fd); + return; + } + let mut r = RELEASES.lock().unwrap_or_else(|e| e.into_inner()); + let at = r.iter().position(|e| e.ino == note.ino); + if note.is_pending() { + close_fd(fd); + match at { + Some(i) => r[i].outstanding = r[i].outstanding.saturating_add(1), + None => { + if r.len() >= MAX_RELEASES { + close_fd(r.remove(0).fd); + } + r.push(Release { + ino: note.ino, + outstanding: 1, + fd: -1, + }); + } + } + return; + } + // A fence nobody is waiting for: the buffer's wait already ran out (it was + // handed to the guest), so the fence is dropped rather than kept for a + // later acquire it does not describe. + let Some(i) = at.filter(|&i| r[i].outstanding > 0) else { + close_fd(fd); + return; + }; + r[i].outstanding -= 1; + close_fd(core::mem::replace(&mut r[i].fd, fd)); +} + +/// What the acquire of one buffer has to wait for. +enum ReleaseState { + /// Nothing: no note, or the release arrived with no fence. + Free, + /// A pending note whose fence has not arrived. + Pending, + /// The fence, taken out of the table (the caller owns and closes it). + Fence(RawFd), +} + +fn take_release(ino: u64) -> ReleaseState { + let mut r = RELEASES.lock().unwrap_or_else(|e| e.into_inner()); + let Some(i) = r.iter().position(|e| e.ino == ino) else { + return ReleaseState::Free; + }; + if r[i].outstanding > 0 { + return ReleaseState::Pending; + } + let fd = r.remove(i).fd; + if fd < 0 { + ReleaseState::Free + } else { + ReleaseState::Fence(fd) + } +} + +/// Forget `ino`'s release state after a wait ran out: a late fence for it is +/// then dropped on arrival. +fn abandon_release(ino: u64) { + let mut r = RELEASES.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(i) = r.iter().position(|e| e.ino == ino) { + close_fd(r.remove(i).fd); + } +} + +/// Block for at most `left` until the channel has a frame to read (or, with no +/// channel, just for a short slice): the wake for a fence still on its way. +fn wait_channel(left: std::time::Duration) { + let slice = left.min(std::time::Duration::from_millis(4)); + let ch = match CHAN.try_lock() { + Ok(g) => *g, + Err(std::sync::TryLockError::Poisoned(p)) => *p.into_inner(), + Err(std::sync::TryLockError::WouldBlock) => -1, + }; + if ch >= 0 { + let mut pfd = libc::pollfd { + fd: ch, + events: libc::POLLIN, + revents: 0, + }; + // SAFETY: one live pollfd for the call. The descriptor is only polled, + // so a reconnect replacing it in the meantime costs at most one slice. + let rc = unsafe { libc::poll(&mut pfd, 1, slice.as_millis().max(1) as i32) }; + if rc > 0 && pfd.revents & libc::POLLIN != 0 { + return; + } + if rc == 0 { + return; + } + } + std::thread::sleep(slice.min(std::time::Duration::from_millis(1))); +} + +/// Wait for a sync file to signal, for at most `left`. `true` when it did. +fn wait_sync_file(fd: RawFd, left: std::time::Duration) -> bool { + let deadline = std::time::Instant::now() + left; + loop { + let rest = deadline.saturating_duration_since(std::time::Instant::now()); + let mut pfd = libc::pollfd { + fd, + events: libc::POLLIN, + revents: 0, + }; + // Rounded up, so a sub-millisecond remainder still waits. + let ms = rest.as_micros().div_ceil(1000) as i32; + // SAFETY: one live pollfd for the call. + let rc = unsafe { libc::poll(&mut pfd, 1, ms) }; + if rc > 0 { + return true; + } + let eintr = rc < 0 && std::io::Error::last_os_error().raw_os_error() == Some(libc::EINTR); + if !eintr || rest.is_zero() { + return false; + } + } +} + +/// Before the guest is given buffer `ino` again: wait, for at most `budget`, +/// until the panel is done with it. Returns at once when the daemon announced +/// no release for it (an older daemon, or a buffer the compositor did not scan +/// out). Each fence is waited once and closed here. +pub(crate) fn wait_release(ino: u64, budget: std::time::Duration) { + if ino == 0 { + return; + } + let start = std::time::Instant::now(); + let deadline = start + budget; + poll_notes(); + let done = loop { + let left = deadline.saturating_duration_since(std::time::Instant::now()); + match take_release(ino) { + ReleaseState::Free => break true, + ReleaseState::Fence(fd) => { + RELEASE_WAITS.fetch_add(1, Ordering::Relaxed); + let signalled = wait_sync_file(fd, left); + close_fd(fd); + break signalled; + } + ReleaseState::Pending => { + if left.is_zero() { + abandon_release(ino); + break false; + } + wait_channel(left); + poll_notes(); + } + } + }; + if !done { + let n = RELEASE_TIMEOUTS.fetch_add(1, Ordering::Relaxed); + if n == 0 { + crate::logi!(gndrm_obf::obf!( + "scan-out: a release wait ran out after {}ms (ino={ino} waits={}); counted from here on", + start.elapsed().as_millis(), + RELEASE_WAITS.load(Ordering::Relaxed) + )); + } } } @@ -971,4 +1257,191 @@ mod tests { assert_eq!(period_ns(FPS_UNCAPPED), 0); assert_eq!(period_ns(3), 333_333_333); } + + /// The release tests share the process-wide tables and counters. + static RELEASE_LOCK: Mutex<()> = Mutex::new(()); + + /// A pipe standing in for a sync file: the read end polls readable once a + /// byte is written, the way a fence polls once it signals. + fn fence_pair() -> (RawFd, RawFd) { + let mut p = [0 as RawFd; 2]; + // SAFETY: two out-params in a live array. + assert_eq!(unsafe { libc::pipe(p.as_mut_ptr()) }, 0); + (p[0], p[1]) + } + + fn signal(write_end: RawFd) { + // SAFETY: the write end of a live pipe. + assert_eq!( + unsafe { libc::write(write_end, b"x".as_ptr().cast(), 1) }, + 1 + ); + } + + fn is_open(fd: RawFd) -> bool { + // SAFETY: probe only. + unsafe { libc::fcntl(fd, libc::F_GETFD) != -1 } + } + + fn pending(ino: u64) -> ReleaseFence { + ReleaseFence { + ino, + flags: ReleaseFence::PENDING, + } + } + + fn fence(ino: u64) -> ReleaseFence { + ReleaseFence { ino, flags: 0 } + } + + #[test] + fn a_pushed_release_fence_is_waited_once_at_acquire() { + let _g = RELEASE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let ino = 0x5eed_0001; + note_owned(ino); + let (rd, wr) = fence_pair(); + record_release(pending(ino), -1); + record_release(fence(ino), rd); + signal(wr); + let waits = RELEASE_WAITS.load(Ordering::Relaxed); + let timeouts = RELEASE_TIMEOUTS.load(Ordering::Relaxed); + wait_release(ino, RELEASE_WAIT); + assert_eq!(RELEASE_WAITS.load(Ordering::Relaxed), waits + 1); + assert!(!is_open(rd), "a waited fence must be closed"); + // The next acquire of the buffer has nothing left to wait for. + wait_release(ino, RELEASE_WAIT); + assert_eq!(RELEASE_WAITS.load(Ordering::Relaxed), waits + 1); + assert_eq!(RELEASE_TIMEOUTS.load(Ordering::Relaxed), timeouts); + // SAFETY: the test owns the write end. + unsafe { libc::close(wr) }; + } + + #[test] + fn an_acquire_waits_for_a_fence_still_on_its_way() { + let _g = RELEASE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let ino = 0x5eed_0002; + note_owned(ino); + record_release(pending(ino), -1); + let (rd, wr) = fence_pair(); + let late = std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_millis(10)); + signal(wr); + record_release(fence(ino), rd); + wr + }); + let timeouts = RELEASE_TIMEOUTS.load(Ordering::Relaxed); + let t0 = std::time::Instant::now(); + wait_release(ino, std::time::Duration::from_millis(500)); + let took = t0.elapsed(); + let wr = late.join().unwrap(); + assert!( + took >= std::time::Duration::from_millis(9), + "returned before the fence: {took:?}" + ); + assert!( + took < std::time::Duration::from_millis(400), + "waited past the fence: {took:?}" + ); + assert_eq!(RELEASE_TIMEOUTS.load(Ordering::Relaxed), timeouts); + assert!(!is_open(rd)); + // SAFETY: the test owns the write end. + unsafe { libc::close(wr) }; + } + + #[test] + fn an_unsignalled_fence_is_waited_for_and_bounded() { + let _g = RELEASE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let ino = 0x5eed_0003; + note_owned(ino); + let (rd, wr) = fence_pair(); + record_release(pending(ino), -1); + record_release(fence(ino), rd); + let timeouts = RELEASE_TIMEOUTS.load(Ordering::Relaxed); + let t0 = std::time::Instant::now(); + wait_release(ino, std::time::Duration::from_millis(20)); + assert!(t0.elapsed() >= std::time::Duration::from_millis(19)); + assert_eq!(RELEASE_TIMEOUTS.load(Ordering::Relaxed), timeouts + 1); + assert!(!is_open(rd)); + // SAFETY: the test owns the write end. + unsafe { libc::close(wr) }; + } + + #[test] + fn a_lost_fence_costs_one_bounded_wait_and_a_late_one_is_dropped() { + let _g = RELEASE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let ino = 0x5eed_0004; + note_owned(ino); + record_release(pending(ino), -1); + let timeouts = RELEASE_TIMEOUTS.load(Ordering::Relaxed); + let t0 = std::time::Instant::now(); + wait_release(ino, std::time::Duration::from_millis(20)); + assert!(t0.elapsed() >= std::time::Duration::from_millis(19)); + assert_eq!(RELEASE_TIMEOUTS.load(Ordering::Relaxed), timeouts + 1); + // The fence arrives after the buffer went to the guest: dropped. + let (rd, wr) = fence_pair(); + record_release(fence(ino), rd); + assert!( + !is_open(rd), + "a fence for a re-acquired buffer must be closed" + ); + let waits = RELEASE_WAITS.load(Ordering::Relaxed); + let t1 = std::time::Instant::now(); + wait_release(ino, RELEASE_WAIT); + assert!(t1.elapsed() < std::time::Duration::from_millis(15)); + assert_eq!(RELEASE_WAITS.load(Ordering::Relaxed), waits); + // SAFETY: the test owns the write end. + unsafe { libc::close(wr) }; + } + + #[test] + fn a_release_with_no_fence_frees_the_buffer_and_others_buffers_are_ignored() { + let _g = RELEASE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let ino = 0x5eed_0005; + note_owned(ino); + record_release(pending(ino), -1); + record_release(fence(ino), -1); + let t0 = std::time::Instant::now(); + wait_release(ino, RELEASE_WAIT); + assert!(t0.elapsed() < std::time::Duration::from_millis(15)); + // A buffer this process was never handed: nothing is kept for it. + let stranger = 0x5eed_0f00; + let (rd, wr) = fence_pair(); + record_release(pending(stranger), -1); + record_release(fence(stranger), rd); + assert!(!is_open(rd)); + let t1 = std::time::Instant::now(); + wait_release(stranger, RELEASE_WAIT); + assert!(t1.elapsed() < std::time::Duration::from_millis(15)); + // SAFETY: the test owns the write end. + unsafe { libc::close(wr) }; + } + + #[test] + fn a_pushed_fence_is_received_with_its_descriptor() { + let mut sv = [0 as RawFd; 2]; + // SAFETY: two out-params in a live array. + assert_eq!( + unsafe { + libc::socketpair( + libc::AF_UNIX, + libc::SOCK_SEQPACKET | libc::SOCK_CLOEXEC, + 0, + sv.as_mut_ptr(), + ) + }, + 0 + ); + let (rd, wr) = fence_pair(); + send_with_fd(sv[0], &fence(42).encode(), rd); + let (buf, len, got) = recv_push(sv[1]).expect("a pushed frame"); + assert_eq!(ReleaseFence::decode(&buf[..len]), Some(fence(42))); + assert!(got >= 0 && got != rd); + assert!(recv_push(sv[1]).is_none(), "nothing else was queued"); + // SAFETY: the test owns every descriptor it opened or received. + unsafe { + [sv[0], sv[1], rd, wr, got].iter().for_each(|fd| { + libc::close(*fd); + }) + }; + } } diff --git a/modules/gndrm-android/src/asc.rs b/modules/gndrm-android/src/asc.rs index a781b08..f2734c2 100644 --- a/modules/gndrm-android/src/asc.rs +++ b/modules/gndrm-android/src/asc.rs @@ -54,6 +54,7 @@ //! [`AscState::publish`] waits on it before reusing the same buffer. use core::ffi::{c_int, c_void}; +use std::collections::VecDeque; use std::collections::{HashMap, HashSet}; use std::io; use std::os::fd::RawFd; @@ -70,7 +71,8 @@ use gndrm_daemon::scanout::{ use gndrm_daemon::{CursorUpdate, FrameCap, Framebuffer, PlaneState, Presenter}; use gndrm_uapi::wire::{ scanout_ahb_format, FrameAcquired, FrameCapSubscribe, LsfgConfig, PresentDone, PresentMark, - ScanoutAllocReq, ScanoutAllocResp, CAP_FRAME_CAP, CAP_PRESENT_ID, CAP_VSYNC_NOTE, + ReleaseFence, ScanoutAllocReq, ScanoutAllocResp, CAP_FRAME_CAP, CAP_PRESENT_ID, + CAP_RELEASE_FENCE, CAP_VSYNC_NOTE, }; use crate::ffi; @@ -264,6 +266,33 @@ struct ReleaseCtx { /// reuse cache keys on it). Never cleared: the flag belongs to this /// registration, and a registration is not reused. latched: Arc, + /// For each transaction this registration was published in and whose + /// completion has not arrived yet, in apply order: the scan-out inode of + /// the buffer it replaced on the layer, 0 for none. The completion pops the + /// front and forwards that buffer's release fence to its producer. + replaced: Mutex>, +} + +/// The most completions one registration can owe before the oldest entry is +/// dropped (its producer's bounded wait then covers the missing note). +const REPLACED_QUEUE_CAP: usize = 8; + +/// Tell the producers that read release fences that `ino` has been replaced on +/// the layer and its release fence follows. Sent before the transaction is +/// applied, hence before gamescope's page-flip event hands the buffer back. +fn push_release_pending(ino: u64) { + let note = ReleaseFence { + ino, + flags: ReleaseFence::PENDING, + }; + gndrm_daemon::pushes::push(CAP_RELEASE_FENCE, ¬e.encode()); +} + +/// Forward the release fence SurfaceFlinger reported for the replaced buffer +/// `ino` (`fd`, borrowed; `-1` when it reported none, i.e. the buffer is free). +fn push_release_fence(ino: u64, fd: RawFd) { + let note = ReleaseFence { ino, flags: 0 }; + gndrm_daemon::pushes::push_with_fd(CAP_RELEASE_FENCE, ¬e.encode(), fd); } // SAFETY: after construction the only mutations are the atomic and the release @@ -425,6 +454,18 @@ unsafe extern "C" fn on_complete(context: *mut c_void, stats: *mut ffi::ASurface // exactly once here. unsafe { (ctx.stats_release_controls)(controls) }; } + // The fence belongs to the buffer this transaction replaced. Its producer + // may already have it back (the flip event left at publish), so the fence + // goes to it as well; the daemon's own wait below keeps its copy. + let replaced = ctx + .replaced + .lock() + .unwrap_or_else(PoisonError::into_inner) + .pop_front() + .unwrap_or(0); + if replaced != 0 { + push_release_fence(replaced, fd); + } if fd >= 0 && ctx .release_fd @@ -857,6 +898,7 @@ fn new_release_ctx(api: &ffi::ScApi) -> (*mut ReleaseCtx, Arc) { applied_ns: AtomicU64::new(0), present_id: AtomicU64::new(0), latched: Arc::clone(&latched), + replaced: Mutex::new(VecDeque::new()), })); (ctx, latched) } @@ -1571,7 +1613,15 @@ impl AscState { self.win_h) ); } - match build_show(&api, self.sc, ahb, ctx, rect, fence) { + // The buffer this frame takes the layer from, whose release + // its producer is told about (none for a re-show of itself). + let replaced = self.last_publish.map_or(0, |lp| lp.ino); + let replaced = if replaced == published_ino { + 0 + } else { + replaced + }; + match build_show(&api, self.sc, ahb, ctx, rect, fence, replaced) { Some(tx) => { apply_transaction(&api, tx); // SAFETY: `ctx` is the leaked mailbox of this slot. @@ -1903,7 +1953,7 @@ impl AscState { dst: lp.dst, dataspace: lp.dataspace, }; - if let Some(tx) = build_show(&api, self.sc, ahb, ctx, rect, None) { + if let Some(tx) = build_show(&api, self.sc, ahb, ctx, rect, None, 0) { apply_transaction(&api, tx); // SAFETY: `ctx` is the leaked mailbox of this slot. unsafe { @@ -3561,6 +3611,7 @@ fn build_show( ctx: *mut ReleaseCtx, rect: ShowRect, fence: Option, + replaced_ino: u64, ) -> Option<*mut ffi::ASurfaceTransaction> { let ShowRect { src, @@ -3662,6 +3713,21 @@ fn build_show( // The completion reports the previous release fence, which is what // gates re-publishing this buffer. (api.st_set_on_complete)(tx, ctx.cast(), on_complete); + // One entry per transaction carrying this callback, so the completion + // pops the one that belongs to it. + { + let mut q = (*ctx) + .replaced + .lock() + .unwrap_or_else(PoisonError::into_inner); + if q.len() >= REPLACED_QUEUE_CAP { + q.pop_front(); + } + q.push_back(replaced_ino); + } + if replaced_ino != 0 { + push_release_pending(replaced_ino); + } Some(tx) } } diff --git a/modules/gndrm-daemon/src/pushes.rs b/modules/gndrm-daemon/src/pushes.rs index 5ad4f29..2fdfeca 100644 --- a/modules/gndrm-daemon/src/pushes.rs +++ b/modules/gndrm-daemon/src/pushes.rs @@ -25,7 +25,7 @@ use std::sync::atomic::{AtomicI32, Ordering}; use std::sync::{Mutex, PoisonError}; #[cfg(test)] -use gndrm_uapi::wire::{CAP_FRAME_CAP, CAP_PRESENT_ID, CAP_VSYNC_NOTE}; +use gndrm_uapi::wire::{CAP_FRAME_CAP, CAP_PRESENT_ID, CAP_RELEASE_FENCE, CAP_VSYNC_NOTE}; /// One connection that asked to be pushed to: its descriptor and the cap bits it /// declared in its subscribe. @@ -124,6 +124,13 @@ pub fn clear_served(fd: RawFd) { /// between. Any other failure means the peer is gone: the sink is dropped, and /// its descriptor closed unless the acceptor is serving it. pub fn push(cap: u32, frame: &[u8]) { + push_with_fd(cap, frame, -1); +} + +/// [`push`], with `attach` riding along via `SCM_RIGHTS` when it is not `-1`: +/// each sink receives its own duplicate, and the caller keeps (and closes) +/// `attach`. +pub fn push_with_fd(cap: u32, frame: &[u8], attach: RawFd) { let served = SERVED_FD.load(Ordering::Relaxed); let mut sinks = SINKS.lock().unwrap_or_else(PoisonError::into_inner); sinks.retain(|sink| { @@ -133,14 +140,7 @@ pub fn push(cap: u32, frame: &[u8]) { // SAFETY: `sink.fd` is a descriptor this registry owns until it is // dropped or closed here, and `frame` outlives the call. MSG_NOSIGNAL // keeps a peer that has gone away from raising SIGPIPE in the daemon. - let sent = unsafe { - libc::send( - sink.fd, - frame.as_ptr().cast(), - frame.len(), - libc::MSG_NOSIGNAL | libc::MSG_DONTWAIT, - ) - }; + let sent = unsafe { send_frame(sink.fd, frame, attach) }; if sent == frame.len() as isize { return true; } @@ -158,6 +158,37 @@ pub fn push(cap: u32, frame: &[u8]) { }); } +/// One non-blocking `sendmsg` of `frame`, with `attach` as `SCM_RIGHTS` unless +/// it is `-1`. Returns what `sendmsg` returned. +/// +/// # Safety +/// `fd` is a live socket and `attach` is `-1` or a live descriptor. +unsafe fn send_frame(fd: RawFd, frame: &[u8], attach: RawFd) -> isize { + let mut iov = libc::iovec { + iov_base: frame.as_ptr().cast_mut().cast(), + iov_len: frame.len(), + }; + // Room for one descriptor's control message, aligned for `cmsghdr`. + let mut cbuf = [0u64; 4]; + // SAFETY: a zeroed msghdr is a valid empty message; every pointer set below + // refers to a buffer that outlives the call. + unsafe { + let mut msg: libc::msghdr = core::mem::zeroed(); + msg.msg_iov = &mut iov; + msg.msg_iovlen = 1; + if attach >= 0 { + msg.msg_control = cbuf.as_mut_ptr().cast(); + msg.msg_controllen = libc::CMSG_SPACE(core::mem::size_of::() as u32) as _; + let cmsg = libc::CMSG_FIRSTHDR(&msg); + (*cmsg).cmsg_level = libc::SOL_SOCKET; + (*cmsg).cmsg_type = libc::SCM_RIGHTS; + (*cmsg).cmsg_len = libc::CMSG_LEN(core::mem::size_of::() as u32) as _; + core::ptr::write_unaligned(libc::CMSG_DATA(cmsg).cast::(), attach); + } + libc::sendmsg(fd, &msg, libc::MSG_NOSIGNAL | libc::MSG_DONTWAIT) + } +} + /// Close and forget every sink (session teardown: nothing is left to push to). pub fn close_all() { let mut sinks = SINKS.lock().unwrap_or_else(PoisonError::into_inner); @@ -361,4 +392,59 @@ mod tests { }) }; } + + #[test] + fn a_descriptor_rides_only_to_sinks_that_read_release_fences() { + let _g = reset(); + let (reads, peer_reads) = pair(); + let (old, peer_old) = pair(); + add(reads, CAP_FRAME_CAP | CAP_RELEASE_FENCE); + add(old, CAP_FRAME_CAP | CAP_PRESENT_ID); + let mut pipe = [0 as RawFd; 2]; + // SAFETY: two out-params in a live array. + assert_eq!(unsafe { libc::pipe(pipe.as_mut_ptr()) }, 0); + push_with_fd(CAP_RELEASE_FENCE, b"fence", pipe[0]); + assert!( + recv_one(peer_old).is_none(), + "an older shim got a release fence" + ); + // SAFETY: one frame and one descriptor's control message into live + // stack buffers. + let (got, fd) = unsafe { + let mut buf = [0u8; 64]; + let mut iov = libc::iovec { + iov_base: buf.as_mut_ptr().cast(), + iov_len: buf.len(), + }; + let mut cbuf = [0u64; 4]; + let mut msg: libc::msghdr = core::mem::zeroed(); + msg.msg_iov = &mut iov; + msg.msg_iovlen = 1; + msg.msg_control = cbuf.as_mut_ptr().cast(); + msg.msg_controllen = core::mem::size_of_val(&cbuf) as _; + let n = libc::recvmsg(peer_reads, &mut msg, libc::MSG_DONTWAIT); + let cmsg = libc::CMSG_FIRSTHDR(&msg); + assert!(!cmsg.is_null(), "no descriptor rode with the note"); + let fd = core::ptr::read_unaligned(libc::CMSG_DATA(cmsg).cast::()); + (buf[..n.max(0) as usize].to_vec(), fd) + }; + assert_eq!(&got[..], b"fence"); + assert!( + fd >= 0 && fd != pipe[0], + "the sink must get its own duplicate" + ); + // The duplicate is the same pipe: a byte written to the write end is + // readable from it. + // SAFETY: the pipe and the received duplicate are live. + unsafe { + assert_eq!(libc::write(pipe[1], b"x".as_ptr().cast(), 1), 1); + let mut b = [0u8; 1]; + assert_eq!(libc::read(fd, b.as_mut_ptr().cast(), 1), 1); + [reads, peer_reads, old, peer_old, pipe[0], pipe[1], fd] + .iter() + .for_each(|x| { + libc::close(*x); + }); + } + } } diff --git a/modules/gndrm-uapi/src/wire.rs b/modules/gndrm-uapi/src/wire.rs index 6038f34..a6314c6 100644 --- a/modules/gndrm-uapi/src/wire.rs +++ b/modules/gndrm-uapi/src/wire.rs @@ -744,6 +744,72 @@ impl PresentDone { } } +/// Release-fence magic: ASCII "GNRF". Daemon -> shim, on the scan-out channel. +/// +/// The daemon sends gamescope's page-flip event when it publishes a frame, and +/// gamescope then hands the buffer that frame replaced back to its producer, +/// while the panel may still be scanning that buffer out. These notes tell the +/// producer when the buffer is off the panel: +/// +/// - at publish, before the flip event leaves, a note with +/// [`ReleaseFence::PENDING`] set: a release fence for `ino` will follow; +/// - when `SurfaceFlinger` completes the transaction, a note without it, carrying +/// the buffer's release fence (one sync-file descriptor via `SCM_RIGHTS`), or +/// no descriptor when `SurfaceFlinger` reported none (the buffer is free). +/// +/// Only a connection that declared [`CAP_RELEASE_FENCE`] is sent either; an +/// older shim never sees them. +pub const RELEASE_FENCE_MAGIC: u32 = 0x474E_5246; + +/// The `caps` bit a [`FrameCapSubscribe`] sets when the shim reads +/// [`ReleaseFence`] notes (and their descriptor). +pub const CAP_RELEASE_FENCE: u32 = 1 << 3; + +/// A scan-out buffer's release, daemon -> shim (see [`RELEASE_FENCE_MAGIC`]). +/// +/// Encoded little-endian, 16 bytes: `magic:u32, flags:u32, ino:u64`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ReleaseFence { + /// The scan-out inode of the buffer the release belongs to. + pub ino: u64, + /// [`ReleaseFence::PENDING`] or 0. + pub flags: u32, +} + +impl ReleaseFence { + pub const LEN: usize = 16; + + /// The buffer was replaced on the layer; its fence follows in a later note. + pub const PENDING: u32 = 1 << 0; + + #[must_use] + pub fn is_pending(self) -> bool { + self.flags & Self::PENDING != 0 + } + + #[must_use] + pub fn encode(self) -> [u8; Self::LEN] { + let mut b = [0u8; Self::LEN]; + b[0..4].copy_from_slice(&RELEASE_FENCE_MAGIC.to_le_bytes()); + b[4..8].copy_from_slice(&self.flags.to_le_bytes()); + b[8..16].copy_from_slice(&self.ino.to_le_bytes()); + b + } + + #[must_use] + pub fn decode(b: &[u8]) -> Option { + if b.len() != Self::LEN + || u32::from_le_bytes(b[0..4].try_into().ok()?) != RELEASE_FENCE_MAGIC + { + return None; + } + Some(Self { + flags: u32::from_le_bytes(b[4..8].try_into().ok()?), + ino: u64::from_le_bytes(b[8..16].try_into().ok()?), + }) + } +} + /// Vsync note magic: ASCII "GNVS". Daemon -> shim. pub const VSYNC_MAGIC: u32 = 0x474E_5653; @@ -1270,4 +1336,27 @@ mod tests { assert_eq!(FrameCapNote::decode(&enc), None); assert_eq!(FrameCapSubscribe::decode(&sub.encode()), Some(sub)); } + + #[test] + fn release_fence_round_trips_and_rejects_other_frames() { + for flags in [0, ReleaseFence::PENDING] { + let n = ReleaseFence { + ino: 0x1234_5678_9abc, + flags, + }; + let enc = n.encode(); + assert_eq!(enc[0..4], RELEASE_FENCE_MAGIC.to_le_bytes()); + assert_eq!(ReleaseFence::decode(&enc), Some(n)); + assert_eq!( + ReleaseFence::decode(&enc).map(ReleaseFence::is_pending), + Some(flags != 0) + ); + assert_eq!(ReleaseFence::decode(&enc[..15]), None); + } + assert_eq!( + ReleaseFence::decode(&PresentDone { id: 1, latch_ns: 2 }.encode()), + None + ); + assert_ne!(CAP_RELEASE_FENCE, CAP_PRESENT_ID); + } }