Repository navigation
fuse: take the DLM write grant in page_mkwrite under the writeback cache - #247
Conversation
With the writeback cache, foreground requests can wait in userspace on DLM revokes. A revoke waits on page invalidation, which waits on writeback. If foreground requests held every entry of an io-uring queue, background writeback could not be sent and none of them would complete. When the connection has the writeback cache, each queue now keeps a share of its entries for background requests. Background requests may use a quarter of the entries, but at least 2. Foreground requests may use the rest, but at least 8. A foreground request over the limit waits on the new fuse_req_fg_queue and only moves to fuse_req_queue when a slot frees up. Background requests keep their own path to free entries and are held to their cap, except that each queue may always have one active background request. If a queue has fewer than 10 entries both minimums cannot hold, and the foreground minimum wins. The foreground count is released on completion, on commit errors and on entry teardown. Requests marked no_fg_limit skip the foreground limit. These are the page cache reads and writes (redfs_do_readfolio, redfs_send_readpages and redfs_send_write_pages) and the flush from write_inode. A page fault holds the inode range lock while its read is outstanding, and the revoke that is waiting for that range lock would never finish if the read were queued behind other foreground requests. The flush is sent by writeback itself. Without the writeback cache, dispatch is unchanged. Signed-off-by: Hai Zhong Zhou <hazhou@ddn.com>
| size_t blen = | ||
| (size_t)FUSE_FAULT_BATCH_PAGES << PAGE_SHIFT; | ||
|
|
||
| err = fuse_range_lock_acquire_init(fi, &rlock, bpos, |
There was a problem hiding this comment.
This blocks on the range lock, and then on the DLM round trip in fuse_get_dlm_lock(), while mmap_lock (or the per-VMA lock) is held. 94effa2 ("fuse: do not block the fault path under mmap_lock") removed exactly this from fuse_filemap_fault(). Both parts of that deadlock are still there:
- The buffered write path still faults in its source buffer while holding the range lock LOCKED. 125a726 says so and leaves
iomap_file_buffered_write()for a follow-up. - A pending
munmap/mprotectwriter still queues between the two in the rwsem.
Writer A holds the range lock LOCKED and waits for mmap_lock in fault-in. W waits for the write side of mmap_lock. B is here, holding mmap_lock for read and waiting for A's range lock. B's read isn't granted behind W's pending write, so all three hang. The waits are killable, but 94effa2 allowed blocking in place only once retries are used up (FAULT_FLAG_TRIED), and page_mkwrite has no retry.
Could the backstop move to the fault-around side instead? For shared-writable VMAs under writeback cache with DLM, use a vm_operations_struct without .map_pages, or a map_pages that skips folios with no recorded WRITE grant. The first touch then goes through fuse_filemap_fault(), which already records the WRITE grant for shared-writable VMAs and can drop mmap_lock and retry. page_mkwrite would keep only the non-blocking check above, perhaps with a warning if it misses.
Re-running the 94effa2 reproducer against this patch would show whether the deadlock is reachable here: a buffered write whose source is an mmap of a fuse file, racing munmap and a write fault on a page that fault-around mapped.
There was a problem hiding this comment.
You're right, and I've verified both halves in the tree. Party A exists: fuse_cache_write_iter holds the range lock LOCKED across generic_perform_write, whose fault-in of the source buffer takes mmap_lock for read — 125a726's commit message leaves the iomap_file_buffered_write() fault-in for a follow-up, and even the current generic_perform_write path is enough for the client-side cycle. Party B is this patch: fuse_range_lock_acquire_init plus the fuse_get_dlm_lock round trip run with mmap_lock (or the per-VMA lock) held, and unlike fuse_filemap_fault there is no release_fault_lock/VM_FAULT_RETRY escape — page_mkwrite never sees FAULT_FLAG_TRIED, so the blocking-in-place exemption from 94effa2 doesn't apply. With a pending mmap_lock writer queued between them, the three-party hang is reachable in a single process: exactly the 94effa2 shape plus fault-around.
I'll rework along the lines you suggest, with one adjustment: a filtering .map_pages isn't workable as-is, because the grant check (fuse_dlm_range_is_locked) takes down_read on the cache rwsem and .map_pages runs under RCU and must not block. So v2 takes your first variant — for shared-writable VMAs under the writeback cache with DLM, a vm_operations_struct without .map_pages, so the first touch always goes through fuse_filemap_fault, which already records the WRITE grant for VM_SHARED|VM_MAYWRITE and has the drop-and-retry machinery. page_mkwrite keeps only the non-blocking check (try-acquire, try-LOCKED, is_held) and returns VM_FAULT_NOPAGE with a ratelimited warning on a miss: a verified miss can then only mean a revoke in flight, whose unmap_mapping_range clears the PTE, and the re-fault goes through .fault, which re-takes the grant. A trylock-contended miss re-faults with mmap_lock released in between, so the range-lock holder can make progress. No sleeping acquisition remains under mmap_lock.
fuse_dlm_lock_range() flipped the mode of every READ range overlapping a newly recorded WRITE grant in its entirety, not just the part the grant covers. A whole-file READ grant -- read(2) leaves one behind via readahead -- overlapped by any WRITE grant was thus recorded as a whole-file WRITE grant, and fuse_dlm_lock_is_held() then reported WRITE coverage the server never gave. Every write-side check trusts that record: the fault fast path, fuse_get_dlm_lock()'s early exit, and the write path's re-validation all see the range as covered, no lock request reaches the server, and writeback sends WRITEs the server observes under a read lock. Split the overlapping READ range at the grant boundaries and upgrade only the intersection. The splits preserve their mode, so an allocation failure partway through still leaves the tree describing exactly the grants held. Found probing the fault-around fix in the next commit: the probe's first-touch write fault should have requested a WRITE grant for its batch window, but the poisoned cache reported the page already covered and no request reached the server. Signed-off-by: Allison Henderson <allison.henderson@ddn.com>
With the writeback cache on, page_mkwrite skipped the DLM lock request
entirely, on the assumption that fuse_filemap_fault() had already taken
a write grant for any page a shared-writable mapping could dirty. That
assumption does not hold: fault-around (filemap_map_pages) PTE-maps any
resident folio in its window without calling ->fault for it, so a folio
read in under a READ grant -- by read(2) or readahead -- can take its
first write fault with no WRITE grant ever requested. The page is
dirtied anyway and writeback later sends a WRITE the server sees under
a read lock (DFS cases 040, 080, 081, 090 and 200).
page_mkwrite cannot take the grant itself: it is called with mmap_lock
(or the per-VMA lock) held and has no VM_FAULT_RETRY protocol to drop
it, while the buffered write path holds the IO range lock LOCKED while
faulting in its source buffer, which takes mmap_lock. With a pending
mmap_lock writer queued between the two, blocking in page_mkwrite on
the range lock or the DLM round trip recreates the three-party deadlock
that 94effa2 ("fuse: do not block the fault path under mmap_lock")
removed from the fault path.
Instead, give shared-writable VMAs under the writeback cache with DLM a
vm_operations_struct without ->map_pages, so the first touch of every
page goes through fuse_filemap_fault(), which records a WRITE grant for
its batch window before any PTE is installed and already drops
mmap_lock to block. page_mkwrite keeps a non-blocking check of the
recorded grant -- the same LOCKED fence the fault fast path uses -- and
returns VM_FAULT_NOPAGE on a miss. A verified miss can only mean a
revoke in flight, whose invalidation clears the PTE, so the re-fault
resolves through fuse_filemap_fault(); a trylock-contended miss
re-faults with mmap_lock released in between, so the range lock holder
can make progress.
This also makes fuse_filemap_fault()'s FAULT_FLAG_TRIED pass take the
drop-and-retry slow path instead of blocking in place: with every first
touch of a shared-writable page now funnelled through ->fault, blocking
there on the range lock with mmap_lock held steps into page_mkwrite's
old role in the same three-party cycle, and did so instantly under a
buffered write sourced from an mmap of the file racing munmap and a
write fault. Retries may repeat (4064b98 "mm: allow VM_FAULT_RETRY for
multiple times"), so waiting for a contended range must happen with
mmap_lock dropped, every time; blocking in place remains only for
callers that allow no retries at all.
The non-writeback-cache arm keeps the existing single-page
fuse_get_page_mkwrite_lock() behaviour.
Reported-by: Hai Zhong Zhou
Suggested-by: Yong Ze Chen
Signed-off-by: Allison Henderson <allison.henderson@ddn.com>
a6456db to
85af4a6
Compare
|
v2 pushed. The rework follows the direction from the review thread, and running the suggested reproducer shape turned up more than expected, so the PR is now two commits. What the reproducer showed. I wrote the 94effa2-shaped repro as suggested: a buffered
The intermediate deadlock was not in The grant cache was lying. Verifying the fix with the fault-around probe exposed an independent bug, now the first commit: Validation. On the lockdep 6.17.13 kernel against the mock server ( I don't have permission to request reviews on this repo, so pinging directly: @hbirth @yongzech @hazhou-ddn @cding-ddn -- when you have a chance. |
|
@achhenderson this was based on HaiZhongs uring change. Since that patch changed this naturally conflicts. I have picked your two relevant patches for DDNStorage and do a backport to 6.12 and will close this PR since the changes are already in the DDNStorage repo |
4 participants
Problem
With
writeback_cacheenabled,fuse_page_mkwrite()skipped the DLM lockrequest entirely, assuming
fuse_filemap_fault()had already taken a WRITEgrant for any page a shared-writable mapping could dirty. Fault-around
(
filemap_map_pages) breaks that assumption: it PTE-maps any resident folioin its window without calling
->fault, so a folio read in under a READgrant (by
read(2)or readahead) takes its first write fault straightthrough
page_mkwritewith no WRITE grant ever requested. The page isdirtied anyway and writeback later sends a WRITE the server sees under a
read lock. Reported by Hai Zhong Zhou against DFS cases 040, 080, 081, 090
and 200.
Fix
Make
page_mkwritethe backstop. Writenotify routes every clean-to-dirtytransition of a shared mapping through it, so it is the one reliable choke
point. The new code first checks for a recorded WRITE grant covering the
page -- anything that did go through
fuse_filemap_fault()on ashared-writable VMA is already covered, so the common case costs no server
round trip. Only when no grant covers the page is a DLM lock requested,
over the same centred batch window the fault path uses
(
FUSE_FAULT_BATCH_PAGES), so neighbouring mkwrites hit the fast path too.The non-writeback-cache arm keeps the existing single-page
fuse_get_page_mkwrite_lock()behaviour.Validation
All on a lockdep kernel (6.17.13),
passthrough_hp --dlm --dlm-exactwith the writeback cache.
Targeted reproducer (
preadthe file, read-fault page 20, first writeto page 30), with kprobes on
fuse_filemap_fault,filemap_map_pagesandfuse_page_mkwriteconfirming the identical fault shape on both arms:fault-around resolved the read fault (no
->faultcall at all) and thewrite hit
page_mkwriteonly.dlm_wb_lock(90112, 155647, type=2)-- the batch window covering page 30xfstests two-arm comparison (mmap-heavy set: generic/029 030 075 112
127 246 247 263 325 346 428 437), baseline vs fix: 11 passed / 0 failed in
both arms, no regressions, lockdep stayed armed, zero splats.