Backport performance changes from Ubuntu HWE 6.17 - #195
Merged
Conversation
hbirth
changed the title
hbirth
requested review from
achhenderson,
bsbernd,
cding-ddn,
hazhou-ddn and
yongzech
August 5, 2026 10:39
Collaborator
Author
|
@yongzech I will merge this after the same changes to 6.17 are merged ... in case we find a problem with this |
fuse_do_readpage() turns the DLM's "granting this read would deadlock" answer into AOP_TRUNCATED_PAGE: the caller is supposed to drop the page and come back, which lets the conflicting holder -- typically a page invalidation on this very node -- take the folio lock and make progress. ->read_folio() callers in filemap.c implement that contract. ->write_begin() callers do not. generic_perform_write() only breaks out of the copy loop on a negative return: status = a_ops->write_begin(file, mapping, pos, bytes, &folio, &fsdata); if (unlikely(status < 0)) break; offset = offset_in_folio(folio, pos); AOP_TRUNCATED_PAGE is 0x80001, so a partial-folio buffered write that loses the race walks straight into offset_in_folio() on the uninitialised folio that fuse_write_begin() never assigned. Handle the retry where the contract is understood. The cleanup path has already dropped the folio lock at that point, so looping back is exactly the recovery AOP_TRUNCATED_PAGE asks for, and the fresh READ each pass sends keeps the loop paced by the server rather than spinning on it. Reset err inside the loop as well: it doubled as the -ENOMEM the __filemap_get_folio() failure path returns, and would otherwise carry AOP_TRUNCATED_PAGE back out through the same hole on a retry. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
-EAGAIN is semantically overloaded for a DLM error and not self describing, switch to -EDEADLK. In order to allow a graceful daemon change, -EAGAIN is kept for now. [hbi: adapted -- this tree has no iomap buffered-write path, so only the fuse_do_readpage() translation applies and the Documentation/filesystems/fuse note the original also updates does not exist here. Both codes are additionally gated on fc->dlm: without a DLM connection there is no lock-ordering conflict to report, so an -EAGAIN arriving from such a server is an ordinary error the caller has to see, not a request to drop the page and retry.] Signed-off-by: Bernd Schubert <bernd@bsbernd.com> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A FUSE_NOTIFY_INVAL_INODE data invalidation means another (remote) entity
is modifying the file.
Rather than react to a single notify, keep a per-inode moving average of
how fast data invalidations arrive for the whole file: an EWMA of the
inter-arrival interval, updated under fi->lock on every notify
(fuse_notify_inval_hot()). The inode is latched only once the average
spacing drops below an internal threshold (FUSE_NOTIFY_DIO_INTERVAL) while
a local writer is open; a lone or occasional notify keeps the average high
and does not trip the switch. The heuristic has no external knob -- its
parameters (EWMA weight, threshold, seed) are source-level constants.
Introduce the forced-direct-IO latch (FUSE_I_FORCE_DIO):
- fuse_reverse_inval_inode() folds each data invalidation into the moving
average and sets the latch when it trips with a local writer present;
- fuse_file_{read,write}_iter() and fuse_cache_write_iter() route to the
direct path while latched; fuse_dio_{wr_exclusive_lock,lock,unlock}()
use the shared parallel-dio path and bypass the cached/uncached
accounting;
- fuse_file_io_open() opens new files uncached so they do not re-enter
caching mode;
- fuse_prepare_release() clears the latch once the last writer is gone
and fuse_file_release() drops any clean folios a racing read
repopulated; fuse_file_mmap() reverts to caching mode (a mapping needs
the page cache).
Latching to direct IO is only coherent if no buffered write can deposit
dirty folios into the page cache after it has been dropped. Add a
per-inode rw_semaphore, wb_inval_rwsem, to serialise the buffered-write
page-cache dirtying against the latch transition. The writeback path
holds it for read around the dirtying and re-checks the latch under it;
fuse_reverse_inval_inode() holds it for write around its invalidate +
latch set. The notification may be delivered by the same server thread
that still owes a reply to an in-flight write holding the inode lock, so
it takes the rwsem with a trylock and never blocks: if the writer has gone
it skips the latch and only invalidates the notified range. The writer's
read-side section stays free of server round-trips because under fc->dlm
the partial-write RMW read is skipped.
Holding the rwsem across the dirtying means the writeback branch of
fuse_cache_write_iter() can no longer delegate to
generic_file_write_iter(), which takes and drops the inode lock itself.
Open-code it -- inode_lock(), generic_write_checks(),
__generic_file_write_iter(), inode_unlock(), generic_write_sync() -- so
the read side spans exactly the page-cache dirtying.
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
commit 0c58a97 ("fuse: remove tmp folio for writebacks and internal rb tree") removed temp folios for dirty page writeback. Consequently, fuse can now use the default writeback accounting. With switching fuse to use default writeback accounting, there are some added benefits. This updates wb->writeback_inodes tracking as well now and updates writeback throughput estimates after writeback completion. This commit also removes inc_wb_stat() and dec_wb_stat(). These have no callers anymore now that fuse does not call them. Signed-off-by: Joanne Koong <joannelkoong@gmail.com> Reviewed-by: David Hildenbrand <david@redhat.com> Reviewed-by: Bernd Schubert <bschubert@ddn.com> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> (cherry picked from commit 494d2f5) Ported to the page-based writeback path this tree still carries: the manual accounting sits in fuse_writepage_finish_stat() and in both fuse_writepage_args_page_fill() and fuse_writepages_fill(), and each of those pairs 1:1 with the folio_start_writeback()/end_page_writeback() the generic accounting hooks, so dropping them is equivalent. The include/linux/backing-dev.h hunk removing the now-callerless inc_wb_stat()/dec_wb_stat() helpers is left out: only fuse ever used them, and fuse ships as a module here, so the header change would buy nothing but a full-tree rebuild. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Extending FOPEN_PARALLEL_DIRECT_WRITES writes were forced onto the
exclusive inode lock, re-serializing the parallel phase. The exclusive
lock only bundled "write + advance i_size + undo-on-failure" into one
unit. But i_size is committed by fuse_write_update_attr() under
fi->lock, only on a successful growing write and independent of the
inode rwsem -- so shared-lock writers commit size correctly and have
nothing to undo. Drop the past-EOF exclusive triggers and gate the
whole-file fuse_do_truncate() rollback on holding the exclusive lock.
Lock mode is passed to __fuse_direct_IO(); i_size is committed at the
same point in every path, only the failure rollback differs:
non-exclusive (relaxed, parallel):
fuse_direct_write_iter
fuse_dio_lock -> inode_lock_shared (exclusive=false)
__fuse_direct_IO(.., false)
fuse_direct_io() write to server
fuse_write_update_attr() commit i_size (on success)
no rollback
exclusive (append / caching / !parallel):
fuse_direct_write_iter
fuse_dio_lock -> inode_lock (exclusive=true)
__fuse_direct_IO(.., true)
fuse_direct_io() write to server
fuse_write_update_attr() commit i_size (on success)
ret<0 & extend -> fuse_do_truncate() rollback
exclusive (caching-mode O_DIRECT):
fuse_cache_write_iter -> inode_lock (exclusive)
generic_file_direct_write -> fuse_direct_IO
__fuse_direct_IO(.., true)
fuse_direct_io() write to server
fuse_write_update_attr() commit i_size (on success)
ret<0 & extend -> fuse_do_truncate() rollback
[hbi: adapted -- kept this tree's local `&& iocb->ki_flags & IOCB_DIRECT`
guards on the async routing in fuse_direct_{read,write}_iter, which the
upstream branch does not have.]
Signed-off-by: Bernd Schubert <bschubert@ddn.com>
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_dio_lock() takes an uncached_io reference (via fuse_inode_uncached_io_start()) only when FUSE_I_FORCE_DIO is clear, while fuse_dio_unlock() decided whether to drop it by re-reading FUSE_I_FORCE_DIO. On this tree the latch is toggled asynchronously by the inode-invalidation notify-storm path, so the bit can differ between the lock and the unlock of a single direct write: - clear at lock (reference taken), set before unlock: the reference is never dropped, leaving fi->iocachectr permanently negative and hanging the next caching-mode open; - set at lock (no reference), cleared before unlock: fuse_inode_uncached_io_end() is called without a matching start, tripping WARN_ON(fi->iocachectr >= 0) and corrupting the counter. Record in fuse_dio_lock() whether a reference was actually taken and have fuse_dio_unlock() drop it based on that captured decision instead of re-testing the racy bit, so the accounting stays balanced regardless of any FORCE_DIO transition mid-write. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
On the DLM-serialised writeback path the DLM write lock already excludes writers cluster-wide, so disjoint local writers (MPI-IO / IOR) may take the inode rwsem shared instead of exclusive, mirroring fuse_dio_wr_exclusive_lock() on the direct path. O_APPEND, O_DIRECT and the killpriv writethrough fallback keep the exclusive lock. i_size is then no longer protected by an exclusive inode lock in that mode. Commit the EOF extension in fuse_write_end() under fi->lock -- the same way fuse_write_update_attr() does on the direct path -- instead of by its previous unlocked read-modify-write, which two concurrent extenders could lose an update to. The upstream branch this comes from writes through iomap, where the generic code owns the i_size update, and therefore has to claim the whole extension up front under fi->lock so iomap never touches i_size itself. Here ->write_end() is fuse's own, so the update can simply be made atomic where it already happens. Growing i_size just behind the write cursor rather than claiming the extension up front also keeps fuse_write_begin()'s beyond-EOF optimisation effective: folios wholly past EOF are zeroed locally instead of sending the server a read-modify-write READ for data that does not exist. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Acquire the dlm lock from fuse server for the normal buffer read path
to ensure the distributed page cache across different nodes can be
co-existing and consistency. More importantly, this change will correct
the DLM lock and folio locks ordering for the buffer read path, thus
can avoid the potential deadlock between the buffer read and page cache
invalidation processes.
[hbi: adapted -- the fuse_dlm_cache.{c,h} rename to fuse_get_dlm_lock()
applies unmodified, but fuse_cache_write_iter() has a different shape
in this tree (a `goto writethrough` fallback rather than a single
writeback flag), so its call site was adjusted by hand.]
Signed-off-by Hai Zhong Zhou <hazhou@ddn.com>
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A FUSE_NOTIFY_INVAL_INODE is a coherency event: once the server signals a remote modify, no local read may return a page it has superseded. The invalidate ran unserialized against cache-serving reads, so a buffered read could hand back a stale folio it still held a reference to. Convert the per-inode wb_inval_rwsem to a percpu_rw_semaphore and take its read side around the cache-serving read as well as the existing buffered write. The read side is per-CPU, so it scales on a shared file; the NOTIFY takes the write side blocking, giving the invalidate priority -- it parks new readers, drains in-flight ones, then drops the cache. Every gated invalidate now runs under the write side, not just the storm-latching one. The gate is allocated only for writeback+dlm regular files and is NULL elsewhere (best-effort invalidate, as before). The blocking write side may run on the notify-delivering server thread, so it is safe only under a server that services request replies on other threads; redfs' dlm server provides that contract. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
The buffered read path now acquires a DLM read lock via fuse_get_dlm_lock(..., FUSE_PAGE_LOCK_READ). Before sending the request to the server, fuse_get_dlm_lock() calls fuse_dlm_range_is_locked() to skip regions we already hold. That coverage check compared the held lock mode for exact equality (range->mode != lock_mode), so a range we already hold with an exclusive WRITE lock was reported as not-locked for a READ request. Because fuse_dlm_lock_range() intentionally does not downgrade a WRITE lock on a read, the region stays WRITE-locked and every subsequent read re-requests a DLM read lock from the server. This made read-after-write and re-read workloads flood the server with redundant FUSE_DLM_WB_LOCK requests, never converging. A held WRITE lock (exclusive) subsumes a READ lock. Treat a range as uncovered only when the held mode is strictly weaker than the requested mode (range->mode < lock_mode). READ requests are now satisfied by either a READ or a WRITE lock, while WRITE requests still require an existing WRITE lock (upgrade otherwise), matching the compatibility rules already documented in fuse_dlm_lock_range(). Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
A partial-folio buffered write reads the folio back from the server first. On the DLM writeback path that read is often pointless: a shared-lock writer commits i_size just behind its own cursor (see fuse_write_end()), so folios can sit inside i_size in ranges no writer has reached yet, and every one of them costs a FUSE_READ for a range that cannot hold data. Against a file opened write-only the server may even fail that read outright. Track fi->server_size, an upper bound on how far the server holds data: seeded from server-reported attributes, advanced when the server acknowledges data (writeback completion and fuse_write_update_attr()), and lowered on truncate. In fuse_write_begin(), when the folio starts at or past that bound and the page-granular DLM write lock covers it, zero-fill the folio locally and skip the READ. The three conditions together are what makes this safe: - fi->server_size bounds data materialized on the server; it is advanced in fuse_writepage_end() before the pages leave writeback, i.e. before they can go clean and be reclaimed, so a reclaimed range the server holds data in can never be zero-filled; - local data not yet acknowledged sits in uptodate folios, which fuse_write_begin() has already returned before reaching here; - the DLM write lock excludes data written by other nodes, and it is re-checked against the live lock tree so a revoked lock falls back to reading. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
In fuse_cache_write_iter, writeback_cache mode was always refreshing STATX_SIZE along with STATX_MODE before a buffered write. The size refresh is only needed for the O_APPEND path, where the kernel must know the current EOF before extending the file. For ordinary writes, fetching size is unnecessary work and can race with concurrent writes and then impact writeback performance. Keep refreshing STATX_MODE in all cases so SUID clearing still sees an up-to-date mode. Request STATX_SIZE only when the file is opened with O_APPEND. Signed-off-by Hai Zhong Zhou <hazhou@ddn.com>
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_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>
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 which fuse_cache_wr_dlm_lock() reports as 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. The relaxed lock has no other justification -- the DLM is what excludes writers on disjoint ranges cluster-wide -- so without it concurrent buffered writers run with no serialisation at all. Re-evaluate the lock mode after the request, while no lock is held yet, so a server without DLM support gets the exclusive path back. Upstream this was found 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. That specific failure was an iomap effect (a shared-lock write claims the i_size extension up front, which suppresses iomap's beyond-EOF zeroing while its fc->dlm-gated replacement also stops running, so an expanding write sends a READ past EOF). It cannot occur here -- fuse_write_end() grows i_size behind the cursor, so fuse_write_begin() keeps zeroing wholly-past-EOF folios itself -- but the unsound lock mode it exposed is the same, and is what this fixes. Signed-off-by: Allison Henderson <allison.henderson@ddn.com> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_write_inode() -> fuse_flush_times() pushes out the mtime/ctime that the kernel owns locally while the writeback cache is on. On the wire that request is indistinguishable from a userspace "touch -m": both arrive as SETATTR with FATTR_MTIME | FATTR_CTIME | FATTR_FH, because trust_local_cmtime makes iattr_to_fattr() send CTIME whenever the writeback cache is enabled. A server that wants to handle a cache flush differently from an explicit attribute change - skipping a cluster-wide lock, merging rather than overwriting - has no way to tell them apart. Add FATTR_WRITEBACK, a control bit in fuse_setattr_in.valid alongside the existing non-attribute bits FATTR_FH, FATTR_LOCKOWNER and FATTR_KILL_SUIDGID. It selects no attribute, it only states that the request originates from writeback. Bit 30 is used rather than the next free one. libfuse mirrors the wire bits into its own FUSE_SET_ATTR_* namespace, where bits 12 to 17 are already taken by library-internal flags, and it masks incoming requests against that namespace; a bit picked from the low end would collide there and need translating on the way in. Bit 30 is free on both sides and clear of the sign bit of the int that the libfuse setattr operation takes, so one value works end to end. The bit is negotiated at INIT time with FUSE_SETATTR_WRITEBACK and is only set on a connection whose server asked for it, so servers that do not know the bit never receive it. Only ->write_inode() is marked. The other kernel-initiated SETATTR, fuse_do_truncate() rolling back a failed extending direct-IO write, is deliberately left unmarked: it is a size correction rather than an attribute writeback, and conflating the two would make the flag ambiguous. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_get_cache_mask() returned STATX_MTIME|CTIME|SIZE whenever writeback_cache was enabled, causing the kernel to trust its locally cached mtime/ctime/size over whatever the server returned in a GETATTR reply. This is unsafe under DLM: another node can hold a PW lock on the inode and modify its size/mtime independently, and the local writeback_cache values have no way of reflecting that. Under the DLM protocol, however, this override is unnecessary in the first place: a GETATTR always acquires a PR sattr lock, which forces every node holding a conflicting PW lock -- including the local node, for its own buffered writes -- to flush dirty pages before the server renders the reply. So under DLM the server's answer is always at least as fresh as anything cached locally, for all three attributes, not just size. Make fuse_get_cache_mask() return 0 whenever fc->dlm is set, when writeback_cache is enabled, so the kernel always trusts the server's attr/size reply in that case. The STATX_MTIME|CTIME|SIZE local-cache override remains only as a fallback for servers without DLM support, where no such flush-before-grant guarantee exists. Signed-off-by Hai Zhong Zhou <hazhou@ddn.com>
fuse_reverse_inval_inode() invalidates with invalidate_inode_pages2_range(),
which waits out folios under writeback and launders dirty ones. Both need a
FUSE_WRITE reply, and while fi->writectr < 0 none can arrive:
fuse_flush_writepages() parks the request on fi->queued_writes until
fuse_release_nowrite().
A truncate holds that freeze across its whole SETATTR, and the server revokes
the truncated range from inside the SETATTR handler, so the notify blocks the
very thread that owes the reply lifting the freeze. generic/014 deadlocks
within seconds, in folio_wait_writeback() under fuse_launder_folio() under
fuse_reverse_inval_inode().
fuse_do_setattr() already states the rule ("Only call
invalidate_inode_pages2() after removing FUSE_NOWRITE, otherwise
fuse_launder_folio() would deadlock"). Give the notify path the same: while
frozen, use invalidate_mapping_pages(), which skips dirty and under-writeback
folios instead of waiting on them. The stale clean folios still go, the DLM
grant is revoked either way, and the freezes that span a request drop the
cache themselves when they finish.
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_cache_write_iter() holds wb_inval_rwsem for read across __generic_file_write_iter(), which starts with file_remove_privs(). Without handle_killpriv[_v2] that asks the server (GETATTR, then SETATTR), and a server that invalidates the inode from inside such a handler blocks in percpu_down_write() draining the gate reader that is waiting for its reply. generic/193 hangs there. The gate only has to fence the page-cache dirtying, so run the privilege kill and the timestamp update before entering it, in both branches. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
fuse_get_cache_mask() returns 0 once the connection has DLM, so every GETATTR reply overwrites i_size, mtime and ctime, and truncate_pagecache() then drops the tail the client still holds dirty. The only way a server can make that answer true is to revoke the client from inside the handler, which deadlocks against the coherency gate. A write grant already guarantees that no other node can touch the range, so keep the cached values while one is held: the size when the server reports less than i_size and [srv_size, i_size) is fully granted, mtime and ctime while the cache under the grant is still dirty. A remote truncate has to revoke first, so the smaller size that follows is applied as usual. The attribute-driven invalidation now keys off STATX_SIZE instead of the whole mask, so a reply that does shrink i_size still truncates the page cache when only the timestamps were served from the cache. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Without handle_killpriv[_v2], fuse_setattr() kills the bits by asking the server, and fuse_do_setattr() freezes writepages around that SETATTR. fuse_set_nowrite() asserts BUG_ON(fi->writectr < 0) under fi->lock, which assumes the caller holds i_rwsem exclusive: with the writeback cache and DLM, buffered writes hold it only shared. Two writers to a suid file can then both pass dentry_needs_remove_privs() before either has cleared the bits, and the second one hits the assert. It oopses inside spin_lock(&fi->lock), so fi->lock stays held and the i_rwsem read count leaks: the inode wedges and the box follows. The race window is a full GETATTR plus SETATTR, so it is not narrow, and an unprivileged user can set the bit on a file it owns. Send those writes down the writethrough branch, which takes i_rwsem exclusive, the way handle_killpriv_v2 writes already go. Scoped to DLM connections, since every other configuration already holds i_rwsem exclusive for a buffered write, and only writes that still find the bits set pay for it. Note that the writethrough branch takes no DLM lock, so those writes leave clean folios in the page cache without a grant covering them. That gap already exists for handle_killpriv_v2 and is not addressed here. Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
hbirth
force-pushed
the
redfs-rhel10_0
branch
2 times, most recently
from
August 14, 2026 13:54
720177f to
db0bd6b
Compare
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
5 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.