fuse: add all improvements for io500 - #191
Merged
hbirth merged 11 commits intoAug 13, 2026
Merged
Conversation
Collaborator
Author
|
@achhenderson @hazhou-ddn the key was to create a way to query whether we have a region marked as exclusive (this was not complete before since we never needed it) ... now we can operate in a loop ... if we lost the dlm lock we can reacquire it without holding the semaphore. This still has a small problem, where we probably acquire a lock twice when two writer were blocked by the notification ... but I think our code can handle that |
Collaborator
Author
|
the second commit I have included because my kunit tests have found those problems. I have not included the AI generated tests, since putting them in the fs/fuse/ directory would make them part of redfs.ko and I don't think that's a good idea |
hazhou-ddn
reviewed
Jul 27, 2026
hbirth
force-pushed
the
redfs-ubuntu-hwe-6.17.0-16.16-24.04.1
branch
from
July 27, 2026 17:30
ac86c7d to
e8231d1
Compare
Collaborator
Author
|
@hazhou-ddn OK, done |
hbirth
force-pushed
the
redfs-ubuntu-hwe-6.17.0-16.16-24.04.1
branch
from
July 27, 2026 18:21
e8231d1 to
d4ff3ce
Compare
hazhou-ddn
previously approved these changes
Jul 28, 2026
hazhou-ddn
left a comment
hazhou-ddn
left a comment
There was a problem hiding this comment.
After addressing Alison and my comments, all others look good to me.
hbirth
force-pushed
the
redfs-ubuntu-hwe-6.17.0-16.16-24.04.1
branch
from
July 28, 2026 09:29
d4ff3ce to
8983871
Compare
Collaborator
Author
|
had AI do a review of this change and it came up with a couple of improvements ... one a nice race (8983871), that I'm not sure we have mitigated |
Collaborator
Author
|
can we merge this? |
fuse_dlm_try_merge() locates the first merge candidate by walking from rb_first_cached() until it reaches the region just granted. The walk runs under the write-held cache rwsem on every fuse_dlm_lock_range() call, and the tree it walks holds every cached grant of the inode. Strided writers (IOR hard-write) accumulate grants that cannot merge with each other, so the tree keeps growing and every new grant pays a scan of all grants below it -- quadratic over the run, with fuse_dlm_range_is_locked() readers blocked behind each scan. Seed the merge with fuse_page_it_iter_first() on the region widened by one unit to each side instead; finding the lowest overlapping range is what the interval tree is there for. This also repairs two edge cases of the linear scan: a region starting at offset 0 made 'start - 1' wrap so the scan degenerated and merging was silently skipped, and a region ending at U64_MAX overflowed 'end + 1' in the loop bound, ending the merge after the first range. Both bounds now saturate. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Both cached IO paths request their DLM lock first and then go to sleep on things a NOTIFY invalidate can be holding: the read path blocks on the coherency gate (writer priority), the write path additionally sleeps on a contended i_rwsem. A NOTIFY invalidate running in that window revokes exactly the lock just granted (fuse_dlm_unlock_range()), so the task wakes up and populates or dirties the page cache with no DLM coverage. Close the window without ever sending a FUSE_DLM_WB_LOCK request while holding the gate (a grant that had to wait on an invalidate delivered to this same client would deadlock against our own gate hold): - Drop the lock record under the gate write side in fuse_reverse_inval_inode(), so revocation and page drop are one atomic step with respect to the gate. - After entering the gate read side, re-check the grant against the live lock tree; if it was revoked while we waited, drop the gate, re-request, re-enter and check again. With the revoke now gated, passing the check means the lock cannot go away for the whole gate hold: a revoke arriving mid-operation parks until the IO is done. - Keep the write path's lock request ahead of the inode lock: the round trip must not capture the writer-priority i_rwsem for unbounded cluster-grant latency, and the in-gate re-validation already closes the grant-to-use window. Only O_APPEND moves below the lock, because its range is the current EOF -- stable only under the exclusive inode lock. This also fixes the append range itself: generic_write_checks() rewrites ki_pos to i_size for IOCB_APPEND, so the old 'i_size + ki_pos' double-counted (ki_pos is absolute, not relative) and locked a range disjoint from where the data lands. fuse_get_dlm_lock() now reports whether the grant is recorded, and the re-validation never re-requests a grant that failed, so it cannot spin (the read path seeds this from its pre-gate request instead of discarding that result). A grant the server issued but that could not be recorded (small-allocation -ENOMEM) reports FUSE_DLM_GRANT_UNRECORDED: coverage exists cluster-wide, so failing the IO would be wrong -- it proceeds, it just cannot re-validate. Empty ranges are trivially held, so a zero-length IO neither sends a doomed request nor spins in the retry loops. The write path returns a real failure to the caller instead of dirtying the cache without DLM coverage; only -ENOSYS still degrades to a plain cached write, since it means the server has no DLM at all and clears fc->dlm. The read path keeps falling through unlocked and additionally bounds its retry: a reader-only inode has no force-DIO latch to end a revoke storm, so after a few re-requests the read is served unlocked rather than looping in the kernel for the duration of the storm. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
The NOTIFY_INVAL_INODE revoke computed
fuse_dlm_unlock_range(fi, offset, pg_end == -1 ? 0 : offset + len - 1)
which is wrong at both degenerate ends: a to-EOF invalidate (len <= 0,
e.g. a remote truncate) with offset > 0 becomes the inverted range
[offset, 0] and removes nothing, so the revoked grant stays visible to
the re-validating IO paths forever -- cached writes with no
server-side lock, zero-filled RMW reads; and an invalidate of byte 0
(offset 0, len 1) becomes [0, 0], the "destroy everything" sentinel,
wiping every grant of the inode.
Map the range in one helper shared by the gated and the ungated
branch: to-EOF revokes through U64_MAX, and the bounds widen to page
boundaries to match how grants are recorded -- revoking too much only
costs a re-request, too little leaves a stale grant.
Drop the in-band (0, 0) sentinel: whole-file invalidates walk the
normal removal path, release-all is fuse_dlm_cache_release_locks(),
and an inverted range is rejected with -EINVAL instead of silently
ignored.
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
…y gate Two revocation paths bypassed the revoke-under-gate invariant the IO paths re-validate against: - fuse_reverse_inval_inode() skipped the gate once mapping_mapped() turned true, revoking concurrently with gate holders -- but fuse_cache_read_iter()/fuse_cache_write_iter() enter the gate unconditionally, so a single mmap() reopened the race. Keep the gate for mmapped inodes; only the force-DIO latch stays disabled for them (a mapping needs the page cache, and fuse_file_mmap() reverts any latch it races with). - The local truncates in fuse_do_setattr() -- the atomic-O_TRUNC open shortcut and the after-setattr trim -- revoked and dropped the cache with no gate at all, so an already re-validated reader could repopulate the truncated range. Take the gate write side around revoke + drop. This cannot deadlock: both run under exclusive i_rwsem, which no gate holder waits on (the write path takes i_rwsem before the gate, the read path never takes it). Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A FUSE_DLM_WB_LOCK reply and a NOTIFY invalidate are serviced on different threads, so a revoke aimed at the grant a reply carries can be processed before fuse_get_dlm_lock() records it: the revoke finds nothing to remove, and the requester then records an already-dead grant that no later NOTIFY will target -- a permanent false positive for the re-validating IO paths. Add a revocation generation to the lock cache, bumped under the cache lock by every revoke path -- unconditionally, because the racing revoke sees an empty overlap precisely when the grant is in flight. fuse_get_dlm_lock() samples it before sending and records through fuse_dlm_lock_range_gen(), which refuses with -EAGAIN once the generation has moved; the grant is then re-requested instead of recorded, bounded so a revoke storm cannot pin the IO here (past the bound the failure reports like any request failure). Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
On the pinned-header send path fuse_uring_dispatch_ent() calls io_uring_cmd_done() directly from the request submitter's context with IO_URING_F_UNLOCKED. Every fuse-uring command is marked cancelable, so io_uring_cmd_del_cancelable() has to take ctx->uring_lock from that foreign task on every request. The ring task holds this mutex for the whole ->uring_cmd() issue path (io_uring_enter() submission), where it also wakes the submitter of the request it just committed - before releasing the lock. The freshly woken submitter usually preempts the ring task on the same CPU, and its next dispatch then blocks on the very mutex its victim still holds. The preempted owner is merely runnable and gets no wakeup boost, so under CPU pressure this convoy costs milliseconds per request while the daemon's actual work is a few microseconds. Keep the copies into the pinned pages in the submitter's context - that is the point of the pinning - but defer the command completion to ring task task-work, like the non-pinned path already does. There io_uring_cmd_del_cancelable() runs under the task-work batch's already held uring_lock, and the submitter no longer touches ctx->uring_lock at all on the fast path (only the rare copy-failure fallback still can). Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_uring_commit_fetch() runs from the ->uring_cmd() issue path with ctx->uring_lock held by io_uring_enter(). Ending the committed request there means wake_up(&req->waitq) runs while the ring task still holds the mutex, and the freshly woken submitter typically preempts the ring task on the same CPU right away - leaving the lock held by a merely runnable task for however long the runqueue is backed up. wake_up_sync() would not help here: WF_SYNC only biases task placement in select_task_rq_fair(), check_preempt_wakeup_fair() never looks at it, so it cannot stop the wakee from preempting the waker. Instead defer fuse_request_end() to task work on the ring task. It runs after the submission path has dropped uring_lock - at latest on return to userspace, typically at io_cqring_wait() entry via io_run_task_work() - so the ring task finishes its critical section and the entry recycling in fuse_uring_next_fuse_req() before the submitter gets the CPU. This also moves the request ->end() callbacks off the locked section, shortening the uring_lock hold time. Fall back to completing in place when the command was issued from an unlocked context (io-wq punt) or task_work_add() fails because the task is exiting. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A connection whose server stops answering has no timeout at all unless the server asked for one at FUSE_INIT or the admin set one of the fs/fuse sysctls. The requesting task itself waits killably and is therefore skipped by the hung task detector, but everything queueing up behind it -- inode locks, folio locks, writeback, unmount -- sleeps uninterruptibly and does get reported, or panics the box where hung_task_panic is set. The result is a machine that logs (or dies on) a hang the filesystem could have turned into an I/O error by itself. Fall back to the hung task timeout in that case, i.e. exactly where the timeout is not otherwise set: a request outstanding for as long as the system is willing to tolerate a stuck task aborts the connection. The timeout checks only run every FUSE_TIMEOUT_TIMER_FREQ seconds, so aim one period below hung_task_timeout_secs to abort before the detector reports on the waiters. hung_task_timeout_secs of 0 (detector disabled, also the value used when CONFIG_DETECT_HUNG_TASK is off) keeps the old behaviour of no timeout. The precedence of an explicit configuration is unchanged: the server's FUSE_INIT timeout, then default_request_timeout, then the clamp against max_request_timeout all take effect first, and the fallback is only consulted when all of them leave the timeout at 0. The value is read once at connection setup, like every other timeout source. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_get_dlm_lock() re-requests the DLM lock when fuse_dlm_lock_range_gen() returns -EAGAIN, i.e. a revoke was processed while the grant request was in flight and the grant it returned may already be dead. That restart was bounded by FUSE_DLM_RECORD_TRIES, and past the bound the function returned -EAGAIN. Reporting that as a request failure is wrong: no one else holds the range at that point, the caller simply lost a race with a revoke, and the write path turns the error into a failed write. Retry unconditionally instead. Every pass issues a fresh FUSE_DLM_WB_LOCK round trip to the server, so a revoke storm throttles the loop rather than spinning it, and the loop ends as soon as one grant survives long enough to be recorded. Drop the now-unused bound and its counter. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_reverse_inval_inode() latches an inode into direct IO when a remote writer keeps invalidating a file that is also open for writing here. That trades the writeback cache away for as long as the latch holds, which only pays off on workloads that actually see such invalidation storms. Make it opt-in through a new 'enable_notify_dio' module parameter, default off. FUSE_I_FORCE_DIO is set in exactly one place, so gating that single site is enough: every other reference only tests or clears the bit, and with the bit never set those paths behave as they did before the latch existed. The moving average is still folded on every invalidation while the parameter is off, so enabling it at runtime takes effect on the next storm instead of after a warm-up. Clearing it stops new latches but leaves already-latched inodes to run out on the usual exits (last writer closes, or mmap). Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
hbirth
force-pushed
the
redfs-ubuntu-hwe-6.17.0-16.16-24.04.1
branch
from
August 12, 2026 15:18
8983871 to
ac617b0
Compare
hbirth
changed the title
fuse_cache_write_iter() picks between the exclusive and the relaxed shared inode lock with fuse_cache_wr_exclusive_lock(), which returns "shared" only when fc->dlm is set. Since "fuse: re-validate the DLM grant after waiting on the coherency gate" that decision is made before the DLM write lock is requested, and the request itself can clear fc->dlm: a server that does not implement FUSE_DLM_WB_LOCK answers -ENOSYS, which fuse_get_dlm_lock() handles by clearing fc->dlm and reporting success. The write then proceeds in a state that was unreachable before: the shared lock was chosen believing DLM was active, but DLM is now known to be absent. That combination is not benign. The shared path claims the i_size extension up front so iomap never updates i_size itself, which also stops iomap_block_needs_zeroing() from ever firing for the write's own range; the zero-fill that compensates for it in fuse_iomap_read_folio_range() is gated on fc->dlm and so no longer runs. An expanding write therefore falls through to fuse_do_readfolio() and sends a READ for a range past EOF that cannot hold data. Against a file the client opened write-only the server fails that read -- passthrough_hp returns EBADF -- and the write fails with it. Re-evaluate the lock mode after the request, while no lock is held yet, so a server without DLM support gets the exclusive path and iomap's own beyond-EOF zeroing back. This showed up as generic/105, 123, 215, 246, 378, 423, 519 and 597 all failing with EBADF on the first write to a newly created file, and bisected to the commit named above. Signed-off-by: Allison Henderson <allison.henderson@ddn.com> (cherry picked from commit 4a9ddcf)
Collaborator
Author
|
I think we have done enough tests to have some confidence to merge ... @achhenderson @hazhou-ddn @cding-ddn any objections? |
hbirth
merged commit Aug 13, 2026
cde4474
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
3 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.
No description provided.