6.17 Fix the fencing around readahead to not clash with an invalidation notification - #208
Merged
hbirth merged 16 commits intoSep 9, 2026
Conversation
A folio made uptodate from the server is served to every later reader, so the grant it was fetched under has to be held from the confirmation until the bytes are in the page cache. Without that a revoke sweeping the range leaves the fill behind it: the folio stays cached, uncovered, and the server sends no further notify for a lock this client no longer holds. Confirm the grant under a pin, as the write path does. Refused, or not covered, unlock the folio and back off with AOP_TRUNCATED_PAGE: neither the wait nor the grant request may be taken with a page lock held. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A readahead reply lands in the page cache from the task that processes it, so the pin over the folios has to span the request: taken before it is sent and dropped once the folios are filled and unlocked. A revoke of the range waits for that and drops the folios after; one already draining refuses the pin and the window goes back unfilled. The node therefore outlives the pinning task, which fuse_dlm_unpin() cannot express. Add a span-owned pin, dropped by node and carrying no owner for the by-owner search to match. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A read grant may not be requested under a page lock, so every buffered read path asks for one before it enters the page cache: buffered read, splice read and read fault. The window covers what readahead may add beyond the read, bounded by the file. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
->readahead runs with every folio of the window locked, so the grant request it sent went out under those locks and a revoke of the window had to be given up on. The read now takes the grant before the page cache is entered; fill only what it covers. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
POSIX_FADV_WILLNEED and readahead(2) fill through ->readahead, which now fills only what a grant already covers. Take the grant for the advised range first, with no folio held. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A streamed write goes to the server out of the caller's pages, so its bytes are in no page cache and a revoke of the range finds nothing to flush. Hold the grant across the FUSE_WRITE, as the writethrough edges do. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
An O_DIRECT read or write neither fills nor dirties the page cache, so a grant over its range covers nothing and only conflicts with the rest of the cluster. Skip it on both sides. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
hbirth
changed the title
hbirth
changed the title
fuse_attr_cache_mask() decides whether the server's smaller size may be taken, then sleeps in the grant query before its answer is used. A write below EOF extends nothing, so it bumps neither fi->attr_version, which would drop the reply, nor fi->size_extenders, which would hold the size. Folios dirtied in the doomed range during that window are invisible to all three legs. Re-testing closer to the truncate is not enough on its own. truncate_pagecache() runs after fi->lock is dropped and holds nothing a writer holds, so a folio dirtied between the decision and the walk, or during the walk, is discarded anyway, with no error to report it: the bytes are gone and a later fsync() succeeds over the hole. A real truncate may discard the cache because fuse_set_nowrite() and i_rwsem hold the writers off; an attribute reply holds off nothing. So keep the folios, and re-test the size under fi->lock where nothing sleeps between the answer and its use. If the size was wrong the folios are written back and the size recovers, and fuse_flush_writepages() crops against a high water mark so nothing is clipped meanwhile. If it was right, the revoke that had to precede it already laundered and dropped the range, so there is nothing left to discard. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_writepage_need_send() takes its alignment bound from wbc->range_end, which a cyclic writeback does not set: write_cache_pages() runs to the end of the mapping there and leaves the field at zero. The bound is then zero as well, the test is true for every aligned index, and each folio is sent as a request of its own. Run to the end of the mapping when the pass is cyclic. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_writeback_kick_stream() reads fi->write_stream_start twice in the same decision, once to round down into the start of the range and again to compare against its end. A writer moving the mark in between inverts the range the kick is given. Racing writers are meant to cost a kick rather than correctness, which needs each mark read once into a local. Take the end from this write's own position rather than reading back the mark just stored, and annotate the accesses. Still no lock: the run is a hint, and nothing waits for it. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Collaborator
Author
|
Just as a heads up ... I have done some reviews with AI ... so a lot of comments got corrected, too. |
The notify-driven latch required a local writer. Under the same storm a reader-only inode refills the page cache between two invalidations and has it dropped again before it can be read twice, so latch on any local opener and let the whole-mapping drop free the folios. The average behind the latch only folds on arrival, so an inode that stops being notified would stay uncached. Clear the latch at the top of the IO paths once the last invalidation is FUSE_NOTIFY_DIO_COLD old, and on release only for a writer. Keep the mapping empty while latched: splice reads copy through ->read_iter, readahead declines. The whole-mapping drop launders only what the mapping says may be dirty. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A read of the same buffer size arriving over and over is a reader working through the file a record at a time, and the folios it fills are dropped unread. Serve a streamed read of FUSE_READ_STREAM_MIN or more into the caller's own pages: one copy instead of two, no folios, and no read grant. What that gives up is the readahead of the next record, not the wait for this one. The detector is the one the write side uses, taking the field pair now so reads keep their own average, and the size is folded for every read that could be cached. Dirty folios over the range are sent first: a direct read does not look in the page cache, and fuse_direct_io() flushes only for a file opened FOPEN_DIRECT_IO. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_set_nowrite() biases an inode-wide counter and asserts BUG_ON(fi->writectr < 0) under fi->lock, which holds only with i_rwsem taken exclusive. Parallel direct writes hold it shared, so two of them reaching fuse_sync_writes() in fuse_direct_io() kill the second inside the spinlock, and every later user of the inode spins on it. Wait on the folios of the caller's own range instead when the lock is shared, which is the range the writeback test above already asked about. Reachable from the streamed write, and from any parallel write over a page cache that has not drained once the direct-IO latch is set. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
The notify that sets the latch drops the mapping without an inode lock, so a cached write already past both latch checks keeps dirtying behind it. Writeback then puts those folios on the server on top of the direct writes that replace them, and direct reads miss them entirely. The per-range flush in the reroute paths covers only the range it is about to write, and only until the next dirty. Take i_rwsem exclusive once per latch instead. fuse_cache_write_iter() dirties under it, so holding it means every such writer has finished, and one that takes the lock afterwards rechecks the latch and reroutes before touching the cache. Flush and drop the mapping there and record it in FUSE_I_FORCE_DIO_DRAINED, which is cleared with the latch. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
The cold check cleared the latch on a quiet inode even with a writer still on it, and nothing tracks the forced direct writes in flight, so a cached write could start beside one. Require write_files to be empty as well, which is what the last writer's release already waits for. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
The three paths that leave the forced direct IO latch dropped the mapping without writing it back first, so a folio a write racing the latch left dirty reached the server through invalidate_inode_pages2(), which launders one FUSE_WRITE per page. Send those with writeback, which batches them into max_write requests, and keep the error on the mapping for fsync. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
hbirth
merged commit Sep 9, 2026
9a903a5
into
DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1
2 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
and various other small fixes and code simplifications like explicit exclusion of dlm from O_DIRECT opened files.