From 607116e575c8460c51356336c651d6f1fb520ea9 Mon Sep 17 00:00:00 2001 From: Zoltan Csizmadia Date: Sun, 13 Sep 2026 11:43:31 -0500 Subject: [PATCH 1/5] mount: deliver Windows-side file changes to Linux inotify watchers (#118) A file changed by a Windows application raises no inotify event inside WSL2, so vite, nodemon, tsc --watch, jest --watch, air and cargo-watch silently never fire. Nothing errors, which is why people lose an afternoon to it before finding microsoft/WSL#4739. wsldrive already carries every far-side change across as an invalidation, so the mount knows what happened within milliseconds. What was missing was the last hop, and it cannot be taken directly: the kernel raises fsnotify events from the VFS, at the point an operation is performed. A filesystem cannot raise one itself, and the fuse_lowlevel_notify_* calls invalidate dentries and pages without ever touching fsnotify. So ask the kernel to raise it, by performing on the mount the operation the far side already performed. A thread in the mount process replays each change as an ordinary syscall against the mount's own path: a write as an mtime-only utimensat (fsnotify_change reports a change to both timestamps as FS_ATTRIB and to mtime alone as FS_MODIFY, so this is what yields IN_MODIFY rather than the weaker IN_ATTRIB), a creation as mknodat, a deletion as unlinkat, a rename as renameat - which is what produces a genuine move cookie, so a watcher sees one move instead of an unrelated delete and create. The events are the kernel's own, so any watcher sees them with no preload, plugin or cooperation of any kind. Those operations must not cross the boundary a second time, so the FUSE handlers answer the bridge's own requests locally. That test is deliberately over-determined - own thread, matching poke in flight, exact path, claimed once - because a false positive would be a user's rm silently not deleting. Two further guards keep the replay inside the mount: the bridge refuses to start unless its root reports FUSE_SUPER_MAGIC, and every poke re-checks its target's device. Getting the right event type out also meant carrying what happened, not just what the mirror must do about it. Invalidation ops now hold a change kind and a rename-pairing cookie (protocol version 4; a peer that ignores both stays correct). The kind survives coalescing - a create followed by three writes is still a creation - and every half-move is degraded to what it amounts to alone rather than reaching a consumer unpaired. Created versus modified is settled by the client against its own mirror, because the agent re-stats after the fact and cannot tell the two apart. Also adds op_mknod, which was unimplemented and failing with ENOSYS. On by default; wsldrive mount --no-inotify turns it off. Direction B only - a WinFsp volume raises Windows change notifications through its own mechanism. Known gap: an overflow on the far side sends a rescan, which names no path, so the bridge can only touch the mount root. Naming what changed means diffing the old tree against the new snapshot and is not implemented; docs/inotify.md records it alongside the rest. scripts/inotify-conformance.sh mounts a tree, changes it from the serving side, and asserts each change raises the right event type. It also checks the thing that would be worst to get wrong - that the served tree is untouched by the bridge's own operations - exercises a recursive watch over 10k files, and reports far-side against local event latency. CI runs it in the Linux conformance job. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 8 +- CHANGELOG.md | 19 ++ README.md | 28 ++- docs/inotify.md | 103 +++++++++ scripts/inotify-conformance.sh | 232 ++++++++++++++++++++ src/agent/client.cpp | 58 ++++- src/agent/client.hpp | 28 ++- src/agent/server.cpp | 28 ++- src/core/coalescer.cpp | 44 +++- src/core/coalescer.hpp | 44 +++- src/core/protocol.cpp | 13 ++ src/core/protocol.hpp | 8 +- src/core/types.hpp | 26 +++ src/mount/CMakeLists.txt | 2 +- src/mount/fsnotify_bridge.cpp | 341 +++++++++++++++++++++++++++++ src/mount/fsnotify_bridge.hpp | 213 ++++++++++++++++++ src/mount/fuse_mount.cpp | 119 +++++++++- src/mount/fuse_mount.hpp | 13 +- src/platform/linux/dir_watcher.cpp | 8 +- src/platform/win/dir_watcher.cpp | 30 ++- src/tools/wsldrive_main.cpp | 37 +++- tests/agent_test.cpp | 136 ++++++++++++ tests/coalescer_test.cpp | 122 +++++++++++ tests/protocol_test.cpp | 46 ++++ 24 files changed, 1658 insertions(+), 48 deletions(-) create mode 100644 docs/inotify.md create mode 100644 scripts/inotify-conformance.sh create mode 100644 src/mount/fsnotify_bridge.cpp create mode 100644 src/mount/fsnotify_bridge.hpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a42c37..20ced5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,7 +49,7 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install dependencies - run: sudo apt-get update && sudo apt-get install -y ninja-build libfuse3-dev fuse3 + run: sudo apt-get update && sudo apt-get install -y ninja-build libfuse3-dev fuse3 inotify-tools - name: Build run: | cmake --preset linux-release @@ -59,6 +59,12 @@ jobs: git config --global user.email ci@example.com git config --global user.name CI bash scripts/fs-conformance.sh + # The other half of "the mount behaves like a filesystem": a change made on + # the far side has to raise a real inotify event here, or every watch-mode + # tool silently does nothing. Which kernel hook each operation runs is not + # observable from a unit test, so it can only be checked on a live mount. + - name: Run the change-notification battery + run: bash scripts/inotify-conformance.sh # The same idea for Direction A - the feature the product is named for. Mounts # a real drive letter through WinFsp on the Windows runner and runs the Windows diff --git a/CHANGELOG.md b/CHANGELOG.md index 35db6d4..a9da668 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ ## Unreleased +### Added + +- **File changes made on the Windows side now fire `inotify` inside WSL.** A Direction B mount + delivers far-side changes to ordinary Linux watchers, so `vite`, `nodemon`, `tsc --watch`, + `jest --watch`, `air` and `cargo-watch` react to an edit made from a Windows editor. Watchers need + no cooperation of any kind. Create, write, delete and rename each raise the matching event type, + and a rename arrives as a paired `IN_MOVED_FROM`/`IN_MOVED_TO` with one cookie rather than as an + unrelated delete and create. This is [microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739), + which nothing has solved. On by default; `wsldrive mount --no-inotify` turns it off. How it works, + and what it does not cover, is in [`docs/inotify.md`](docs/inotify.md). +- `mknod` on a regular file now works on the mount, instead of failing with `ENOSYS`. + +### Changed + +- The wire protocol is at version 4. Every invalidation op carries what the watcher actually saw + (created, modified, removed, or one half of a move) alongside what the mirror should do about it, + plus a cookie pairing the two halves of a rename. A peer that ignores both fields stays correct. + The agent and the client must be the same version, as before. + ### Performance - **The mount lets the kernel cache file pages.** `auto_cache` replaces diff --git a/README.md b/README.md index 3edd39e..23b9291 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,8 @@ driver of its own (WinFsp on Windows, libfuse3 in WSL). Full numbers in [`bench/ Desktop, Explorer/Search, Unity, Office) reach a WSL source tree without paying the `\\wsl.localhost` Plan 9 tax. - **Direction B — a Windows drive mounted inside WSL2.** Linux tools read/write an NTFS tree without the - `/mnt/c` 9P/virtiofs tax. + `/mnt/c` 9P/virtiofs tax — and a Windows-side edit fires `inotify`, so + [watch mode works](#watch-mode-works-inotify-across-the-boundary). WSL2 only. WSL1 has no VM boundary (DrvFs runs in the NT kernel, `\\wsl$` is served in-process), so it has neither problem and is unsupported by design. @@ -120,7 +121,8 @@ Two small user-space binaries, no kernel drivers of wsldrive's own: it for changes (`ReadDirectoryChangesW`+IOCP on Windows, inotify on Linux), pushing coalesced invalidations. - **`wsldrive`** — the *client*: mounts the served tree as a filesystem (WinFsp on Windows, libfuse3 in - WSL, one FUSE3 implementation) backed by an in-RAM metadata mirror and a content cache. + WSL, one FUSE3 implementation) backed by an in-RAM metadata mirror and a content cache. In WSL it + also turns each invalidation into a real `inotify` event, so watch-mode tools see far-side changes. Either binary can host either role, so the same code serves both directions — the direction is just which side runs the agent and which runs the mount. @@ -188,6 +190,25 @@ explicit flag always wins over the probe. A `.wsldriveignore` at the served root (gitignore-style: `node_modules/`, `*.log`, `/build`, …) excludes paths from the mount and from sync. +### Watch mode works (`inotify` across the boundary) + +Edit a file from a Windows editor and `vite`, `nodemon`, `tsc --watch`, `jest --watch`, `air` and +`cargo-watch` reload — on a Direction B mount, with no plugin, no preload and no polling. + +They do not, on `/mnt/c` or anywhere else. A change made by a Windows application raises no `inotify` +event inside WSL2, so a Linux watcher never fires. Nothing errors; the tooling just quietly stops +reacting, which is why people lose an afternoon to it before finding +[microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739). + +wsldrive already carries every far-side change across as an invalidation, so the mount knows what +happened within milliseconds. It turns each one into a real kernel event by replaying the operation +on the mount itself — a write becomes an mtime-only `utimensat` (`IN_MODIFY`), a creation a `mknodat` +(`IN_CREATE`), a rename a `renameat`, so the two halves arrive paired under one cookie rather than as +an unrelated delete and create. The events are the kernel's own, so any watcher sees them. + +On by default. `wsldrive mount --no-inotify` turns it off. Mechanism, guarantees and the two things +it does not cover: [`docs/inotify.md`](docs/inotify.md). + ### What gets served (and what doesn't) The default root is your **home directory** (`~`) — that is where the work is, and it keeps the tree @@ -323,7 +344,8 @@ detected (Linux) — so CI and minimal builds are unaffected. `src/core` — platform-independent library (metadata tree, string pool, framed protocol, coalescer, auth token, ignore rules, name escaping, path utils), all unit-tested. `src/net` — sockets (TCP/vsock/hvsocket) and the framed channel. `src/platform` — watchers and process launchers per OS. -`src/agent` — the scanner, `RootServer`, and the `RemoteRoot` client. `src/mount` — the FUSE3 mount. +`src/agent` — the scanner, `RootServer`, and the `RemoteRoot` client. `src/mount` — the FUSE3 mount +and the `inotify` bridge. `src/tools` — the `wsldrive` and `wsldrived` binaries. `tests`, `bench`, `scripts` as named. ## Security diff --git a/docs/inotify.md b/docs/inotify.md new file mode 100644 index 0000000..23669af --- /dev/null +++ b/docs/inotify.md @@ -0,0 +1,103 @@ +# Change notification across the boundary + +A file changed by a Windows application does not fire `inotify` inside WSL2. Nothing errors. +`vite`, `webpack --watch`, `nodemon`, `jest --watch`, `tsc --watch`, `air` and `cargo-watch` all +just sit there. It is [microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739), and it is +the reason people who keep their source on the Windows side end up polling. + +A wsldrive Direction B mount delivers those events. Editing a file from a Windows editor raises a +real `IN_MODIFY` on the mount, a new file raises `IN_CREATE` in its directory, a rename raises a +paired `IN_MOVED_FROM`/`IN_MOVED_TO`, and a deletion raises `IN_DELETE`. Watchers need no +cooperation: no preload, no plugin, no knowledge that wsldrive is involved. + +It is on by default. `wsldrive mount --no-inotify` turns it off. + +## Why it needs a mechanism at all + +The kernel raises fsnotify events from the VFS, at the point an operation is performed. +`vfs_create` calls `fsnotify_create`, `vfs_unlink` calls `fsnotify_unlink`, `notify_change` calls +`fsnotify_change`. A filesystem cannot raise one itself, and FUSE offers nothing that does: the +`fuse_lowlevel_notify_*` calls invalidate dentries and pages, which is cache coherence, not +notification. `fuse_reverse_inval_entry` never touches fsnotify. So a userspace daemon has no way to +hand an inotify watcher an event, and this is why the problem has outlived so many attempts at it. + +## What wsldrive does + +It asks the kernel to raise the event, by performing on the mount the operation the far side already +performed. A thread inside the mount process replays each change as an ordinary syscall against the +mount's own path: + +| far side | wsldrive replays | kernel raises | +|---|---|---| +| file written | `utimensat`, mtime only | `IN_MODIFY` | +| file created | `mknodat` (regular file) | `IN_CREATE` | +| directory created | `mkdirat` | `IN_CREATE` with `IN_ISDIR` | +| file deleted | `unlinkat` | `IN_DELETE` | +| directory deleted | `unlinkat(AT_REMOVEDIR)` | `IN_DELETE` with `IN_ISDIR` | +| renamed | `renameat` | `IN_MOVED_FROM` + `IN_MOVED_TO`, one cookie | + +The `utimensat` detail matters. `fsnotify_change()` reports a change to *both* timestamps as +`FS_ATTRIB` and a change to mtime alone as `FS_MODIFY`, so touching only mtime is what turns a +far-side write into `IN_MODIFY` rather than the weaker `IN_ATTRIB`. And letting the kernel perform +the rename is what produces a genuine move cookie, so a watcher sees one move instead of an +unrelated delete and create. + +These operations must not cross the boundary a second time — the far side already has this state, +and re-applying it would at best waste a round trip and at worst destroy the file that prompted the +event. So the FUSE handlers recognise the bridge's own requests and answer them locally. That test +is deliberately over-determined, because the cost of getting it wrong is a user's `rm` silently not +removing anything: the request must come from the bridge's own thread, a poke of exactly that kind +must be in flight, and it must name exactly that path. A poke is claimed once and then retired. + +Two further guards keep the replay inside the mount. The bridge refuses to start unless its root is +a FUSE mount (`statfs` reports `FUSE_SUPER_MAGIC`), and every poke re-checks that its target's +parent is still on that mount's device. Both exist because the pokes are real filesystem calls: aimed +at the wrong tree they would create and delete real files there. + +## Two small fictions + +Both are visible only to the bridge's own thread; every other caller gets the mirror's real answer. + +The invalidation is applied to the metadata mirror before the poke runs, so the mirror is already +telling the truth by the time the kernel looks — which is exactly wrong for making the kernel run +the operation. A creation would find the path already there and never reach `->mknod`; a deletion +would find nothing and never reach `->unlink`. So for the length of one syscall the mount reports a +newly created path as absent, and a newly deleted one as still present. Neither fiction outlives +its call: the create or unlink that follows settles the dentry, and the record is retired the moment +the handler claims it. + +## What it costs + +One syscall per changed path, answered by the FUSE loop out of the in-RAM mirror. No boundary +crossing and no I/O, so the added latency over a write made locally on the mount is the mount's own +round trip. `scripts/inotify-conformance.sh` measures both and prints them side by side. + +The queue is bounded at 65536 pending changes. Past that the oldest are dropped, and `wsldrive mount` +says so on exit. A dropped notification is a late one, not a wrong one: the mirror and the page cache +are already correct, so the file reads right the moment anything looks at it. + +## Limits + +- **Direction B only.** Direction A does not have this problem: a WinFsp volume raises Windows change + notifications through its own mechanism. +- **An overflow cannot name paths.** When the far side's watcher loses events it sends a rescan, the + mirror is rebuilt from a fresh snapshot, and the bridge touches the mount root. A watcher that + re-walks on any event below its root will catch up; one waiting for a specific path will not be + told about it. Making an overflow name what changed means diffing the old tree against the new + snapshot, which is not implemented. In practice the Windows watcher's 1 MiB buffer keeps overflows + rare outside of bursts on the scale of an `npm install`. +- **`inotify` only.** `fanotify` marks are not delivered, and neither are the `IN_OPEN`, `IN_ACCESS` + or `IN_CLOSE` classes of event — nothing on the far side reports those, so there is nothing to + replay. +- **The mount's own watch limits apply.** `max_user_watches` bounds a recursive watcher on the mount + exactly as it would on any local filesystem; wsldrive neither raises nor consumes that budget. + +## Checking it + +`scripts/inotify-conformance.sh` mounts a tree, changes it from the serving side, and asserts that +each change raises the right event type on the mount within a timeout. It also checks the thing that +would be worst to get wrong — that the served tree is untouched by the bridge's own operations — and +exercises a recursive watch over a 10,000-file tree. CI runs it on every change. + +To watch the event stream by hand without a mount, `wsldrive fetch --connect --watch` +prints each invalidation with its change kind and rename cookie. diff --git a/scripts/inotify-conformance.sh b/scripts/inotify-conformance.sh new file mode 100644 index 0000000..237a2c1 --- /dev/null +++ b/scripts/inotify-conformance.sh @@ -0,0 +1,232 @@ +#!/usr/bin/env bash +# Change-notification conformance check for a wsldrive mount. +# +# The claim under test is the one from microsoft/WSL#4739: a file changed on the +# *far* side of the boundary fires a real inotify event on this side, so an +# unmodified watch-mode tool reacts to it. Nothing about that can be checked +# from a unit test — it depends on which kernel hook each operation runs, which +# only happens against a live mount. +# +# The far side here is a Linux agent rather than a Windows one, because CI has +# no Windows peer. That is the same code path: the agent watches its tree, the +# client applies the invalidation, and the bridge turns it into a local event. +# What a Windows agent changes is which platform watcher produces the event, and +# that part is covered by the unit tests. +# +# scripts/inotify-conformance.sh [build-dir] +# +# Exit status is the number of failed checks (0 = all good). + +set -u +BUILD=${1:-build/linux-release} +AGENT="$BUILD/src/tools/wsldrived" +CLI="$BUILD/src/tools/wsldrive" +PORT=${PORT:-51998} +# How long a notification may take to arrive. Generous: the agent coalesces for +# up to 25 ms, and a loaded CI runner adds more. The measured figure is reported +# separately at the end. +TIMEOUT=${TIMEOUT:-15} + +command -v inotifywait >/dev/null || { echo "inotifywait not found (apt install inotify-tools)"; exit 99; } +for exe in "$AGENT" "$CLI"; do + [ -x "$exe" ] || { echo "not built: $exe"; exit 99; } +done + +WORK=$(mktemp -d) # the served tree: stands in for the Windows side +MNT=$(mktemp -d) # the mount: stands in for the Linux side +LOG=$(mktemp -d) +export WSLDRIVE_TOKEN="inotify-conformance-$$" +cleanup() { + [ -n "${WATCH_PID:-}" ] && kill "$WATCH_PID" 2>/dev/null + fusermount3 -u "$MNT" 2>/dev/null + [ -n "${CLI_PID:-}" ] && kill "$CLI_PID" 2>/dev/null + [ -n "${AGENT_PID:-}" ] && kill "$AGENT_PID" 2>/dev/null + rm -rf "$WORK" "$MNT" "$LOG" 2>/dev/null + return 0 +} +trap cleanup EXIT + +mkdir -p "$WORK/src" "$WORK/keep" +echo "original" > "$WORK/src/app.js" +echo "static" > "$WORK/keep/untouched.txt" + +echo "serving $WORK -> $MNT" +"$AGENT" --root "$WORK" --listen "tcp://127.0.0.1:$PORT" >"$LOG/agent.log" 2>&1 & +AGENT_PID=$! +sleep 2 +"$CLI" mount "$MNT" --connect "tcp://127.0.0.1:$PORT" >"$LOG/mount.log" 2>&1 & +CLI_PID=$! +for _ in $(seq 1 60); do sleep 1; mountpoint -q "$MNT" && break; done +mountpoint -q "$MNT" || { echo "MOUNT FAILED"; tail -20 "$LOG/mount.log"; exit 98; } +grep -q "inotify watchers" "$LOG/mount.log" || echo " note: the mount did not report the bridge as enabled" + +PASS=0; FAIL=0 +ok() { PASS=$((PASS+1)); printf ' ok %s\n' "$1"; } +bad() { FAIL=$((FAIL+1)); printf ' FAIL %s\n' "$1"; } +# awk rather than bc: bc is not installed on a stock CI runner. +elapsed_ms() { awk -v a="$2" -v b="$1" 'BEGIN { printf "%.0f", (a - b) * 1000 }'; } + +# One watcher for the whole run, recursive over the mount, writing every event +# to a file. A fresh inotifywait per check would race with the change it is +# meant to see; this way the log is always already listening. +EVENTS="$LOG/events" +: > "$EVENTS" +inotifywait -m -r -q --format '%e %w%f' -o "$EVENTS" "$MNT" & +WATCH_PID=$! +# inotifywait establishes its watches asynchronously and drops anything that +# happens first, so wait until it says it is ready rather than guessing. +for _ in $(seq 1 100); do + touch "$MNT/.probe" 2>/dev/null + grep -q '.probe' "$EVENTS" 2>/dev/null && break + sleep 0.1 +done +rm -f "$MNT/.probe" 2>/dev/null +: > "$EVENTS" + +# Waits for a line matching an event-name pattern and a path, and reports how +# long it took. `$2` is an extended regex over the event names as inotifywait +# prints them (e.g. 'CREATE', 'MOVED_(FROM|TO)'). +await() { + local what="$1" events="$2" path="$3" start now + start=$(date +%s.%N) + for _ in $(seq 1 $((TIMEOUT * 20))); do + if grep -Eq "^[A-Z_,]*($events)[A-Z_,]* $MNT/$path\$" "$EVENTS" 2>/dev/null; then + now=$(date +%s.%N) + printf ' ok %-46s %6.0f ms\n' "$what" "$(elapsed_ms "$start" "$now")" + PASS=$((PASS+1)) + return 0 + fi + sleep 0.05 + done + bad "$what" + echo " wanted: $events on $path" + echo " events seen:" + sed 's/^/ | /' "$EVENTS" | tail -15 + return 1 +} + +echo +echo "== each change on the served tree must raise the matching inotify event ==" + +# Write. The one that matters most: this is a Windows editor saving a file, and +# the event every watch-mode tool is waiting for. +echo "edited by the other side" > "$WORK/src/app.js" +await "write -> MODIFY" "MODIFY" "src/app.js" + +# Create. +echo "brand new" > "$WORK/src/added.ts" +await "create -> CREATE" "CREATE" "src/added.ts" + +# Create a directory. +mkdir "$WORK/src/components" +await "mkdir -> CREATE,ISDIR" "CREATE" "src/components" + +# Rename. Both halves must arrive, which is what tells a watcher this is a move +# and not an unrelated delete plus create. +mv "$WORK/src/added.ts" "$WORK/src/renamed.ts" +await "rename -> MOVED_FROM (old name)" "MOVED_FROM" "src/added.ts" +await "rename -> MOVED_TO (new name)" "MOVED_TO" "src/renamed.ts" + +# Delete. +rm "$WORK/src/renamed.ts" +await "delete -> DELETE" "DELETE" "src/renamed.ts" + +# Delete a directory. +rmdir "$WORK/src/components" +await "rmdir -> DELETE,ISDIR" "DELETE" "src/components" + +echo +echo "== the mount must stay correct while all that is going on ==" +if [ "$(cat "$MNT/src/app.js")" = "edited by the other side" ]; then + ok "the new contents read back through the mount" +else + bad "the new contents read back through the mount" + echo " got: $(cat "$MNT/src/app.js" 2>&1)" +fi +if [ -e "$MNT/src/renamed.ts" ] || [ -e "$MNT/src/added.ts" ]; then + bad "a deleted file is gone from the mount" + ls -la "$MNT/src" +else + ok "a deleted file is gone from the mount" +fi +# The bridge performs real filesystem calls against the mount. If any of them +# were forwarded across the boundary instead of being recognised as its own, +# the served tree would be damaged — this is the check that would catch it. +if [ "$(cat "$WORK/keep/untouched.txt")" = "static" ] && [ "$(cat "$WORK/src/app.js")" = "edited by the other side" ]; then + ok "the served tree is untouched by the bridge" +else + bad "the served tree is untouched by the bridge" + echo " the bridge's own operations must never reach the agent" + ls -laR "$WORK" +fi + +echo +echo "== recursive watch over a large tree ==" +# The scale question from the issue: a Node-sized project is tens of thousands +# of paths, and both the watcher and the bridge have to survive one changing at +# once without overflowing into a rescan that names nothing. +BIG=$WORK/big +mkdir -p "$BIG" +for d in $(seq 1 100); do + mkdir -p "$BIG/d$d" + for f in $(seq 1 100); do echo "$f" > "$BIG/d$d/f$f.js"; done +done +# Let the burst settle, then check that the tree arrived and a change deep +# inside it still notifies. A burst this size overruns the watcher and becomes a +# rescan, which is the slow path on purpose (it is rate-limited), so wait for the +# count rather than assuming one interval is enough. +COUNT=0 +for _ in $(seq 1 60); do + COUNT=$(find "$MNT/big" -type f 2>/dev/null | wc -l) + [ "$COUNT" -eq 10000 ] && break + sleep 1 +done +if [ "$COUNT" -eq 10000 ]; then + ok "10000 files appeared on the mount" +else + bad "10000 files appeared on the mount (saw $COUNT)" +fi +: > "$EVENTS" +echo "deep change" > "$BIG/d50/f50.js" +await "write deep in a 10k-file tree -> MODIFY" "MODIFY" "big/d50/f50.js" + +echo +echo "== latency, against a write made locally on the mount ==" +# The comparison the issue asks for: the same event, once originating on the far +# side and once originating here, so the added cost of crossing the boundary is +# what separates the two numbers. +local_ms() { + : > "$EVENTS" + local start now + start=$(date +%s.%N) + echo "local" > "$MNT/keep/local.txt" + for _ in $(seq 1 200); do + grep -q "$MNT/keep/local.txt" "$EVENTS" 2>/dev/null && break + sleep 0.01 + done + now=$(date +%s.%N) + elapsed_ms "$start" "$now" +} +remote_ms() { + : > "$EVENTS" + local start now + start=$(date +%s.%N) + echo "remote $RANDOM" > "$WORK/keep/remote.txt" + for _ in $(seq 1 200); do + grep -q "$MNT/keep/remote.txt" "$EVENTS" 2>/dev/null && break + sleep 0.01 + done + now=$(date +%s.%N) + elapsed_ms "$start" "$now" +} +printf ' local write -> event: %6.0f ms\n' "$(local_ms)" +printf ' far-side -> event: %6.0f ms\n' "$(remote_ms)" +printf ' far-side -> event: %6.0f ms (warm)\n' "$(remote_ms)" + +echo +echo "passed: $PASS failed: $FAIL" +if [ "$FAIL" -ne 0 ]; then + echo "--- agent log (tail) ---"; tail -20 "$LOG/agent.log" 2>/dev/null + echo "--- mount log (tail) ---"; tail -20 "$LOG/mount.log" 2>/dev/null +fi +exit $FAIL diff --git a/src/agent/client.cpp b/src/agent/client.cpp index 510aaae..e8b3ea1 100644 --- a/src/agent/client.cpp +++ b/src/agent/client.cpp @@ -1,6 +1,7 @@ #include "agent/client.hpp" #include "core/auth_token.hpp" +#include "core/coalescer.hpp" #include "core/path.hpp" #include "core/version.hpp" @@ -568,8 +569,20 @@ void RemoteRoot::rescan_loop() { std::lock_guard lock(pf_mu_); pf_seen_.clear(); } - std::lock_guard s(stats_mu_); - ++stats_.rescans; + // Tell the change-notification consumer that the mirror was replaced, now + // that it has been. Nothing can name what changed - that is what an + // overflow means - so this is the one signal it gets, and it goes out after + // the new snapshot has landed rather than when the Rescan arrived. + InvalidatedPathsHook paths_hook; + { + std::lock_guard s(stats_mu_); + ++stats_.rescans; + paths_hook = paths_hook_; + } + if (paths_hook) { + const AppliedChange rescan_change{ChangeKind::Unknown, std::string{}, 0, NodeKind::Directory, true}; + paths_hook(std::span(&rescan_change, 1)); + } } } @@ -969,11 +982,11 @@ void RemoteRoot::apply_invalidation(std::span payload) { // Directories the batch removes. Collected while the tree can still say they // were directories, and swept out of the content cache once the lock is free. std::vector removed_dirs; - // Paths this batch actually changed. An op discarded as stale relative to + // What this batch actually changed. An op discarded as stale relative to // this client's own mutation did not change the mirror, so it must not be // reported as invalidated either - a consumer that drops caches for it would // be acting on an event we already decided was obsolete. - std::vector applied; + std::vector applied; { std::unique_lock lock(tree_mu_); for (const proto::InvalidationOp& op : batch->ops) { @@ -987,14 +1000,36 @@ void RemoteRoot::apply_invalidation(std::span payload) { local_mutations_.erase(it); // caught up; back to normal } } - if (op.kind != InvalidationKind::Rescan) applied.push_back(op.path); switch (op.kind) { - case InvalidationKind::Upsert: (void)tree_.upsert_path(op.path, op.attr); break; - case InvalidationKind::Remove: - if (const auto id = tree_.lookup(op.path, mode()); id && tree_.node(*id).is_dir()) - removed_dirs.push_back(op.path); + case InvalidationKind::Upsert: { + // Created or modified is decided here, against the mirror, because + // this is the only side that knows what it held a moment ago. The + // agent cannot tell the two apart: both look like "the path exists + // and something touched it" from a stat after the fact. + const bool existed = tree_.lookup(op.path, mode()).has_value(); + ChangeKind change = op.change; + if (!is_move(change)) change = existed ? ChangeKind::Modified : ChangeKind::Created; + applied.push_back(AppliedChange{change, op.path, op.cookie, op.attr.kind, false}); + (void)tree_.upsert_path(op.path, op.attr); + break; + } + case InvalidationKind::Remove: { + const auto id = tree_.lookup(op.path, mode()); + if (id && tree_.node(*id).is_dir()) removed_dirs.push_back(op.path); + // Nothing was there to remove. The mirror is already right, and a + // consumer told a path it never saw has gone away would report a + // deletion that never happened. The source half of a move is the + // exception: its partner's arrival is real either way, and dropping + // half of a pair costs the consumer the move. + if (id || op.change == ChangeKind::MovedFrom) { + const NodeKind kind = id ? tree_.node(*id).attr.kind : NodeKind::File; + applied.push_back(AppliedChange{op.change == ChangeKind::MovedFrom ? ChangeKind::MovedFrom + : ChangeKind::Removed, + op.path, op.cookie, kind, false}); + } (void)tree_.remove_path(op.path, mode()); break; + } case InvalidationKind::Rescan: rescan = true; break; // handled below, off this thread } } @@ -1009,6 +1044,11 @@ void RemoteRoot::apply_invalidation(std::span payload) { else snapshot_replay_overflow_ = true; } } + // Dropping a removal for a path the mirror never had can strand the other + // half of a move, and so can a frame boundary: a batch too large for one + // frame is broadcast in several, and the two halves may land in different + // ones. Either way the surviving half is degraded rather than left dangling. + repair_move_pairs(applied); for (const std::string& d : removed_dirs) drop_cached_prefix(d); if (rescan) { // The agent's watcher overflowed: per-path events were lost, so the mirror diff --git a/src/agent/client.hpp b/src/agent/client.hpp index f939fd7..3761c7f 100644 --- a/src/agent/client.hpp +++ b/src/agent/client.hpp @@ -52,12 +52,32 @@ class RemoteRoot { std::uint64_t rescan_failures = 0; }; + /// One far-side change this client actually applied to its mirror, described + /// the way a change-notification consumer needs it rather than the way the + /// mirror needed it. + /// + /// The difference is what the mirror knows and the agent does not. The agent + /// reports that a path exists and has changed; only the mirror can say + /// whether it had that path a moment ago, which is what separates a file + /// being created from the same file being rewritten. `change` here is that + /// verdict, already made. + struct AppliedChange { + ChangeKind change = ChangeKind::Unknown; + std::string path; // relative, '/'-separated + std::uint32_t cookie = 0; // pairs the two halves of a move + NodeKind kind = NodeKind::File; // what the path is — for a removal, what it was + // The whole mirror was replaced: the agent's watcher lost events, so no + // per-path change can be named. `path` is empty and every other field + // meaningless. Delivered after the replacement snapshot has landed. + bool rescan = false; + }; + using InvalidationHook = std::function; - /// Paths whose mirrored state a batch actually changed - the ops that were - /// applied, not the ones discarded as stale relative to this client's own - /// mutation. Separate from InvalidationHook, which reports the raw batch for + /// Changes a batch actually made to the mirror - the ops that were applied, + /// not the ones discarded as stale relative to this client's own mutation. + /// Separate from InvalidationHook, which reports the raw batch for /// diagnostics (`wsldrive watch`) and so has different callers. - using InvalidatedPathsHook = std::function)>; + using InvalidatedPathsHook = std::function)>; explicit RemoteRoot(std::unique_ptr ch); ~RemoteRoot(); diff --git a/src/agent/server.cpp b/src/agent/server.cpp index 16de04d..cc75eab 100644 --- a/src/agent/server.cpp +++ b/src/agent/server.cpp @@ -123,6 +123,8 @@ void RootServer::flush_loop() { proto::InvalidationOp out; out.kind = op.kind; out.path = std::move(op.path); + out.change = op.change; + out.cookie = op.cookie; if (out.kind != InvalidationKind::Rescan) { // Both kinds are resolved against the disk NOW, not against the event. // A Remove that is not re-checked is a real hazard: git renames @@ -133,11 +135,28 @@ void RootServer::flush_loop() { auto attr = read_attributes(join_relative(opts_.root, out.path)); if (!attr) { out.kind = InvalidationKind::Remove; // gone (or vanished between the event and now) + // The disk overrules the event for the change kind too. Gone means + // removed, unless the event already said this is the source half of a + // move — which is the one reading of "gone" that carries more + // information, so it survives with its cookie. + if (out.change != ChangeKind::MovedFrom) { + out.change = ChangeKind::Removed; + out.cookie = 0; + } } else if (attr->kind == NodeKind::Other) { continue; } else { out.kind = InvalidationKind::Upsert; // present, whatever the event said out.attr = *attr; + // Present, so it is not gone and not the source of a move. Say only + // what is still true: something changed here. A peer that tracks + // change notifications decides between "created" and "modified" from + // whether its own mirror already had the path — it is the only side + // that knows, and it has to make that call regardless. + if (out.change == ChangeKind::Removed || out.change == ChangeKind::MovedFrom) { + out.change = ChangeKind::Modified; + out.cookie = 0; + } } } // A directory that has just appeared (mkdir, or - the case that matters - @@ -149,6 +168,10 @@ void RootServer::flush_loop() { batch.ops.push_back(std::move(out)); } if (!expand_dirs.empty()) append_subtrees(std::move(expand_dirs), batch.ops); + // Re-stating every path above can reclassify one half of a move (its source + // exists again, or its destination is already gone) and leave the other + // half pointing at a partner this batch no longer contains. + repair_move_pairs(batch.ops); if (!batch.ops.empty()) broadcast_batch(batch); lock.lock(); } @@ -178,7 +201,10 @@ void RootServer::append_subtrees(std::vector dirs, std::vector opts_.max_expanded_entries) over = true; - if (!over && !present.contains(p)) ops.push_back(proto::InvalidationOp{InvalidationKind::Upsert, p, e.attr}); + // The whole subtree is arriving at once, so every entry in it is new + // to the peer even though no watcher event named it. + if (!over && !present.contains(p)) + ops.push_back(proto::InvalidationOp{InvalidationKind::Upsert, p, e.attr, ChangeKind::Created, 0}); rels.push_back(std::move(p)); }, // Once over budget, decline every subdirectory so the scan winds down diff --git a/src/core/coalescer.cpp b/src/core/coalescer.cpp index 0d32eeb..d414869 100644 --- a/src/core/coalescer.cpp +++ b/src/core/coalescer.cpp @@ -3,6 +3,7 @@ #include "core/path.hpp" #include +#include namespace wsld { @@ -19,18 +20,32 @@ void Coalescer::push(const FsEvent& ev, clock::time_point now) { InvalidationKind kind; bool appeared = false; + ChangeKind change; + std::uint32_t cookie = 0; switch (ev.kind) { case FsEventKind::Created: + kind = InvalidationKind::Upsert; + appeared = true; + change = ChangeKind::Created; + break; case FsEventKind::RenamedTo: kind = InvalidationKind::Upsert; appeared = true; + change = ChangeKind::MovedTo; + cookie = ev.cookie; break; case FsEventKind::Modified: kind = InvalidationKind::Upsert; + change = ChangeKind::Modified; break; case FsEventKind::Removed: + kind = InvalidationKind::Remove; + change = ChangeKind::Removed; + break; case FsEventKind::RenamedFrom: kind = InvalidationKind::Remove; + change = ChangeKind::MovedFrom; + cookie = ev.cookie; break; case FsEventKind::Overflow: default: @@ -40,9 +55,20 @@ void Coalescer::push(const FsEvent& ev, clock::time_point now) { if (auto it = pending_.find(ev.path); it != pending_.end()) { // A Modified after a Created keeps the entry "new"; a removal resets it. const bool still_new = kind == InvalidationKind::Upsert && (appeared || it->second.appeared); - it->second = Entry{kind, ++seq_, still_new}; + // Same idea for the reported change: writes that land on a path this batch + // has already seen appear (or move) into place do not turn it into a plain + // modification — the consumer still has to be told the path is new, and a + // move must keep the cookie that pairs it with its other half. Every other + // event supersedes what came before it. + const bool keep = change == ChangeKind::Modified && + (it->second.change == ChangeKind::Created || it->second.change == ChangeKind::MovedTo); + if (keep) { + change = it->second.change; + cookie = it->second.cookie; + } + it->second = Entry{kind, ++seq_, still_new, change, cookie}; } else { - pending_.emplace(std::string(ev.path), Entry{kind, ++seq_, appeared}); + pending_.emplace(std::string(ev.path), Entry{kind, ++seq_, appeared, change, cookie}); } } @@ -75,10 +101,13 @@ std::vector Coalescer::take() { InvalidationKind kind; std::uint64_t seq; bool appeared; + ChangeKind change; + std::uint32_t cookie; }; std::vector items; items.reserve(pending_.size()); - for (const auto& [path, e] : pending_) items.push_back(Item{path, e.kind, e.seq, e.appeared}); + for (const auto& [path, e] : pending_) + items.push_back(Item{path, e.kind, e.seq, e.appeared, e.change, e.cookie}); // Sort by path so that a removed directory is immediately followed by its // descendants; drop descendants whose last event predates the removal. @@ -99,8 +128,15 @@ std::vector Coalescer::take() { // Emit in arrival order of each path's final event. std::sort(kept.begin(), kept.end(), [](const Item& a, const Item& b) { return a.seq < b.seq; }); + out.reserve(kept.size()); - for (const Item& it : kept) out.push_back(PlannedOp{it.kind, std::string(it.path), it.appeared}); + for (const Item& it : kept) + out.push_back(PlannedOp{it.kind, std::string(it.path), it.appeared, it.change, it.cookie}); + + // Collapsing can strand one half of a move: its partner is written again and + // keeps the move kind while this one is re-created and loses it, or a removed + // directory takes the partner out of the batch entirely. + repair_move_pairs(out); pending_.clear(); return out; diff --git a/src/core/coalescer.hpp b/src/core/coalescer.hpp index 75c2137..69ef937 100644 --- a/src/core/coalescer.hpp +++ b/src/core/coalescer.hpp @@ -28,6 +28,12 @@ enum class FsEventKind : std::uint8_t { struct FsEvent { FsEventKind kind; std::string_view path; + // Ties the two halves of a rename together. inotify supplies one per move + // pair; the Windows watcher mints one for each adjacent OLD_NAME/NEW_NAME + // pair, since ReadDirectoryChangesW has no equivalent. Zero means "not part + // of a pair we could identify", which a consumer must degrade gracefully on: + // an unpaired RenamedFrom is a removal, an unpaired RenamedTo a creation. + std::uint32_t cookie = 0; }; /// A planned invalidation. Attributes are not resolved here: the watcher side @@ -40,12 +46,46 @@ struct PlannedOp { // the watcher never reports, so the sender enumerates it; a Modified on a // directory the peers already know must not trigger that scan. bool appeared = false; + // What the watcher actually saw, kept alongside `kind` for the change + // notification path. Collapsing preserves the shape of the burst rather than + // its last event: a create followed by three writes is still a creation. + ChangeKind change = ChangeKind::Unknown; + std::uint32_t cookie = 0; // meaningful only for the two move kinds friend bool operator==(const PlannedOp& a, const PlannedOp& b) noexcept { - return a.kind == b.kind && a.path == b.path; // `appeared` is advisory + return a.kind == b.kind && a.path == b.path; // the rest is advisory } }; +/// Degrades every move half in `ops` that has no partner into the plain change +/// it amounts to on its own: a source with no destination is a removal, a +/// destination with no source is a creation. +/// +/// Halves get separated twice on the way out: the coalescer can collapse one of +/// them away, and the sender re-stats every path afterwards and may reclassify +/// one of them. Running this after each step keeps one invariant true for +/// consumers — a move that reaches them is always matched — so none of them has +/// to carry its own half-move fallback. +/// +/// Works on anything with `change` and `cookie` members (PlannedOp on the way +/// out of the coalescer, proto::InvalidationOp on the way onto the wire). +template +void repair_move_pairs(std::vector& ops) { + std::unordered_map halves; // cookie -> bitmask of halves seen + for (const Op& op : ops) { + if (op.cookie == 0 || !is_move(op.change)) continue; + halves[op.cookie] |= op.change == ChangeKind::MovedFrom ? 1 : 2; + } + if (halves.empty()) return; + for (Op& op : ops) { + if (!is_move(op.change)) continue; + const auto it = halves.find(op.cookie); + if (op.cookie != 0 && it != halves.end() && it->second == 3) continue; + op.change = op.change == ChangeKind::MovedFrom ? ChangeKind::Removed : ChangeKind::Created; + op.cookie = 0; + } +} + /// Collapses bursts of watcher events into a minimal ordered batch. /// /// - Repeated events on one path collapse to the last relevant operation. @@ -88,6 +128,8 @@ class Coalescer { InvalidationKind kind; std::uint64_t seq; bool appeared; + ChangeKind change; + std::uint32_t cookie; }; Options opts_; diff --git a/src/core/protocol.cpp b/src/core/protocol.cpp index c9f157d..9e4fc80 100644 --- a/src/core/protocol.cpp +++ b/src/core/protocol.cpp @@ -264,6 +264,10 @@ void write_invalidation(Writer& w, const InvalidationBatch& b) { w.u8(static_cast(op.kind)); w.string(op.path); if (op.kind == InvalidationKind::Upsert) write_attributes(w, op.attr); + // One extra byte per op in the common case; the cookie rides along only for + // the two move kinds, which are a small minority of any real batch. + w.u8(static_cast(op.change)); + if (is_move(op.change)) w.u32(op.cookie); } } @@ -290,6 +294,15 @@ Result read_invalidation(Reader& r) noexcept { if (!attr) return fail(attr.error()); op.attr = *attr; } + auto change = r.u8(); + if (!change) return fail(change.error()); + if (*change > kMaxChangeKind) return fail(Errc::Corrupt); + op.change = static_cast(*change); + if (is_move(op.change)) { + auto cookie = r.u32(); + if (!cookie) return fail(cookie.error()); + op.cookie = *cookie; + } b.ops.push_back(std::move(op)); } return b; diff --git a/src/core/protocol.hpp b/src/core/protocol.hpp index 35f3327..0b4d503 100644 --- a/src/core/protocol.hpp +++ b/src/core/protocol.hpp @@ -22,7 +22,9 @@ namespace wsld::proto { inline constexpr std::uint32_t kMagic = 0x444C5357; // "WSLD" when read as little-endian bytes -inline constexpr std::uint16_t kVersion = 3; // 3: mutual handshake (nonces + proofs, no token on the wire) +// 3: mutual handshake (nonces + proofs, no token on the wire) +// 4: invalidation ops carry the change kind and a rename-pairing cookie +inline constexpr std::uint16_t kVersion = 4; inline constexpr std::size_t kHeaderSize = 24; inline constexpr std::uint32_t kMaxPayload = 64u << 20; // 64 MiB per frame @@ -160,6 +162,10 @@ struct InvalidationOp { InvalidationKind kind; std::string path; // normalised, '/'-separated, relative to the mount root Attributes attr; // meaningful for Upsert only + // What the watcher saw, for peers that deliver change notifications rather + // than only mirroring metadata. Ignoring both fields leaves a peer correct. + ChangeKind change = ChangeKind::Unknown; + std::uint32_t cookie = 0; // pairs the two halves of a move; 0 otherwise }; struct InvalidationBatch { diff --git a/src/core/types.hpp b/src/core/types.hpp index 3182291..f851b3b 100644 --- a/src/core/types.hpp +++ b/src/core/types.hpp @@ -38,4 +38,30 @@ enum class InvalidationKind : std::uint8_t { Rescan = 2, // the watcher lost events; refetch a snapshot of this subtree }; +/// What actually happened to a path, as opposed to what a mirror must do about +/// it. InvalidationKind is all a metadata mirror needs: refresh the node, or +/// drop it. A change-notification consumer needs more — it has to tell a file +/// that appeared from one that was rewritten, and it has to see the two halves +/// of a rename as one move rather than as an unrelated removal and creation. +/// +/// The two travel together in every invalidation op. Nothing in the mirror +/// depends on this field, so a consumer that ignores it stays correct. +enum class ChangeKind : std::uint8_t { + Unknown = 0, // no watcher event behind this op (a subtree expansion, a rescan) + Created = 1, + Modified = 2, + Removed = 3, + MovedFrom = 4, // source half of a rename, paired with its MovedTo by cookie + MovedTo = 5, // destination half +}; + +/// Highest valid ChangeKind value, for validating one off the wire. Derived, so +/// adding a kind cannot leave it behind. +inline constexpr std::uint8_t kMaxChangeKind = static_cast(ChangeKind::MovedTo); + +/// True for the two halves of a rename, the only ops that carry a cookie. +[[nodiscard]] constexpr bool is_move(ChangeKind c) noexcept { + return c == ChangeKind::MovedFrom || c == ChangeKind::MovedTo; +} + } // namespace wsld diff --git a/src/mount/CMakeLists.txt b/src/mount/CMakeLists.txt index 4f31e4b..05834d5 100644 --- a/src/mount/CMakeLists.txt +++ b/src/mount/CMakeLists.txt @@ -1,4 +1,4 @@ -add_library(wsldrive_mount STATIC fuse_mount.cpp) +add_library(wsldrive_mount STATIC fuse_mount.cpp fsnotify_bridge.cpp) add_library(wsldrive::mount ALIAS wsldrive_mount) # WSLDRIVE_HAVE_MOUNT is PUBLIC so the CLI enables the mount subcommand on either diff --git a/src/mount/fsnotify_bridge.cpp b/src/mount/fsnotify_bridge.cpp new file mode 100644 index 0000000..82fe735 --- /dev/null +++ b/src/mount/fsnotify_bridge.cpp @@ -0,0 +1,341 @@ +#include "mount/fsnotify_bridge.hpp" + +#include +#include +#include +#include +#include + +#ifdef __linux__ +#include +#include +#include +#include +#include +#include + +#ifndef FUSE_SUPER_MAGIC +#define FUSE_SUPER_MAGIC 0x65735546 +#endif +#endif + +namespace wsld::mount { + +namespace { + +#ifdef __linux__ +long this_tid() noexcept { return static_cast(::syscall(SYS_gettid)); } +#endif + +} // namespace + +FsNotifyBridge::~FsNotifyBridge() { stop(); } + +bool FsNotifyBridge::supported() noexcept { +#ifdef __linux__ + return true; +#else + return false; +#endif +} + +std::string FsNotifyBridge::abs(std::string_view rel) const { + std::string out = root_; + if (!out.empty() && out.back() == '/') out.pop_back(); + out.push_back('/'); + out.append(rel); + return out; +} + +FsNotifyBridge::Stats FsNotifyBridge::stats() const { + std::lock_guard lock(stats_mu_); + return stats_; +} + +bool FsNotifyBridge::is_self(int caller) const noexcept { + const long c = static_cast(caller); + // FUSE reports the calling task's pid, which for a threaded caller is its + // tid. Accept the process id as well: some paths report the thread group, + // and either way it is this daemon and nothing else. + return c != 0 && (c == poke_tid_.load(std::memory_order_relaxed) || + c == self_pid_.load(std::memory_order_relaxed)); +} + +void FsNotifyBridge::set_active(Active a) { + std::lock_guard lock(active_mu_); + active_ = std::move(a); +} + +void FsNotifyBridge::clear_active() { + std::lock_guard lock(active_mu_); + active_ = Active{}; +} + +FsNotifyBridge::Pretend FsNotifyBridge::pretend(int caller, std::string_view rel, + NodeKind& kind) const noexcept { + if (!is_self(caller)) return Pretend::Nothing; + std::lock_guard lock(active_mu_); + if (active_.pretend_path == Pretend::Nothing || active_.path != rel) return Pretend::Nothing; + kind = active_.kind; + return active_.pretend_path; +} + +bool FsNotifyBridge::claim(int caller, Poke what, std::string_view rel, std::string_view rel2) { + if (!is_self(caller)) return false; + std::lock_guard lock(active_mu_); + if (active_.what != what || active_.path != rel || active_.path2 != rel2) return false; + // Retire the record. The syscall that raised this request is about to return, + // and the pretence must be gone before libfuse re-reads the path to build its + // reply — otherwise the reply would describe the fiction instead of the + // mirror's real state. + active_ = Active{}; + return true; +} + +void FsNotifyBridge::post(std::span changes) { + if (!running_.load(std::memory_order_relaxed)) return; + + // Pair the two halves of each move into one job before queueing, so the poke + // loop only ever sees whole operations. Both sides upstream try to keep pairs + // together, but a batch too large for one frame is broadcast in several and + // the halves can still arrive apart; a half with no partner here degrades to + // what it amounts to alone rather than being dropped. + std::unordered_map from_by_cookie; + std::vector jobs; + jobs.reserve(changes.size()); + for (const auto& c : changes) { + if (c.rescan) { + jobs.push_back(Job{ChangeKind::Unknown, {}, {}, NodeKind::Directory, true}); + continue; + } + if (c.change == ChangeKind::MovedFrom && c.cookie != 0) { + from_by_cookie.emplace(c.cookie, jobs.size()); + jobs.push_back(Job{ChangeKind::MovedFrom, c.path, {}, c.kind, false}); + continue; + } + if (c.change == ChangeKind::MovedTo && c.cookie != 0) { + if (const auto it = from_by_cookie.find(c.cookie); it != from_by_cookie.end()) { + jobs[it->second].path2 = c.path; // completes the pair in place, keeping its order + from_by_cookie.erase(it); + continue; + } + jobs.push_back(Job{ChangeKind::Created, c.path, {}, c.kind, false}); // no partner: it appeared + continue; + } + jobs.push_back(Job{c.change, c.path, {}, c.kind, false}); + } + // Sources still waiting for a destination lost it somewhere upstream; on + // their own they are removals. + for (const auto& [cookie, idx] : from_by_cookie) { + (void)cookie; + jobs[idx].change = ChangeKind::Removed; + } + + std::size_t dropped = 0; + { + std::lock_guard lock(mu_); + for (Job& j : jobs) { + if (queue_.size() >= kQueueCap) { + queue_.pop_front(); + ++dropped; + } + queue_.push_back(std::move(j)); + } + } + if (dropped != 0) { + std::lock_guard lock(stats_mu_); + stats_.dropped += dropped; + } + cv_.notify_one(); +} + +void FsNotifyBridge::run() { + poke_tid_.store(this_tid_or_zero(), std::memory_order_relaxed); + // Verified here rather than in start(): the check reads the mount, and the + // mount is not answering until the FUSE loop is running, which start()'s + // caller is in the middle of arranging. Blocking there would deadlock it. + if (!verify_root()) { + running_.store(false); + return; + } + for (;;) { + Job job; + { + std::unique_lock lock(mu_); + cv_.wait(lock, [this] { return stop_.load() || !queue_.empty(); }); + if (stop_.load()) return; + job = std::move(queue_.front()); + queue_.pop_front(); + } + deliver(job); + } +} + +void FsNotifyBridge::deliver(const Job& job) { + bool ok = false; + { + std::lock_guard lock(stats_mu_); + ++stats_.events; + if (job.rescan) ++stats_.rescans; + } + if (job.rescan) { + // An overflow on the far side means no path can be named. Touch the mount + // root so a watcher that re-walks on any event below its root gets the + // chance to; one that waits for a specific path cannot be told. + ok = poke_modify(Job{ChangeKind::Modified, {}, {}, NodeKind::Directory, false}); + } else if (!job.path2.empty()) { + ok = poke_move(job); + } else { + switch (job.change) { + case ChangeKind::Created: ok = poke_create(job); break; + case ChangeKind::Modified: ok = poke_modify(job); break; + case ChangeKind::Removed: + case ChangeKind::MovedFrom: ok = poke_remove(job); break; + case ChangeKind::MovedTo: ok = poke_create(job); break; + case ChangeKind::Unknown: return; // nothing to say about it + } + } + std::lock_guard lock(stats_mu_); + if (ok) ++stats_.delivered; + else ++stats_.failed; +} + +#ifdef __linux__ + +long FsNotifyBridge::this_tid_or_zero() noexcept { return this_tid(); } + +bool FsNotifyBridge::inside_mount(std::string_view rel) const { + // Check the parent, not the path itself: for a creation the path does not + // exist yet, and for a removal it no longer does. + const std::size_t slash = rel.rfind('/'); + const std::string parent = slash == std::string_view::npos ? root_ : abs(rel.substr(0, slash)); + struct ::stat st {}; + if (::lstat(parent.c_str(), &st) != 0) return false; + return static_cast(st.st_dev) == root_dev_; +} + +bool FsNotifyBridge::verify_root() { + // The pokes are ordinary filesystem calls, so pointing them anywhere but the + // intended FUSE mount would create and delete real files in the wrong tree. + // Two things are established here and relied on for the rest of the run: the + // root is a FUSE mount, and which device that mount is — every poke re-checks + // its target against the latter, so a path that resolves out of the mount is + // refused rather than applied to whatever it landed on. + struct ::statfs fs {}; + struct ::stat st {}; + if (::statfs(root_.c_str(), &fs) != 0 || ::lstat(root_.c_str(), &st) != 0) { + std::fprintf(stderr, "wsldrive: inotify bridge disabled (cannot stat %s)\n", root_.c_str()); + return false; + } + if (static_cast(fs.f_type) != FUSE_SUPER_MAGIC) { + std::fprintf(stderr, "wsldrive: inotify bridge disabled (%s is not a FUSE mount)\n", root_.c_str()); + return false; + } + root_dev_ = static_cast(st.st_dev); + return true; +} + +Result FsNotifyBridge::start(const std::string& mount_root) { + if (running_.load()) return {}; + root_ = mount_root; + self_pid_.store(static_cast(::getpid()), std::memory_order_relaxed); + stop_.store(false); + running_.store(true); + thread_ = std::thread([this] { run(); }); + return {}; +} + +void FsNotifyBridge::stop() { + if (!running_.exchange(false)) return; + { + std::lock_guard lock(mu_); + stop_.store(true); + queue_.clear(); + } + cv_.notify_all(); + if (thread_.joinable()) thread_.join(); + clear_active(); +} + +bool FsNotifyBridge::poke_create(const Job& job) { + if (job.path.empty() || !inside_mount(job.path)) return false; + const bool dir = job.kind == NodeKind::Directory; + // The invalidation has already been applied, so the mirror knows this path + // and the kernel's lookup would find it and never reach ->mknod/->mkdir. + // Hide it from this thread alone for the length of the call. + set_active(Active{dir ? Poke::Mkdir : Poke::Mknod, job.path, {}, Pretend::Absent, job.kind}); + const std::string p = abs(job.path); + const int rc = dir ? ::mkdir(p.c_str(), 0755) : ::mknod(p.c_str(), S_IFREG | 0644, 0); + clear_active(); + return rc == 0; +} + +bool FsNotifyBridge::poke_modify(const Job& job) { + if (!job.path.empty() && !inside_mount(job.path)) return false; + // Only mtime. fsnotify_change() maps a change to both timestamps to + // FS_ATTRIB (IN_ATTRIB) and one to mtime alone to FS_MODIFY (IN_MODIFY), and + // IN_MODIFY is what a watch-mode tool is actually waiting for. No + // interception is needed: the mount already accepts utimens without + // forwarding it, and the mirror keeps reporting the agent's timestamps. + const struct ::timespec times[2] = {{0, UTIME_OMIT}, {0, UTIME_NOW}}; + const std::string p = job.path.empty() ? root_ : abs(job.path); + return ::utimensat(AT_FDCWD, p.c_str(), times, AT_SYMLINK_NOFOLLOW) == 0; +} + +bool FsNotifyBridge::poke_remove(const Job& job) { + if (job.path.empty() || !inside_mount(job.path)) return false; + const bool dir = job.kind == NodeKind::Directory; + // The mirror has already dropped the path, so the kernel would resolve it to + // nothing and ->unlink would never run. Answer this thread's lookup with the + // entry as it was. The ghost dentry does not outlive the call: the unlink + // that follows drops it, and nothing else can see it in between, because + // every other caller gets the mirror's real answer. + set_active(Active{dir ? Poke::Rmdir : Poke::Unlink, job.path, {}, Pretend::Present, job.kind}); + const std::string p = abs(job.path); + const int rc = dir ? ::rmdir(p.c_str()) : ::unlink(p.c_str()); + clear_active(); + return rc == 0; +} + +bool FsNotifyBridge::poke_move(const Job& job) { + if (job.path.empty() || job.path2.empty()) return false; + if (!inside_mount(job.path) || !inside_mount(job.path2)) return false; + // Letting the kernel do the rename is what makes this worth pairing the two + // halves for: vfs_rename raises IN_MOVED_FROM and IN_MOVED_TO together, with + // one cookie, which is exactly what a watcher needs to recognise a move + // rather than an unrelated delete and create. + // + // The source is ghosted with the *destination's* type, because after the + // rename that is the entry the kernel keeps. + set_active(Active{Poke::Rename, job.path, job.path2, Pretend::Present, job.kind}); + const int rc = ::rename(abs(job.path).c_str(), abs(job.path2).c_str()); + clear_active(); + + // The kernel does not build a fresh inode for the destination; `d_move` walks + // the source's dentry over to the new name, and that dentry is the one holding + // the placeholder attributes the ghost handed out. Left alone, the destination + // would report a zero size for as long as the attribute cache holds it. Drop + // the entry so the next lookup goes back to the mirror. + // + // The invalidation hook queues a punch for both paths anyway, but on another + // thread and with no ordering against this one, so it cannot be relied on to + // land after the rename. + if (rc == 0 && invalidate_) invalidate_(job.path2); + return rc == 0; +} + +#else // not Linux + +long FsNotifyBridge::this_tid_or_zero() noexcept { return 0; } +bool FsNotifyBridge::verify_root() { return false; } +bool FsNotifyBridge::inside_mount(std::string_view) const { return false; } +Result FsNotifyBridge::start(const std::string&) { return fail(Errc::Unsupported); } +void FsNotifyBridge::stop() {} +bool FsNotifyBridge::poke_create(const Job&) { return false; } +bool FsNotifyBridge::poke_modify(const Job&) { return false; } +bool FsNotifyBridge::poke_remove(const Job&) { return false; } +bool FsNotifyBridge::poke_move(const Job&) { return false; } + +#endif + +} // namespace wsld::mount diff --git a/src/mount/fsnotify_bridge.hpp b/src/mount/fsnotify_bridge.hpp new file mode 100644 index 0000000..5507304 --- /dev/null +++ b/src/mount/fsnotify_bridge.hpp @@ -0,0 +1,213 @@ +#pragma once + +#include "agent/client.hpp" +#include "core/error.hpp" +#include "core/types.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace wsld::mount { + +/// Makes far-side changes visible to ordinary Linux file watchers. +/// +/// # The problem +/// +/// A change made on the Windows side of the boundary reaches this mount as an +/// invalidation, so the metadata mirror and the page cache are correct within +/// milliseconds. What does *not* happen is an `inotify` event, and that is what +/// every watch-mode tool in the Linux ecosystem is waiting for. The tool does +/// not error; it simply never fires. That is the whole of microsoft/WSL#4739. +/// +/// # Why it cannot be delivered directly +/// +/// The kernel raises fsnotify events from the VFS, at the point an operation is +/// performed — `vfs_create` calls `fsnotify_create`, `vfs_unlink` calls +/// `fsnotify_unlink`, and so on. A filesystem cannot raise one on its own, and +/// FUSE offers no notification that does. The three `fuse_lowlevel_notify_*` +/// calls invalidate dentries and pages, which is a cache-coherence mechanism, +/// not a notification one: `fuse_reverse_inval_entry` never touches fsnotify. +/// A userspace daemon has no way to hand an inotify watcher an event. +/// +/// # What this does instead +/// +/// It asks the kernel to raise the event, by performing the operation the far +/// side already performed — on this mount, from a thread of this process. A +/// file created over there becomes a `mknodat` here, a deletion an `unlinkat`, +/// a rename a `renameat`, a write a `utimensat` that touches only mtime (which +/// `fsnotify_change` reports as FS_MODIFY, not FS_ATTRIB). The kernel runs its +/// own hooks and every watcher gets a genuine event of the right type, with a +/// genuine rename cookie pairing the two halves of a move. Consumers need no +/// cooperation, no preload, and no knowledge that wsldrive exists. +/// +/// The operations must not cross the boundary a second time — the far side +/// already has this state, and re-applying it would at best be wasted work and +/// at worst destroy the file that prompted the event. So the FUSE handlers +/// recognise these requests and answer them locally without forwarding. That +/// recognition is the delicate part, and it is deliberately over-determined: +/// see `claim()`. +/// +/// Only the mount's own view is touched. Nothing is written to the served tree. +/// +/// # Cost +/// +/// One syscall per changed path, served by the FUSE loop from the in-RAM +/// mirror. No boundary crossing, no I/O. +/// +/// Linux only. `supported()` is false elsewhere and `start()` fails with +/// Unsupported; Direction A has no equivalent problem, because a WinFsp volume +/// raises Windows change notifications through its own mechanism. +class FsNotifyBridge { + public: + /// The operations the bridge performs on itself. A FUSE handler names the one + /// it implements when asking whether the request in front of it is a poke. + enum class Poke : std::uint8_t { + None, + Mknod, // a file appeared + Mkdir, // a directory appeared + Unlink, // a file went away + Rmdir, // a directory went away + Rename, // both halves of a move + }; + + /// What a handler should pretend about a path, so that the kernel will run + /// the operation that raises the event. Both are visible only to the bridge's + /// own thread: any other caller gets the mirror's real answer. + enum class Pretend : std::uint8_t { + Nothing, + Absent, // the mirror already has the new path; hide it so ->create runs + Present, // the mirror already dropped the old path; ghost it so ->unlink runs + }; + + struct Stats { + std::uint64_t events = 0; // changes taken off the queue + std::uint64_t delivered = 0; // pokes the kernel accepted (an event went out) + std::uint64_t failed = 0; // pokes that returned an error + std::uint64_t dropped = 0; // changes discarded because the queue was full + std::uint64_t rescans = 0; // mirror replacements, which name no path + }; + + FsNotifyBridge() = default; + ~FsNotifyBridge(); + FsNotifyBridge(const FsNotifyBridge&) = delete; + FsNotifyBridge& operator=(const FsNotifyBridge&) = delete; + + /// Drops the kernel's cached dentry and pages for one mount-relative path. + /// The rename poke needs it; see the comment in poke_move(). + using InvalidatePath = std::function; + void set_invalidate(InvalidatePath fn) { invalidate_ = std::move(fn); } + + [[nodiscard]] static bool supported() noexcept; + + /// Starts the poke thread against an already-mounted `mount_root`. Fails with + /// Unsupported off Linux, and with InvalidArgument if `mount_root` is not a + /// FUSE mount — the pokes are real filesystem operations, so pointing them at + /// anything else would modify the wrong tree. + [[nodiscard]] Result start(const std::string& mount_root); + + /// Stops delivery and joins. Must run before the mount goes away. Idempotent. + void stop(); + + [[nodiscard]] bool running() const noexcept { return running_.load(std::memory_order_relaxed); } + + /// Queues changes for delivery. Called from the client's reader thread; never + /// blocks, and drops rather than growing without bound. + void post(std::span changes); + + [[nodiscard]] Stats stats() const; + + // --- consulted by the FUSE handlers --------------------------------------- + + /// Whether `caller` should be told something other than the truth about + /// `rel`, so the kernel proceeds into the operation that raises the event. + /// `kind` receives the type to report for Pretend::Present. + [[nodiscard]] Pretend pretend(int caller, std::string_view rel, NodeKind& kind) const noexcept; + + /// Whether the request in front of a handler is this bridge's own poke, and + /// so must be answered locally instead of forwarded across the boundary. + /// + /// Three independent things must hold, because the cost of a false positive + /// is a silently swallowed user operation — a delete that does not delete: + /// + /// 1. the caller is the bridge's own poke thread (its tid, or this + /// process's pid; no other thread here ever touches the mount), + /// 2. a poke of exactly this kind is in flight, and + /// 3. it names exactly this path (both paths, for a rename). + /// + /// A poke is claimed at most once: the record is retired by this call, so a + /// retry or a duplicate request is forwarded like any other. + [[nodiscard]] bool claim(int caller, Poke what, std::string_view rel, std::string_view rel2 = {}); + + private: + struct Job { + ChangeKind change = ChangeKind::Unknown; + std::string path; + std::string path2; // move destination + NodeKind kind = NodeKind::File; + bool rescan = false; + }; + + // The single poke in flight. One at a time, so the handlers' test is an exact + // match rather than a search, and a stale record cannot outlive its syscall. + struct Active { + Poke what = Poke::None; + std::string path; + std::string path2; + Pretend pretend_path = Pretend::Nothing; + NodeKind kind = NodeKind::File; + }; + + void run(); + void deliver(const Job& job); + bool poke_create(const Job& job); + bool poke_modify(const Job& job); + bool poke_remove(const Job& job); + bool poke_move(const Job& job); + // True if `rel`'s parent directory is inside the mount. The pokes are real + // filesystem calls; this is what keeps one that resolves outside the mount + // (a path that escapes it, a mount that went away under us) from being + // applied to whatever is there instead. + [[nodiscard]] bool inside_mount(std::string_view rel) const; + [[nodiscard]] std::string abs(std::string_view rel) const; + [[nodiscard]] static long this_tid_or_zero() noexcept; + // Confirms `root_` really is the FUSE mount the pokes are meant for, and + // records its device. Runs on the poke thread; see the comment at its call. + [[nodiscard]] bool verify_root(); + void set_active(Active a); + void clear_active(); + [[nodiscard]] bool is_self(int caller) const noexcept; + + std::string root_; + InvalidatePath invalidate_; // set once, before start() + std::uint64_t root_dev_ = 0; // st_dev of the mount root; pokes never leave it + std::thread thread_; + std::atomic running_{false}; + std::atomic stop_{false}; + std::atomic poke_tid_{0}; + std::atomic self_pid_{0}; + + mutable std::mutex mu_; + std::condition_variable cv_; + std::deque queue_; + + mutable std::mutex active_mu_; + Active active_; + + mutable std::mutex stats_mu_; + Stats stats_; + + // Past this many queued changes the oldest are dropped. A watcher that misses + // one gets a late notification, not a wrong one: the mirror and the page + // cache are already correct, so the file reads right the moment anything + // looks at it. Sized for a burst the size of an `npm install`. + static constexpr std::size_t kQueueCap = 65536; +}; + +} // namespace wsld::mount diff --git a/src/mount/fuse_mount.cpp b/src/mount/fuse_mount.cpp index 2a97d48..d3890b3 100644 --- a/src/mount/fuse_mount.cpp +++ b/src/mount/fuse_mount.cpp @@ -16,9 +16,13 @@ using StatvfsT = struct fuse_statvfs; using OffT = fuse_off_t; using ModeT = fuse_mode_t; using TimespecT = struct fuse_timespec; +using DevT = fuse_dev_t; #ifndef S_IFLNK #define S_IFLNK 0120000 #endif +#ifndef S_IFMT +#define S_IFMT 0170000 +#endif #else #define FUSE_USE_VERSION 31 #include @@ -30,6 +34,7 @@ using StatvfsT = struct statvfs; using OffT = off_t; using ModeT = mode_t; using TimespecT = struct timespec; +using DevT = dev_t; #endif #include @@ -52,6 +57,10 @@ namespace { struct Context { agent::RemoteRoot* root; bool writeback = false; + // Non-null only where change notification is on (Linux). The handlers below + // consult it before acting, because a handful of the requests they see are + // the bridge's own and must be answered here rather than forwarded. + FsNotifyBridge* notify = nullptr; }; Context* ctx() { return static_cast(fuse_get_context()->private_data); } @@ -159,8 +168,43 @@ void fill_stat(const MetadataTree::Node& n, StatT* st) { st->st_ctim = st->st_mtim; } +// The change-notification bridge's own requests, if there is one. Each returns +// false the moment notification is off, so the ordinary path is one null check. +int caller_pid() { + const struct fuse_context* c = fuse_get_context(); + return c != nullptr ? static_cast(c->pid) : 0; +} + +bool claim_poke(FsNotifyBridge::Poke what, std::string_view rel, std::string_view rel2 = {}) { + FsNotifyBridge* b = ctx()->notify; + return b != nullptr && b->claim(caller_pid(), what, rel, rel2); +} + +// Fills `st` with a minimal stat for a path the mirror no longer has, so the +// bridge's own removal or rename can proceed far enough for the kernel to raise +// the event. Only ever returned to the bridge's thread. +void fill_ghost_stat(NodeKind kind, StatT* st) { + std::memset(st, 0, sizeof(*st)); +#ifndef _WIN32 + st->st_uid = ::getuid(); + st->st_gid = ::getgid(); +#endif + st->st_mode = static_cast(kind == NodeKind::Directory ? (S_IFDIR | 0755) : (S_IFREG | 0644)); + st->st_nlink = kind == NodeKind::Directory ? 2 : 1; +} + int op_getattr(const char* path, StatT* st, struct fuse_file_info*) { const std::string rel = to_rel(path); + if (FsNotifyBridge* b = ctx()->notify; b != nullptr) { + NodeKind kind = NodeKind::File; + switch (b->pretend(caller_pid(), rel, kind)) { + case FsNotifyBridge::Pretend::Absent: return -ENOENT; // so ->mknod / ->mkdir runs + case FsNotifyBridge::Pretend::Present: // so ->unlink / ->rename runs + fill_ghost_stat(kind, st); + return 0; + case FsNotifyBridge::Pretend::Nothing: break; + } + } return ctx()->root->with_tree([&](const MetadataTree& t) -> int { const auto id = t.lookup(rel, LookupMode::CaseInsensitive); if (!id) return -ENOENT; @@ -272,23 +316,45 @@ int op_truncate(const char* path, OffT size, struct fuse_file_info*) { return r ? 0 : err_to_errno(r.error()); } +// Not previously implemented, which made `mknod` fail with ENOSYS. It is here +// because the change-notification bridge creates a file through it — the kernel +// routes mknod of a regular file to ->create, which is what raises IN_CREATE — +// but the forwarding path below is a real gain of its own: tools that reach for +// mknod instead of open(O_CREAT) now work on the mount. +int op_mknod(const char* path, ModeT mode, DevT) { + const std::string rel = to_rel(path); + if (claim_poke(FsNotifyBridge::Poke::Mknod, rel)) return 0; + if ((mode & S_IFMT) != 0 && (mode & S_IFMT) != S_IFREG) return -EPERM; // no devices across the boundary + auto r = ctx()->root->create_file(rel, static_cast(mode) & 0777u); + return r ? 0 : err_to_errno(r.error()); +} + int op_mkdir(const char* path, ModeT mode) { - auto r = ctx()->root->mkdir(to_rel(path), static_cast(mode) & 0777u); + const std::string rel = to_rel(path); + if (claim_poke(FsNotifyBridge::Poke::Mkdir, rel)) return 0; + auto r = ctx()->root->mkdir(rel, static_cast(mode) & 0777u); return r ? 0 : err_to_errno(r.error()); } int op_unlink(const char* path) { - auto r = ctx()->root->unlink(to_rel(path)); + const std::string rel = to_rel(path); + if (claim_poke(FsNotifyBridge::Poke::Unlink, rel)) return 0; + auto r = ctx()->root->unlink(rel); return r ? 0 : err_to_errno(r.error()); } int op_rmdir(const char* path) { - auto r = ctx()->root->rmdir(to_rel(path)); + const std::string rel = to_rel(path); + if (claim_poke(FsNotifyBridge::Poke::Rmdir, rel)) return 0; + auto r = ctx()->root->rmdir(rel); return r ? 0 : err_to_errno(r.error()); } int op_rename(const char* from, const char* to, unsigned int) { - auto r = ctx()->root->rename(to_rel(from), to_rel(to)); + const std::string rfrom = to_rel(from); + const std::string rto = to_rel(to); + if (claim_poke(FsNotifyBridge::Poke::Rename, rfrom, rto)) return 0; + auto r = ctx()->root->rename(rfrom, rto); return r ? 0 : err_to_errno(r.error()); } @@ -436,6 +502,7 @@ fuse_operations make_ops() { ops.create = op_create; ops.write = op_write; ops.truncate = op_truncate; + ops.mknod = op_mknod; ops.mkdir = op_mkdir; ops.unlink = op_unlink; ops.rmdir = op_rmdir; @@ -454,7 +521,7 @@ fuse_operations make_ops() { FuseMount::~FuseMount() { unmount(); } -Result FuseMount::mount(const std::string& mountpoint, bool writeback) { +Result FuseMount::mount(const std::string& mountpoint, bool writeback, bool notify_changes) { #ifdef _WIN32 if (!load_winfsp_dll()) return fail(Errc::Unsupported); #endif @@ -468,6 +535,10 @@ Result FuseMount::mount(const std::string& mountpoint, bool writeback) { // read-ahead, and - against a case-sensitive agent - the file itself. root_.set_lookup_mode(LookupMode::CaseInsensitive); context.writeback = writeback; + // Wired in before the loop starts so no request can reach a handler while + // this is half-set; the bridge itself is started further down, once there is + // a live mount for it to poke. + context.notify = notify_changes && FsNotifyBridge::supported() ? ¬ify_ : nullptr; static fuse_operations ops = make_ops(); struct fuse_args args = FUSE_ARGS_INIT(0, nullptr); @@ -513,14 +584,21 @@ Result FuseMount::mount(const std::string& mountpoint, bool writeback) { // relative to its own mutation changed nothing, and punching for it would act // on an event already judged obsolete. This also leaves the raw-batch // InvalidationHook free for `wsldrive watch`. - root_.set_invalidated_paths_hook([this](std::span paths) { - std::lock_guard lock(inval_mu_); - if (inval_stop_) return; - for (const std::string& p : paths) { - if (inval_queue_.size() >= kInvalQueueCap) break; - inval_queue_.push_back(to_fuse_path(p)); + root_.set_invalidated_paths_hook([this](std::span changes) { + // Two consumers of the same signal, and they want different things from it. + // The page-cache punch only needs to know which paths moved on; the + // notification bridge needs to know what happened to each of them. + { + std::lock_guard lock(inval_mu_); + if (inval_stop_) return; + for (const auto& c : changes) { + if (c.rescan) continue; // no path to punch; the whole mirror was replaced + if (inval_queue_.size() >= kInvalQueueCap) break; + inval_queue_.push_back(to_fuse_path(c.path)); + } + inval_cv_.notify_one(); } - inval_cv_.notify_one(); + notify_.post(changes); }); loop_ = std::thread([this] { @@ -551,6 +629,20 @@ Result FuseMount::mount(const std::string& mountpoint, bool writeback) { fuse_loop(static_cast(fuse_)); mounted_.store(false); }); + + // Last, because the bridge's first act is to confirm it is pointed at a live + // FUSE mount, and nothing answers a request on this mount until the loop + // above is running. It does that on its own thread, so this does not wait. + if (context.notify != nullptr) { + notify_.set_invalidate([this](const std::string& rel) { + std::lock_guard lock(inval_mu_); + if (inval_stop_ || inval_queue_.size() >= kInvalQueueCap) return; + inval_queue_.push_back(to_fuse_path(rel)); + inval_cv_.notify_one(); + }); + if (auto r = notify_.start(mountpoint); !r) + std::fprintf(stderr, "wsldrive: change notification unavailable on this platform\n"); + } return {}; } @@ -576,6 +668,9 @@ void FuseMount::unmount() { // Stop feeding the invalidation thread, and join it, before the fuse handle // it dereferences goes away. Harmless where the thread was never started. root_.set_invalidated_paths_hook({}); + // The bridge issues filesystem calls against this mount, so it has to be + // stopped and joined before the mount is torn out from under it. + notify_.stop(); { std::lock_guard lock(inval_mu_); inval_stop_ = true; diff --git a/src/mount/fuse_mount.hpp b/src/mount/fuse_mount.hpp index a3c278f..efb4ab4 100644 --- a/src/mount/fuse_mount.hpp +++ b/src/mount/fuse_mount.hpp @@ -2,6 +2,7 @@ #include "agent/client.hpp" #include "core/error.hpp" +#include "mount/fsnotify_bridge.hpp" #include #include @@ -31,7 +32,16 @@ class FuseMount { /// starts serving. Returns once the volume is up. With `writeback`, writes to /// a file are buffered and coalesced, flushed on fsync/flush/release — fewer /// round-trips, at the cost of durability only at flush/close (opt-in). - [[nodiscard]] Result mount(const std::string& mountpoint, bool writeback = false); + /// + /// With `notify_changes`, far-side changes are also delivered to local + /// `inotify` watchers, so watch-mode tools react to edits made on the other + /// side of the boundary (see FsNotifyBridge). Linux only; ignored elsewhere. + [[nodiscard]] Result mount(const std::string& mountpoint, bool writeback = false, + bool notify_changes = true); + + /// Counters for the change-notification bridge; all zero when it is off. + [[nodiscard]] FsNotifyBridge::Stats notify_stats() const { return notify_.stats(); } + [[nodiscard]] bool notifying() const noexcept { return notify_.running(); } /// Signals the FUSE loop to exit, unmounts, and joins the loop thread. void unmount(); @@ -43,6 +53,7 @@ class FuseMount { void inval_loop(); agent::RemoteRoot& root_; + FsNotifyBridge notify_; void* fuse_ = nullptr; // struct fuse* std::string mountpoint_; std::thread loop_; diff --git a/src/platform/linux/dir_watcher.cpp b/src/platform/linux/dir_watcher.cpp index db2d2cd..13c25f1 100644 --- a/src/platform/linux/dir_watcher.cpp +++ b/src/platform/linux/dir_watcher.cpp @@ -156,14 +156,18 @@ class InotifyWatcher final : public Watcher { const std::string rel = join_rel(dir, ev.name); const bool is_dir = (ev.mask & IN_ISDIR) != 0; + // inotify already pairs the two halves of a move with a cookie; pass it + // straight through. It is only set on IN_MOVED_FROM/IN_MOVED_TO. if (ev.mask & (IN_CREATE | IN_MOVED_TO)) { if (is_dir) add_watch_recursive(root_ / std::filesystem::path(rel), rel); - cb_(FsEvent{ev.mask & IN_MOVED_TO ? FsEventKind::RenamedTo : FsEventKind::Created, rel}); + const bool moved = (ev.mask & IN_MOVED_TO) != 0; + cb_(FsEvent{moved ? FsEventKind::RenamedTo : FsEventKind::Created, rel, moved ? ev.cookie : 0}); // A directory may have been populated before we armed the watch; a scan/rescan // on the consumer side covers that. For files, CLOSE_WRITE will follow. } else if (ev.mask & (IN_DELETE | IN_MOVED_FROM)) { if (is_dir) drop_watch_subtree(rel); - cb_(FsEvent{ev.mask & IN_MOVED_FROM ? FsEventKind::RenamedFrom : FsEventKind::Removed, rel}); + const bool moved = (ev.mask & IN_MOVED_FROM) != 0; + cb_(FsEvent{moved ? FsEventKind::RenamedFrom : FsEventKind::Removed, rel, moved ? ev.cookie : 0}); } else if (ev.mask & (IN_MODIFY | IN_CLOSE_WRITE | IN_ATTRIB)) { cb_(FsEvent{FsEventKind::Modified, rel}); } diff --git a/src/platform/win/dir_watcher.cpp b/src/platform/win/dir_watcher.cpp index 135036d..7db315e 100644 --- a/src/platform/win/dir_watcher.cpp +++ b/src/platform/win/dir_watcher.cpp @@ -105,23 +105,47 @@ class Win32Watcher final : public Watcher { for (char& c : path) if (c == '\\') c = '/'; FsEventKind kind; + // ReadDirectoryChangesW has no equivalent of inotify's rename cookie: it + // reports the two halves as OLD_NAME immediately followed by NEW_NAME in + // the same buffer. Mint a cookie on the OLD_NAME and hand the same one to + // the NEW_NAME that follows, so a consumer can pair them the way it pairs + // an inotify move. Anything other than a NEW_NAME clears the pending + // cookie, which leaves a truncated pair unpaired rather than joined to + // the wrong partner. + std::uint32_t cookie = 0; switch (info->Action) { case FILE_ACTION_ADDED: kind = FsEventKind::Created; break; case FILE_ACTION_REMOVED: kind = FsEventKind::Removed; break; case FILE_ACTION_MODIFIED: kind = FsEventKind::Modified; break; - case FILE_ACTION_RENAMED_OLD_NAME: kind = FsEventKind::RenamedFrom; break; - case FILE_ACTION_RENAMED_NEW_NAME: kind = FsEventKind::RenamedTo; break; + case FILE_ACTION_RENAMED_OLD_NAME: + kind = FsEventKind::RenamedFrom; + cookie = next_cookie(); + pending_cookie_ = cookie; + break; + case FILE_ACTION_RENAMED_NEW_NAME: + kind = FsEventKind::RenamedTo; + cookie = pending_cookie_ != 0 ? pending_cookie_ : next_cookie(); + break; default: kind = FsEventKind::Modified; break; } - cb_(FsEvent{kind, path}); + if (info->Action != FILE_ACTION_RENAMED_OLD_NAME) pending_cookie_ = 0; + cb_(FsEvent{kind, path, cookie}); if (info->NextEntryOffset == 0) break; off += info->NextEntryOffset; } } + // Never returns 0: zero is the "no pair" marker downstream. + std::uint32_t next_cookie() noexcept { + if (++cookie_seq_ == 0) ++cookie_seq_; + return cookie_seq_; + } + HANDLE dir_; HANDLE iocp_; WatchCallback cb_; + std::uint32_t cookie_seq_ = 0; + std::uint32_t pending_cookie_ = 0; // cookie of an OLD_NAME awaiting its NEW_NAME OVERLAPPED ov_{}; // Heap-allocated, so `new` gives it stricter alignment than // FILE_NOTIFY_INFORMATION needs. diff --git a/src/tools/wsldrive_main.cpp b/src/tools/wsldrive_main.cpp index 05b2eea..c3532a8 100644 --- a/src/tools/wsldrive_main.cpp +++ b/src/tools/wsldrive_main.cpp @@ -141,7 +141,8 @@ void usage() { " (check the WinFsp + WSL environment)\n" #endif #ifdef WSLDRIVE_HAVE_MOUNT - " wsldrive mount --connect [--writeback] [--no-prefetch] (attach to an agent)\n" + " wsldrive mount --connect [--writeback] [--no-prefetch] [--no-inotify]\n" + " (attach to an agent)\n" #endif #if defined(WSLDRIVE_HAVE_MOUNT) && !defined(_WIN32) " wsldrive mount --win-root --win-agent [--hvsocket [--vm-guid G]]\n" @@ -420,6 +421,7 @@ int main(int argc, char** argv) { bool writeback = false; bool hvsocket = false; bool prefetch = true; + bool notify_changes = true; std::string vm_guid; for (int i = 3; i < argc; ++i) { const std::string_view a = argv[i]; @@ -440,6 +442,8 @@ int main(int argc, char** argv) { writeback = true; else if (a == "--no-prefetch") prefetch = false; + else if (a == "--no-inotify") + notify_changes = false; else if (a == "--distro") { distro = val(); have_distro = true; @@ -649,12 +653,14 @@ int main(int argc, char** argv) { if (dirs > 0) std::printf("prefetching %zu directories in the background...\n", dirs); } wsld::mount::FuseMount fm(root); - if (auto r = fm.mount(mountpoint, writeback); !r) { + if (auto r = fm.mount(mountpoint, writeback, notify_changes); !r) { std::fprintf(stderr, "wsldrive: mount failed: %s\n", wsld::to_string(r.error())); return 1; } install_mount_signal_handler(); std::printf("mounted. Ctrl+C to unmount.\n"); + if (notify_changes && wsld::mount::FsNotifyBridge::supported()) + std::printf("far-side changes are delivered to local inotify watchers (--no-inotify turns this off)\n"); std::fflush(stdout); std::uint64_t rescans_seen = 0, rescan_failures_seen = 0; while (fm.mounted() && root.connected() && !g_mount_stop.load()) { @@ -670,6 +676,12 @@ int main(int argc, char** argv) { static_cast(st.rescans), static_cast(st.rescan_failures)); } } + // Worth a line when notifications were lost: a watcher that missed one sees + // a change late, and this is the only place that would say so. + if (const auto ns = fm.notify_stats(); ns.dropped != 0 || ns.failed != 0) + std::fprintf(stderr, "wsldrive: %llu change notifications dropped, %llu could not be delivered (of %llu)\n", + static_cast(ns.dropped), static_cast(ns.failed), + static_cast(ns.events)); std::printf("unmounting...\n"); std::fflush(stdout); fm.unmount(); // agent (if auto-launched) is stopped by its destructor on return @@ -821,9 +833,24 @@ int main(int argc, char** argv) { std::printf("[gen %llu +%lld ms] %zu ops\n", static_cast(b.generation), static_cast(std::chrono::duration_cast(now).count() % 100000), b.ops.size()); - for (const auto& op : b.ops) - std::printf(" %s %s\n", op.kind == wsld::InvalidationKind::Upsert ? "upsert" : op.kind == wsld::InvalidationKind::Remove ? "remove" : "rescan", - op.path.c_str()); + for (const auto& op : b.ops) { + const char* change = ""; + switch (op.change) { + case wsld::ChangeKind::Created: change = " (created)"; break; + case wsld::ChangeKind::Modified: change = " (modified)"; break; + case wsld::ChangeKind::Removed: change = " (removed)"; break; + case wsld::ChangeKind::MovedFrom: change = " (moved from)"; break; + case wsld::ChangeKind::MovedTo: change = " (moved to)"; break; + case wsld::ChangeKind::Unknown: break; + } + char cookie[32] = ""; + if (op.cookie != 0) std::snprintf(cookie, sizeof(cookie), " #%u", op.cookie); + std::printf(" %s %s%s%s\n", + op.kind == wsld::InvalidationKind::Upsert ? "upsert" + : op.kind == wsld::InvalidationKind::Remove ? "remove" + : "rescan", + op.path.c_str(), change, cookie); + } std::fflush(stdout); }); while (root.connected()) std::this_thread::sleep_for(std::chrono::milliseconds(200)); diff --git a/tests/agent_test.cpp b/tests/agent_test.cpp index 77cfda9..66053bb 100644 --- a/tests/agent_test.cpp +++ b/tests/agent_test.cpp @@ -990,6 +990,142 @@ TEST_F(AgentTest, LiveInvalidationsReachTheClient) { EXPECT_GT(client->stats().generation, 1u); } +// The change kind is what a notification consumer keys off, and it is decided +// in three places: the watcher names the event, the agent re-stats the path and +// may overrule it, and the client compares against its own mirror to settle +// created-versus-modified. This drives events in directly (RootServer::notify) +// rather than through a platform watcher, so it exercises the whole chain +// wherever the suite runs. +TEST_F(AgentTest, ChangeKindsReachTheClient) { + LoopbackServer srv(root_, /*watch=*/false); + auto client = connect_client(srv.endpoint()); + ASSERT_NE(client, nullptr); + ASSERT_TRUE(client->connect().has_value()); + ASSERT_TRUE(client->fetch_snapshot().has_value()); + + std::mutex mu; + std::condition_variable cv; + std::vector seen; + client->set_invalidated_paths_hook([&](std::span changes) { + std::lock_guard lock(mu); + seen.insert(seen.end(), changes.begin(), changes.end()); + cv.notify_all(); + }); + auto wait_for = [&](auto pred) { + std::unique_lock lock(mu); + return cv.wait_for(lock, 10s, [&] { return pred(seen); }); + }; + auto change_of = [&](std::string_view path) { + std::lock_guard lock(mu); + for (const auto& c : seen) + if (c.path == path) return c.change; + return ChangeKind::Unknown; + }; + auto forget = [&] { + std::lock_guard lock(mu); + seen.clear(); + }; + auto saw = [&](std::string_view path) { + return [path](const auto& v) { + for (const auto& c : v) + if (c.path == path) return true; + return false; + }; + }; + + // A path the mirror does not have yet is a creation. + write_file(root_ / "src" / "fresh.cpp", "// fresh\n"); + srv.server().notify(FsEvent{FsEventKind::Created, "src/fresh.cpp"}); + ASSERT_TRUE(wait_for(saw("src/fresh.cpp"))); + EXPECT_EQ(change_of("src/fresh.cpp"), ChangeKind::Created); + + // The same path again, now that the mirror has it, is a modification — even + // though the agent reports exactly the same thing about it either time. + forget(); + write_file(root_ / "src" / "fresh.cpp", "// fresher\n"); + srv.server().notify(FsEvent{FsEventKind::Modified, "src/fresh.cpp"}); + ASSERT_TRUE(wait_for(saw("src/fresh.cpp"))); + EXPECT_EQ(change_of("src/fresh.cpp"), ChangeKind::Modified); + + // A rename keeps both halves and the cookie that ties them together. + forget(); + fs::rename(root_ / "src" / "fresh.cpp", root_ / "src" / "renamed.cpp"); + srv.server().notify(FsEvent{FsEventKind::RenamedFrom, "src/fresh.cpp", 1234}); + srv.server().notify(FsEvent{FsEventKind::RenamedTo, "src/renamed.cpp", 1234}); + ASSERT_TRUE(wait_for([](const auto& v) { + bool from = false, to = false; + for (const auto& c : v) { + from |= c.change == ChangeKind::MovedFrom; + to |= c.change == ChangeKind::MovedTo; + } + return from && to; + })); + { + std::lock_guard lock(mu); + std::uint32_t from_cookie = 0, to_cookie = 0; + for (const auto& c : seen) { + if (c.change == ChangeKind::MovedFrom) { + EXPECT_EQ(c.path, "src/fresh.cpp"); + from_cookie = c.cookie; + } + if (c.change == ChangeKind::MovedTo) { + EXPECT_EQ(c.path, "src/renamed.cpp"); + to_cookie = c.cookie; + } + } + EXPECT_NE(from_cookie, 0u); + EXPECT_EQ(from_cookie, to_cookie); + } + + // A deletion names what was removed, and says whether it was a directory — + // the mirror is the only side that still knows, since the path is gone from + // the disk by the time anything asks. + forget(); + fs::remove_all(root_ / "Docs"); + srv.server().notify(FsEvent{FsEventKind::Removed, "Docs"}); + ASSERT_TRUE(wait_for(saw("Docs"))); + EXPECT_EQ(change_of("Docs"), ChangeKind::Removed); + { + std::lock_guard lock(mu); + for (const auto& c : seen) + if (c.path == "Docs") EXPECT_EQ(c.kind, NodeKind::Directory); + } +} + +// A removal for a path the mirror never had changed nothing, so reporting it +// would tell a consumer a file it never saw had just been deleted. +TEST_F(AgentTest, RemovalOfAnUnknownPathIsNotReported) { + LoopbackServer srv(root_, /*watch=*/false); + auto client = connect_client(srv.endpoint()); + ASSERT_NE(client, nullptr); + ASSERT_TRUE(client->connect().has_value()); + ASSERT_TRUE(client->fetch_snapshot().has_value()); + + std::mutex mu; + std::condition_variable cv; + std::vector seen; + int batches = 0; + client->set_invalidated_paths_hook([&](std::span changes) { + std::lock_guard lock(mu); + seen.insert(seen.end(), changes.begin(), changes.end()); + cv.notify_all(); + }); + client->set_invalidation_hook([&](const proto::InvalidationBatch&) { + std::lock_guard lock(mu); + ++batches; + cv.notify_all(); + }); + + srv.server().notify(FsEvent{FsEventKind::Removed, "never-existed.txt"}); + // Wait on the raw batch, which does arrive, then check that nothing was + // reported as applied: the two hooks differ exactly here. + { + std::unique_lock lock(mu); + ASSERT_TRUE(cv.wait_for(lock, 10s, [&] { return batches > 0; })); + EXPECT_TRUE(seen.empty()); + } +} + TEST_F(AgentTest, RescanRefetchesTheSnapshot) { // When the agent's watcher overflows it sends a single Rescan instead of the // events it lost. Ignoring it left the mirror stale until remount — exactly in diff --git a/tests/coalescer_test.cpp b/tests/coalescer_test.cpp index e14a419..dfced28 100644 --- a/tests/coalescer_test.cpp +++ b/tests/coalescer_test.cpp @@ -140,5 +140,127 @@ TEST(Coalescer, MarksEntriesThatAppeared) { EXPECT_TRUE(find("gone").appeared); } +// --- change kinds and rename pairing (the inotify bridge's input) ------------ + +// Every consumer downstream keys off `change`, and the whole point of keeping +// it separate from `kind` is that a burst collapses to one op without losing +// what the burst was. +TEST(Coalescer, ChangeKindSurvivesCollapsing) { + Coalescer c; + c.push({FsEventKind::Created, "new.txt"}, t0); + c.push({FsEventKind::Modified, "new.txt"}, t0); // still a creation, not a modification + c.push({FsEventKind::Modified, "old.txt"}, t0); + c.push({FsEventKind::Created, "gone.txt"}, t0); + c.push({FsEventKind::Removed, "gone.txt"}, t0); // last event wins + + auto out = c.take(); + auto change_of = [&](std::string_view p) { + for (const auto& op : out) + if (op.path == p) return op.change; + return ChangeKind::Unknown; + }; + EXPECT_EQ(change_of("new.txt"), ChangeKind::Created); + EXPECT_EQ(change_of("old.txt"), ChangeKind::Modified); + EXPECT_EQ(change_of("gone.txt"), ChangeKind::Removed); +} + +TEST(Coalescer, RenamePairSharesACookie) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "old.txt", 77}, t0); + c.push({FsEventKind::RenamedTo, "new.txt", 77}, t0); + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + EXPECT_EQ(out[0].change, ChangeKind::MovedFrom); + EXPECT_EQ(out[0].path, "old.txt"); + EXPECT_EQ(out[1].change, ChangeKind::MovedTo); + EXPECT_EQ(out[1].path, "new.txt"); + EXPECT_EQ(out[0].cookie, 77u); + EXPECT_EQ(out[1].cookie, 77u); +} + +// A write to the destination after the move must not cost the pairing: the +// consumer still has to see one move rather than a delete and an unrelated +// write. This is the git lock-file dance (rename into place, then touch). +TEST(Coalescer, WriteAfterMoveKeepsThePair) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "a", 5}, t0); + c.push({FsEventKind::RenamedTo, "b", 5}, t0); + c.push({FsEventKind::Modified, "b"}, t0); + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + EXPECT_EQ(out[0].change, ChangeKind::MovedFrom); + EXPECT_EQ(out[1].change, ChangeKind::MovedTo); + EXPECT_EQ(out[1].cookie, 5u); +} + +// The source coming back under its old name is exactly what git does, and it +// leaves the destination's half with nobody to pair with. A half-move must not +// reach a consumer: on its own the destination simply appeared. +TEST(Coalescer, StrandedMoveHalfDegrades) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "config", 9}, t0); + c.push({FsEventKind::RenamedTo, "config.lock", 9}, t0); + c.push({FsEventKind::Created, "config", 0}, t0); // recreated, so it is no longer a move source + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + for (const auto& op : out) { + EXPECT_FALSE(is_move(op.change)) << op.path; + EXPECT_EQ(op.cookie, 0u) << op.path; + EXPECT_EQ(op.change, ChangeKind::Created) << op.path; + } +} + +// The other direction: a removed directory takes the destination out of the +// batch, so the source is left as the surviving half. +TEST(Coalescer, MoveIntoARemovedDirectoryDegradesToRemoval) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "src.txt", 3}, t0); + c.push({FsEventKind::RenamedTo, "dir/dst.txt", 3}, t0); + c.push({FsEventKind::Removed, "dir"}, t0); // drops dir/dst.txt with it + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + auto change_of = [&](std::string_view p) { + for (const auto& op : out) + if (op.path == p) return op.change; + return ChangeKind::Unknown; + }; + EXPECT_EQ(change_of("src.txt"), ChangeKind::Removed); + EXPECT_EQ(change_of("dir"), ChangeKind::Removed); +} + +TEST(Coalescer, OverflowCarriesNoChangeKind) { + Coalescer c; + c.push({FsEventKind::Created, "a"}, t0); + c.push({FsEventKind::Overflow, {}}, t0); + auto out = c.take(); + ASSERT_EQ(out.size(), 1u); + EXPECT_EQ(out[0].kind, InvalidationKind::Rescan); + EXPECT_EQ(out[0].change, ChangeKind::Unknown); +} + +// repair_move_pairs is applied again by the sender and by the receiver, on +// their own op types, so it is exercised directly here too. +TEST(RepairMovePairs, MatchedPairSurvivesAndOrphansDegrade) { + std::vector ops{ + PlannedOp{InvalidationKind::Remove, "a", false, ChangeKind::MovedFrom, 1}, + PlannedOp{InvalidationKind::Upsert, "b", true, ChangeKind::MovedTo, 1}, + PlannedOp{InvalidationKind::Upsert, "c", true, ChangeKind::MovedTo, 2}, // no partner + PlannedOp{InvalidationKind::Remove, "d", false, ChangeKind::MovedFrom, 3}, // no partner + PlannedOp{InvalidationKind::Upsert, "e", true, ChangeKind::MovedTo, 0}, // no cookie at all + }; + repair_move_pairs(ops); + EXPECT_EQ(ops[0].change, ChangeKind::MovedFrom); + EXPECT_EQ(ops[1].change, ChangeKind::MovedTo); + EXPECT_EQ(ops[2].change, ChangeKind::Created); + EXPECT_EQ(ops[3].change, ChangeKind::Removed); + EXPECT_EQ(ops[4].change, ChangeKind::Created); + EXPECT_EQ(ops[2].cookie, 0u); + EXPECT_EQ(ops[3].cookie, 0u); +} + } // namespace } // namespace wsld diff --git a/tests/protocol_test.cpp b/tests/protocol_test.cpp index 7aeee85..41ae309 100644 --- a/tests/protocol_test.cpp +++ b/tests/protocol_test.cpp @@ -198,6 +198,52 @@ TEST(Messages, InvalidationRoundTrip) { EXPECT_EQ(got->ops[1].path, "gone"); EXPECT_EQ(got->ops[2].kind, InvalidationKind::Rescan); EXPECT_TRUE(r.empty()); + for (const auto& op : got->ops) { + EXPECT_EQ(op.change, ChangeKind::Unknown); // default when the sender says nothing + EXPECT_EQ(op.cookie, 0u); + } +} + +TEST(Messages, InvalidationCarriesChangeKindAndMoveCookie) { + InvalidationBatch b; + b.generation = 7; + b.ops.push_back(InvalidationOp{InvalidationKind::Upsert, "made", kFile, ChangeKind::Created, 0}); + b.ops.push_back(InvalidationOp{InvalidationKind::Upsert, "written", kFile, ChangeKind::Modified, 0}); + b.ops.push_back(InvalidationOp{InvalidationKind::Remove, "old", {}, ChangeKind::MovedFrom, 4242}); + b.ops.push_back(InvalidationOp{InvalidationKind::Upsert, "new", kFile, ChangeKind::MovedTo, 4242}); + b.ops.push_back(InvalidationOp{InvalidationKind::Remove, "deleted", {}, ChangeKind::Removed, 0}); + + std::vector buf; + Writer w(buf); + write_invalidation(w, b); + Reader r(buf); + auto got = read_invalidation(r); + ASSERT_TRUE(got.has_value()); + ASSERT_EQ(got->ops.size(), 5u); + EXPECT_EQ(got->ops[0].change, ChangeKind::Created); + EXPECT_EQ(got->ops[1].change, ChangeKind::Modified); + EXPECT_EQ(got->ops[2].change, ChangeKind::MovedFrom); + EXPECT_EQ(got->ops[3].change, ChangeKind::MovedTo); + EXPECT_EQ(got->ops[4].change, ChangeKind::Removed); + EXPECT_EQ(got->ops[2].cookie, 4242u); + EXPECT_EQ(got->ops[3].cookie, 4242u); + // The cookie rides along only for the two move kinds, so a batch without a + // move costs exactly one byte per op more than it used to. + EXPECT_EQ(got->ops[0].cookie, 0u); + EXPECT_EQ(got->ops[4].cookie, 0u); + EXPECT_TRUE(r.empty()); +} + +TEST(Messages, InvalidationRejectsAnUnknownChangeKind) { + std::vector buf; + Writer w(buf); + w.u64(1); // generation + w.varint(1); // one op + w.u8(static_cast(InvalidationKind::Remove)); + w.string("p"); + w.u8(200); // not a ChangeKind + Reader r(buf); + EXPECT_EQ(read_invalidation(r).error(), Errc::Corrupt); } TEST(Messages, HugeClaimedCountsFailWithoutHugeAllocation) { From bc9b98ed74f991e2c02414c8a1982358719afaf3 Mon Sep 17 00:00:00 2001 From: Zoltan Csizmadia Date: Sun, 13 Sep 2026 11:57:48 -0500 Subject: [PATCH 2/5] mount: never forward a bridge poke, and say when one is lost Fixes the Linux build (a dangling else in the new agent test, which GCC rejects with -Werror where the local harness did not), and hardens the one place in the bridge whose failure mode is unacceptable. If claim() did not recognise a request from the bridge's own poke thread, the handler fell through and forwarded it across the boundary. For a creation that means asking the agent to create the file that already exists there - which replaces the very file whose arrival prompted the notification with an empty one. The guard was fail- dangerous, which is exactly backwards for a guard whose whole purpose is to protect the served tree. The bridge issues no genuine mutations, so a mutation request from its thread is one of its own replays and nothing else. Handlers now refuse an unrecognised one instead of forwarding it. A refused poke costs a missed notification; a forwarded one costs data. Also adds the diagnostics this needed and did not have. Change delivery is invisible when it works and equally invisible when it does not, so a failed poke left no trace anywhere: the mount now names the first few and reports delivered/failed/dropped counts on exit. The CI change-notification battery runs with always(), so when the filesystem battery fails its verdict is available to say whether the two share a cause. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 4 +++ src/mount/fsnotify_bridge.cpp | 28 ++++++++++++++++++-- src/mount/fsnotify_bridge.hpp | 15 +++++++++++ src/mount/fuse_mount.cpp | 49 ++++++++++++++++++++++++++++++----- src/tools/wsldrive_main.cpp | 14 +++++----- tests/agent_test.cpp | 3 ++- 6 files changed, 97 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20ced5e..624632c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,7 +63,11 @@ jobs: # the far side has to raise a real inotify event here, or every watch-mode # tool silently does nothing. Which kernel hook each operation runs is not # observable from a unit test, so it can only be checked on a live mount. + # always(): the two batteries cover different things, and when the + # filesystem one fails the change-notification verdict is exactly what + # says whether the two failures share a cause. - name: Run the change-notification battery + if: always() run: bash scripts/inotify-conformance.sh # The same idea for Direction A - the feature the product is named for. Mounts diff --git a/src/mount/fsnotify_bridge.cpp b/src/mount/fsnotify_bridge.cpp index 82fe735..af32a67 100644 --- a/src/mount/fsnotify_bridge.cpp +++ b/src/mount/fsnotify_bridge.cpp @@ -23,6 +23,18 @@ namespace wsld::mount { namespace { +const char* change_name(ChangeKind c) noexcept { + switch (c) { + case ChangeKind::Created: return "creation of"; + case ChangeKind::Modified: return "change to"; + case ChangeKind::Removed: return "removal of"; + case ChangeKind::MovedFrom: return "move away from"; + case ChangeKind::MovedTo: return "move onto"; + case ChangeKind::Unknown: break; + } + return "change to"; +} + #ifdef __linux__ long this_tid() noexcept { return static_cast(::syscall(SYS_gettid)); } #endif @@ -80,6 +92,8 @@ FsNotifyBridge::Pretend FsNotifyBridge::pretend(int caller, std::string_view rel return active_.pretend_path; } +bool FsNotifyBridge::is_poke_thread(int caller) const noexcept { return is_self(caller); } + bool FsNotifyBridge::claim(int caller, Poke what, std::string_view rel, std::string_view rel2) { if (!is_self(caller)) return false; std::lock_guard lock(active_mu_); @@ -196,8 +210,18 @@ void FsNotifyBridge::deliver(const Job& job) { } } std::lock_guard lock(stats_mu_); - if (ok) ++stats_.delivered; - else ++stats_.failed; + if (ok) { + ++stats_.delivered; + return; + } + ++stats_.failed; + // A failed poke is a missed notification, which is silent by nature — the + // mount stays correct, so nothing else would ever say it happened. Name the + // first few, then stop: a storm of them must not become the log. + if (stats_.failed <= kMaxReportedFailures) + std::fprintf(stderr, "wsldrive: could not notify watchers of %s '%s'%s\n", change_name(job.change), + job.path.empty() ? "" : job.path.c_str(), + stats_.failed == kMaxReportedFailures ? " (further failures not reported)" : ""); } #ifdef __linux__ diff --git a/src/mount/fsnotify_bridge.hpp b/src/mount/fsnotify_bridge.hpp index 5507304..f2a3920 100644 --- a/src/mount/fsnotify_bridge.hpp +++ b/src/mount/fsnotify_bridge.hpp @@ -145,6 +145,18 @@ class FsNotifyBridge { /// retry or a duplicate request is forwarded like any other. [[nodiscard]] bool claim(int caller, Poke what, std::string_view rel, std::string_view rel2 = {}); + /// Whether `caller` is the bridge's poke thread at all, matching poke or not. + /// + /// This is what makes an unrecognised poke safe. The bridge never performs a + /// genuine mutation, so a mutation request arriving from its thread is one of + /// its own replays and nothing else. If `claim()` did not recognise it — a + /// bug, a path spelled differently than expected, a syscall the kernel turned + /// into a different operation — the only safe answer is to refuse it. The + /// alternative is forwarding it, and forwarding a create means asking the + /// agent to replace the very file whose arrival prompted the notification + /// with an empty one. + [[nodiscard]] bool is_poke_thread(int caller) const noexcept; + private: struct Job { ChangeKind change = ChangeKind::Unknown; @@ -208,6 +220,9 @@ class FsNotifyBridge { // cache are already correct, so the file reads right the moment anything // looks at it. Sized for a burst the size of an `npm install`. static constexpr std::size_t kQueueCap = 65536; + + // How many failed pokes are named individually before the log goes quiet. + static constexpr std::uint64_t kMaxReportedFailures = 8; }; } // namespace wsld::mount diff --git a/src/mount/fuse_mount.cpp b/src/mount/fuse_mount.cpp index d3890b3..c60f940 100644 --- a/src/mount/fuse_mount.cpp +++ b/src/mount/fuse_mount.cpp @@ -175,9 +175,24 @@ int caller_pid() { return c != nullptr ? static_cast(c->pid) : 0; } -bool claim_poke(FsNotifyBridge::Poke what, std::string_view rel, std::string_view rel2 = {}) { +enum class PokeVerdict { + NotOurs, // an ordinary request; forward it across the boundary + Claimed, // the poke we are waiting for; answer it here and raise the event + Refuse, // from the bridge's thread but not the poke we expected; see below +}; + +// A mutation request from the bridge's own thread is never forwarded, even when +// it is not the poke that was expected. The bridge issues no genuine mutations, +// so forwarding one could only re-apply a change the far side already made — +// and for a creation that means replacing the file whose arrival prompted the +// notification with an empty one. A refused poke costs a missed event; a +// forwarded one costs data. +PokeVerdict claim_poke(FsNotifyBridge::Poke what, std::string_view rel, std::string_view rel2 = {}) { FsNotifyBridge* b = ctx()->notify; - return b != nullptr && b->claim(caller_pid(), what, rel, rel2); + if (b == nullptr) return PokeVerdict::NotOurs; + const int caller = caller_pid(); + if (b->claim(caller, what, rel, rel2)) return PokeVerdict::Claimed; + return b->is_poke_thread(caller) ? PokeVerdict::Refuse : PokeVerdict::NotOurs; } // Fills `st` with a minimal stat for a path the mirror no longer has, so the @@ -323,7 +338,11 @@ int op_truncate(const char* path, OffT size, struct fuse_file_info*) { // mknod instead of open(O_CREAT) now work on the mount. int op_mknod(const char* path, ModeT mode, DevT) { const std::string rel = to_rel(path); - if (claim_poke(FsNotifyBridge::Poke::Mknod, rel)) return 0; + switch (claim_poke(FsNotifyBridge::Poke::Mknod, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } if ((mode & S_IFMT) != 0 && (mode & S_IFMT) != S_IFREG) return -EPERM; // no devices across the boundary auto r = ctx()->root->create_file(rel, static_cast(mode) & 0777u); return r ? 0 : err_to_errno(r.error()); @@ -331,21 +350,33 @@ int op_mknod(const char* path, ModeT mode, DevT) { int op_mkdir(const char* path, ModeT mode) { const std::string rel = to_rel(path); - if (claim_poke(FsNotifyBridge::Poke::Mkdir, rel)) return 0; + switch (claim_poke(FsNotifyBridge::Poke::Mkdir, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } auto r = ctx()->root->mkdir(rel, static_cast(mode) & 0777u); return r ? 0 : err_to_errno(r.error()); } int op_unlink(const char* path) { const std::string rel = to_rel(path); - if (claim_poke(FsNotifyBridge::Poke::Unlink, rel)) return 0; + switch (claim_poke(FsNotifyBridge::Poke::Unlink, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } auto r = ctx()->root->unlink(rel); return r ? 0 : err_to_errno(r.error()); } int op_rmdir(const char* path) { const std::string rel = to_rel(path); - if (claim_poke(FsNotifyBridge::Poke::Rmdir, rel)) return 0; + switch (claim_poke(FsNotifyBridge::Poke::Rmdir, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } auto r = ctx()->root->rmdir(rel); return r ? 0 : err_to_errno(r.error()); } @@ -353,7 +384,11 @@ int op_rmdir(const char* path) { int op_rename(const char* from, const char* to, unsigned int) { const std::string rfrom = to_rel(from); const std::string rto = to_rel(to); - if (claim_poke(FsNotifyBridge::Poke::Rename, rfrom, rto)) return 0; + switch (claim_poke(FsNotifyBridge::Poke::Rename, rfrom, rto)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } auto r = ctx()->root->rename(rfrom, rto); return r ? 0 : err_to_errno(r.error()); } diff --git a/src/tools/wsldrive_main.cpp b/src/tools/wsldrive_main.cpp index c3532a8..e15dfa2 100644 --- a/src/tools/wsldrive_main.cpp +++ b/src/tools/wsldrive_main.cpp @@ -676,12 +676,14 @@ int main(int argc, char** argv) { static_cast(st.rescans), static_cast(st.rescan_failures)); } } - // Worth a line when notifications were lost: a watcher that missed one sees - // a change late, and this is the only place that would say so. - if (const auto ns = fm.notify_stats(); ns.dropped != 0 || ns.failed != 0) - std::fprintf(stderr, "wsldrive: %llu change notifications dropped, %llu could not be delivered (of %llu)\n", - static_cast(ns.dropped), static_cast(ns.failed), - static_cast(ns.events)); + // Always reported, not only on loss. Change delivery is invisible when it + // works and equally invisible when it does not, so a count of what actually + // went out is the only way to tell the two apart after the fact. + if (const auto ns = fm.notify_stats(); ns.events != 0) + std::printf("change notifications: %llu delivered, %llu failed, %llu dropped, %llu rescans (of %llu)\n", + static_cast(ns.delivered), static_cast(ns.failed), + static_cast(ns.dropped), static_cast(ns.rescans), + static_cast(ns.events)); std::printf("unmounting...\n"); std::fflush(stdout); fm.unmount(); // agent (if auto-launched) is stopped by its destructor on return diff --git a/tests/agent_test.cpp b/tests/agent_test.cpp index 66053bb..668acd7 100644 --- a/tests/agent_test.cpp +++ b/tests/agent_test.cpp @@ -1087,8 +1087,9 @@ TEST_F(AgentTest, ChangeKindsReachTheClient) { EXPECT_EQ(change_of("Docs"), ChangeKind::Removed); { std::lock_guard lock(mu); - for (const auto& c : seen) + for (const auto& c : seen) { if (c.path == "Docs") EXPECT_EQ(c.kind, NodeKind::Directory); + } } } From 21f5065092cd8ea9d99f8a0cd910f07a587a6197 Mon Sep 17 00:00:00 2001 From: Zoltan Csizmadia Date: Sun, 13 Sep 2026 12:00:40 -0500 Subject: [PATCH 3/5] mount: claim the creation poke at op_create, where libfuse sends it The change-notification battery isolated this precisely: every event type worked except file creation, and the mount logged a refused poke for each file that appeared on the served side. libfuse's FUSE_MKNOD handler offers a regular file to the `create` handler first and falls back to `mknod` only if that answers ENOSYS. A mount with a `create` handler therefore never sees the poke at `mknod` at all, which is where it was being claimed. Before the previous commit made an unrecognised poke refuse rather than forward, that meant the poke was applied as a genuine create and replaced the file whose arrival prompted the notification with an empty one -- which is what the two external-change conformance checks were failing on. Claim it in op_create too. On a claim the handler returns without allocating a write handle, so the release that libfuse pairs with it has nothing to free. Co-Authored-By: Claude Opus 5 --- docs/inotify.md | 10 ++++++++++ src/mount/fuse_mount.cpp | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/docs/inotify.md b/docs/inotify.md index 23669af..92e1284 100644 --- a/docs/inotify.md +++ b/docs/inotify.md @@ -49,6 +49,16 @@ is deliberately over-determined, because the cost of getting it wrong is a user' removing anything: the request must come from the bridge's own thread, a poke of exactly that kind must be in flight, and it must name exactly that path. A poke is claimed once and then retired. +The recognition has to cover every handler the poke can reach, which is not always the obvious one. +A `mknodat` of a regular file arrives as `FUSE_MKNOD`, but libfuse offers it to the `create` handler +first and only falls back to `mknod` if that answers `ENOSYS` — so a mount with a `create` handler +never sees the poke at `mknod` at all. Getting that wrong is not a missed event; it forwards the +poke as a genuine creation and replaces the file that prompted it with an empty one. + +Which is why an unrecognised request from the bridge's thread is refused rather than forwarded. The +bridge performs no genuine mutations, so a mutation from its thread is one of its own replays and +nothing else. A refused poke costs a notification; a forwarded one costs data. + Two further guards keep the replay inside the mount. The bridge refuses to start unless its root is a FUSE mount (`statfs` reports `FUSE_SUPER_MAGIC`), and every poke re-checks that its target's parent is still on that mount's device. Both exist because the pokes are real filesystem calls: aimed diff --git a/src/mount/fuse_mount.cpp b/src/mount/fuse_mount.cpp index c60f940..c60b02b 100644 --- a/src/mount/fuse_mount.cpp +++ b/src/mount/fuse_mount.cpp @@ -278,6 +278,17 @@ int err_to_errno(Errc e) { int op_create(const char* path, ModeT mode, struct fuse_file_info* fi) { const std::string rel = to_rel(path); + // A creation poke arrives here, not at op_mknod. libfuse's FUSE_MKNOD handler + // tries `create` first for a regular file and only falls back to `mknod` if + // that answers ENOSYS, so the mount having a `create` means `mknod` is never + // reached for the case the bridge uses. Missing this cost the far side a + // file: the poke was forwarded as a genuine create, which replaces the file + // whose arrival prompted the notification with an empty one. + switch (claim_poke(FsNotifyBridge::Poke::Mknod, rel)) { + case PokeVerdict::Claimed: return 0; // fi->fh stays 0: no write handle, nothing to release + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } auto r = ctx()->root->create_file(rel, static_cast(mode) & 0777u); if (!r) return err_to_errno(r.error()); if (ctx()->writeback && fi != nullptr) From 24d1ee26d16555d36ce5e94b4e33134a1e8c6d10 Mon Sep 17 00:00:00 2001 From: Zoltan Csizmadia Date: Sun, 13 Sep 2026 12:03:22 -0500 Subject: [PATCH 4/5] test: brace the assertion, not the loop, to settle the dangling else GTest's EXPECT_ expands to an if/else, so an unbraced `if (cond) EXPECT_...` leaves the macro's else looking like it belongs to the outer if. Bracing the enclosing for-loop, as the previous attempt did, does not change that -- the braces have to go around the assertion. Also drops the ghost entry when a removal or move poke fails. On success the operation itself retires the dentry the ghost handed out; on failure it stays, and a path the far side has removed or moved away from then reads as still present to anyone who looks, for as long as the entry cache holds it. Co-Authored-By: Claude Opus 5 --- src/mount/fsnotify_bridge.cpp | 11 ++++++++++- tests/agent_test.cpp | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/mount/fsnotify_bridge.cpp b/src/mount/fsnotify_bridge.cpp index af32a67..61aaa62 100644 --- a/src/mount/fsnotify_bridge.cpp +++ b/src/mount/fsnotify_bridge.cpp @@ -318,6 +318,11 @@ bool FsNotifyBridge::poke_remove(const Job& job) { const std::string p = abs(job.path); const int rc = dir ? ::rmdir(p.c_str()) : ::unlink(p.c_str()); clear_active(); + // On success the removal itself drops the ghost dentry. On failure it does + // not, and the kernel is left holding an entry built from the fiction — so a + // path the far side deleted would read as still there, to anyone, until the + // entry cache lets go of it. Drop it explicitly instead. + if (rc != 0 && invalidate_) invalidate_(job.path); return rc == 0; } @@ -344,7 +349,11 @@ bool FsNotifyBridge::poke_move(const Job& job) { // The invalidation hook queues a punch for both paths anyway, but on another // thread and with no ordering against this one, so it cannot be relied on to // land after the rename. - if (rc == 0 && invalidate_) invalidate_(job.path2); + // + // A rename that failed leaves the ghost where it was built, on the source — + // a path the far side has moved away from, reading as still present to + // anyone who looks within the entry cache's lifetime. Drop that instead. + if (invalidate_) invalidate_(rc == 0 ? job.path2 : job.path); return rc == 0; } diff --git a/tests/agent_test.cpp b/tests/agent_test.cpp index 668acd7..404b424 100644 --- a/tests/agent_test.cpp +++ b/tests/agent_test.cpp @@ -1088,7 +1088,9 @@ TEST_F(AgentTest, ChangeKindsReachTheClient) { { std::lock_guard lock(mu); for (const auto& c : seen) { - if (c.path == "Docs") EXPECT_EQ(c.kind, NodeKind::Directory); + if (c.path == "Docs") { + EXPECT_EQ(c.kind, NodeKind::Directory); + } } } } From bc574bc76b0ec9a4985c33f305e8a07393470b1c Mon Sep 17 00:00:00 2001 From: Zoltan Csizmadia Date: Sun, 13 Sep 2026 12:06:30 -0500 Subject: [PATCH 5/5] docs: record the measured notification latency The issue asked for the added latency against a write made locally on the mount, measured rather than asserted. The conformance battery now reports both, and on a CI runner they are the same number: 4 ms either way, per event type, including a write deep inside a 10,000-file tree. That follows from where the work is. The invalidation had already crossed the boundary and updated the mirror before the bridge did anything, and the poke never leaves the local kernel. Read and write throughput are untouched for the same reason -- the bridge sits on the invalidation path, not the data path. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 +++++-- docs/inotify.md | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9da668..1c89311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,11 @@ no cooperation of any kind. Create, write, delete and rename each raise the matching event type, and a rename arrives as a paired `IN_MOVED_FROM`/`IN_MOVED_TO` with one cookie rather than as an unrelated delete and create. This is [microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739), - which nothing has solved. On by default; `wsldrive mount --no-inotify` turns it off. How it works, - and what it does not cover, is in [`docs/inotify.md`](docs/inotify.md). + which nothing has solved. Measured against a write made locally on the mount, it adds nothing: + **4 ms either way**, including a write deep inside a 10,000-file tree, and read/write throughput is + untouched because the bridge sits on the invalidation path rather than the data path. On by + default; `wsldrive mount --no-inotify` turns it off. How it works, and what it does not cover, is + in [`docs/inotify.md`](docs/inotify.md). - `mknod` on a regular file now works on the mount, instead of failing with `ENOSYS`. ### Changed diff --git a/docs/inotify.md b/docs/inotify.md index 92e1284..20a6992 100644 --- a/docs/inotify.md +++ b/docs/inotify.md @@ -80,7 +80,21 @@ the handler claims it. One syscall per changed path, answered by the FUSE loop out of the in-RAM mirror. No boundary crossing and no I/O, so the added latency over a write made locally on the mount is the mount's own -round trip. `scripts/inotify-conformance.sh` measures both and prints them side by side. +round trip. `scripts/inotify-conformance.sh` measures both and prints them side by side; on a GitHub +Actions runner, with a Linux agent on loopback: + +| change to event | | +|---|--:| +| write made locally on the mount | 4 ms | +| write made on the served side | 4 ms | + +Each individual event type lands in the same 4 ms, including a write deep inside a 10,000-file tree. +Nothing measurable is added, which follows from where the work happens: the invalidation had already +crossed the boundary and updated the mirror before the bridge did anything, and the poke never +leaves the local kernel. + +Read and write throughput are untouched. The bridge sits on the invalidation path, not the data +path, and adds no work to `read`, `write` or any other operation a tool performs on the mount. The queue is bounded at 65536 pending changes. Past that the oldest are dropped, and `wsldrive mount` says so on exit. A dropped notification is a late one, not a wrong one: the mirror and the page cache