From 9e79a84cd4b2094c47be835ac0815479165d5a15 Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Sat, 7 Nov 2026 23:56:08 +0530 Subject: [PATCH 1/2] daemon+shim: wait for the panel to release a buffer before the guest renders into it DXVK games under gamescope tear on 60 Hz panels above about 40 fps, vsync on or off. On direct scan-out the daemon publishes the guest's pool buffer and sends gamescope's page-flip event with the publish, so gamescope hands back the buffer that flip replaced while SurfaceFlinger may still be scanning it out. The shim frees the slot on that release, the guest acquires it at once and renders into the buffer on the panel. The daemon's own release-fence wait only guards its next publish of that buffer, not the guest's writes. The flip-event timing stays as it is (holding the event for the release halves the rate above half the panel's), so the release goes to the buffer's producer instead: - wire: a ReleaseFence note ("GNRF") on the scan-out channel, sent only to a connection that declared CAP_RELEASE_FENCE. A pending note says a buffer was replaced on the layer and its fence follows; the final note carries the release fence as a sync file (SCM_RIGHTS), or none when SurfaceFlinger reported none. - daemon: each show transaction records the inode of the buffer it replaces. The pending note goes out when the transaction is built, before the apply and so before the flip event; the completion callback forwards the previous-release fence it already queries for that inode. The daemon's own wait is unchanged. - shim: subscribes with CAP_RELEASE_FENCE, keeps notes only for buffers it was handed, and at acquire waits for the slot's release (the note still on its way, then the fence) for at most 33 ms or the acquire's timeout. Each fence is waited once and closed; a fence that arrives after its wait ran out is dropped; a timeout is counted and logged once. The wait is on the CPU: the fence only exists once the replacing transaction completes, which can be after the acquire, and a binary semaphore cannot be waited before its signal is submitted. An older daemon sends no notes and acquire is as before. --- icd_shim/src/proxy.rs | 16 + icd_shim/src/scanout.rs | 515 +++++++++++++++++++++++++++-- modules/gndrm-android/src/asc.rs | 72 +++- modules/gndrm-daemon/src/pushes.rs | 104 +++++- modules/gndrm-uapi/src/wire.rs | 89 +++++ 5 files changed, 763 insertions(+), 33 deletions(-) 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..8d0ad56 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); + } } From 77c581dcfcd0b02799d500fdf9f66045a91bf015 Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Sat, 7 Nov 2026 23:58:35 +0530 Subject: [PATCH 2/2] uapi: name SurfaceFlinger as code in the release note's docs clippy's doc_markdown lint (-D warnings) flags the bare name. --- modules/gndrm-uapi/src/wire.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/gndrm-uapi/src/wire.rs b/modules/gndrm-uapi/src/wire.rs index 8d0ad56..a6314c6 100644 --- a/modules/gndrm-uapi/src/wire.rs +++ b/modules/gndrm-uapi/src/wire.rs @@ -753,9 +753,9 @@ impl PresentDone { /// /// - 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 +/// - 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). +/// 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.