From 495dc12b05070be831baaec0c817f4184f4b8d45 Mon Sep 17 00:00:00 2001
From: Matthew Jackson <matthew@pq.io>
Date: Sat, 16 May 2026 09:30:44 -0700
Subject: [PATCH] =?UTF-8?q?Revert=20"io/writeback/linux:=20drop=20is=5Fnfs?=
 =?UTF-8?q?=20skip=20=E2=80=94=20bounded=20cache=20works=20on=20every=20me?=
 =?UTF-8?q?dium"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit b2154c2cf7067fba7f47e459dfcd10d39187bb6d.
---
 src/io/writeback/linux.rs | 43 ++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/io/writeback/linux.rs b/src/io/writeback/linux.rs
index 9c4eb2f..0d1ede3 100644
--- a/src/io/writeback/linux.rs
+++ b/src/io/writeback/linux.rs
@@ -22,24 +22,21 @@
 //! fast storage (NVMe) sees smaller chunks to keep cache pressure
 //! tight. Bounds: [4 MiB, 256 MiB].
 //!
-//! ## NFS — same path, same safety net
+//! ## NFS escape hatch
 //!
-//! Earlier revisions of this code unconditionally skipped WAIT_AFTER +
-//! `posix_fadvise(DONTNEED)` on NFS-mounted files, on the premise that
-//! "NFS clients have their own buffering and commit semantics that
-//! handle dirty-page bounds without us forcing the issue." That premise
-//! was empirically wrong: on Linux NFS clients dirty pages still
-//! accumulate up to `vm.dirty_ratio` (default 20% of RAM, ~6.6 GB on a
-//! 32 GB box) before the kernel forces writeback and throttles app
-//! writes. Mux throughput on NFS therefore cycled between ~50 MB/s
-//! (cache absorbing) and ~10 MB/s (cache draining under throttle) on
-//! a ~100 s period — exactly the pathology this pipeline was built to
-//! fix, but disabled on the medium where it actually mattered.
+//! `sync_file_range(WAIT_AFTER)` on an NFS-mounted file can block
+//! indefinitely waiting for the server's commit ack. If the server
+//! never acks (network partition, server-side hang, slow commit), the
+//! syscall never returns and the consumer thread is stuck inside the
+//! kernel — `/api/stop` can't reach it because halt is cooperative.
 //!
-//! `is_nfs` is still detected (for logging and observability) but
-//! `skip_wait` no longer keys off it. WAIT_AFTER + DONTNEED run on NFS
-//! exactly as on local storage. The safety net described below
-//! (`WAIT_AFTER_TIMEOUT`) catches the original NFS-hang concern.
+//! When `fstatfs` reports the file lives on an NFS mount
+//! (`f_type == NFS_SUPER_MAGIC`), the pipeline skips the WAIT_AFTER +
+//! `posix_fadvise(DONTNEED)` dance entirely. NFS clients have their
+//! own buffering and commit semantics that handle dirty-page bounds
+//! without us forcing the issue. The async `SYNC_FILE_RANGE_WRITE`
+//! kickoff still runs (non-blocking by spec) so writeback still gets
+//! a nudge.
 //!
 //! ## Defence in depth: WAIT_AFTER timeout
 //!
@@ -116,8 +113,8 @@ impl WritebackPipeline {
         let is_nfs = detect_nfs(fd);
         tracing::info!(
             target: "mux",
-            "WritebackPipeline fd={fd} is_nfs={is_nfs} chunk_bytes={chunk_bytes} strategy=wait+dontneed (falls back to skip if WAIT_AFTER timeouts past {}s)",
-            WAIT_AFTER_TIMEOUT.as_secs(),
+            "WritebackPipeline fd={fd} is_nfs={is_nfs} chunk_bytes={chunk_bytes} strategy={}",
+            if is_nfs { "nfs-skip-wait" } else { "wait+dontneed" }
         );
         Self {
             fd,
@@ -132,15 +129,11 @@ impl WritebackPipeline {
     }
 
     /// True if we should bypass the WAIT_AFTER + DONTNEED finalisation
-    /// step. The pipeline starts in the normal path on every medium
-    /// (NFS, local, etc.) and flips here only if a real
-    /// `WAIT_AFTER` call exceeds [`WAIT_AFTER_TIMEOUT`] — at which
-    /// point we conclude this particular FS/server combination cannot
-    /// safely service WAIT_AFTER and fall back to skip mode for the
-    /// rest of the pipeline's life. See module-level comment.
+    /// step. NFS always bypasses; local storage bypasses once the
+    /// pipeline has flipped to degraded after a WAIT_AFTER timeout.
     #[inline]
     fn skip_wait(&self) -> bool {
-        self.degraded.load(Ordering::Relaxed)
+        self.is_nfs || self.degraded.load(Ordering::Relaxed)
     }
 
     /// Caller advanced the file position to `pos`. If a chunk boundary