A failed mount tears down with scoutfs_put_super(), which calls
scoutfs_srch_destroy() first. srch_destroy() does cancel_work_sync() on
the srch compact worker, but that worker can be parked in an
uninterruptible scoutfs_net_sync_request() to a server that will never
respond (e.g. the server is stuck in recovery). Nothing completes the
request: the forced-unmount drain in the net shutdown path only runs for
umount -f, and a failed mount never calls ->umount_begin, so the request
sits on the resend queue and cancel_work_sync() waits forever.
The result is an unkillable D-state mount that survives the SIGKILL a
mount timeout sends, and only clears on reboot:
systemd[1]: data-archive.mount: Killing process 15740 (mount) with signal SIGKILL.
systemd[1]: data-archive.mount: Mount process still around after SIGKILL. Ignoring.
cat /proc/26717/stack
[<0>] __flush_work+0x16f/0x240
[<0>] __cancel_work_sync+0x135/0x1a0
[<0>] scoutfs_srch_destroy+0x33/0x70 [scoutfs]
[<0>] scoutfs_put_super+0x4f/0x1a0 [scoutfs]
[<0>] scoutfs_fill_super+0x260/0x520 [scoutfs]
[<0>] mount_bdev+0xf9/0x150
[<0>] do_new_mount+0x17a/0x310
[<0>] __x64_sys_mount+0x107/0x140
The worker it waits on, blocked in the sync request that never returns:
task:kworker/u269:1 state:D
Workqueue: scoutfs_srch_compact scoutfs_srch_compact_worker [scoutfs]
Call Trace:
__wait_for_common+0x90/0x1d0
scoutfs_net_sync_request+0xdb/0xf0 [scoutfs]
scoutfs_client_srch_get_compact+0x2e/0x40 [scoutfs]
scoutfs_srch_compact_worker+0x64/0x3d0 [scoutfs]
On the fill_super error path, mark forced_unmount and shut the client
connection down before teardown. That drains pending requests with
-ECONNABORTED so the worker returns and srch_destroy()'s cancel_work_sync
completes. It is done for any failure, before the direct put_super call
and before returning to generic_shutdown_super (which calls put_super
when s_root was set), so both teardown paths are covered. sbi is
allocated before any goto out, and scoutfs_client_net_shutdown() is a
no-op when the client or connection was never set up, so early failures
are safe.
Signed-off-by: Auke Kok <auke.kok@versity.com>
A node only needs to be fenced once, but scoutfs_fence_start() can be
called for the same rid more than once. When a new leader starts it
fences the previous leader as it removes it from the quorum
(quorum_block_leader), and that same rid can also be a mounted client
that then fails to recover within the timeout (client_recovery).
The second fence call collides on that name, sysfs returns -EEXIST,
and the error is propagated to fence_pending_recov_worker() which
treats any error as fatal and shuts the server down. On the next
mount a new leader hits the same stale set and the same collision,
so the filesystem can never finish recovery.
Jun 15 09:22:35 kernel: scoutfs f.000000.r.222222: fencing previous leader f.000000.r.111111 at term 183942 in slot 3 with address x.x.x.x:6000
Jun 15 09:22:36 scoutfs-fenced[9194]: [2026-06-15 09:22:36.588037842] server f.000000.r.222222 fencing rid 1111111111111111 at IP x.x.x.x for quorum_block_leader
Jun 15 09:23:09 kernel: scoutfs f.000000.r.222222 error: 30000 ms recovery timeout expired for client rid 1111111111111111, fencing
Jun 15 09:23:09 kernel: sysfs: cannot create duplicate filename '/fs/scoutfs/f.000000.r.222222/fence/1111111111111111'
Jun 15 09:23:09 kernel: scoutfs f.000000.r.222222 error: fence returned err -17, shutting down server
The reclaim path keys off the rid, not the reason, so one pending fence
covers both needs. Skip submitting a duplicate fence for an rid that
already has one pending, and treat a -EEXIST from the sysfs create as
success to also cover a race or a stale rid-named dir.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Add trace events to aid debugging recovery stalls:
- scoutfs_recov_prepare(sb, rid, which):
Server registered that a client needs to recover something.
- scoutfs_recov_finish(sb, rid, which, remaining):
Client finished one recovery item, and lists remaining count.
- scoutfs_recov_timeout_fire(sb, nr_pending):
The 30s recovery window expired, with count of clients that
failed to recover within the timeout.
- scoutfs_recov_fence_rid(sb, rid):
One event per client about to be fenced.
- scoutfs_lock_server_greeting(sb, rid, recov_pending):
Shows which rid we expect to recover locks.
Signed-off-by: Auke Kok <auke.kok@versity.com>
The trace events added in the subsequent change take recinf->lock
while in timer_callback() to iterate recinf->pending, but it does
so in softirq context. To do this safely, it has to use the _bh()
spin_lock variant. Convert the other lock takers, too.
```
================================
WARNING: inconsistent lock state
5.14.0-427.35.1.el9_4.x86_64+debug #1 Tainted: G OE ------- ---
--------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/2/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
ffff88813cdd9c20 (&recinf->lock){+.?.}-{2:2}, at: timer_callback+0x26/0x380 [scoutfs]
{SOFTIRQ-ON-W} state was registered at:
__lock_acquire+0x7d0/0x1900
lock_acquire+0x1da/0x640
_raw_spin_lock+0x34/0x80
scoutfs_recov_finish+0x80/0x830 [scoutfs]
server_greeting+0x244/0xe60 [scoutfs]
scoutfs_net_proc_worker+0x28a/0xce0 [scoutfs]
recv_one_message+0x7e3/0xd10 [scoutfs]
scoutfs_net_recv_worker+0x441/0xe00 [scoutfs]
process_one_work+0x8e5/0x1530
worker_thread+0x598/0xf70
kthread+0x2a4/0x350
ret_from_fork+0x29/0x50
irq event stamp: 549813370
hardirqs last enabled at (549813370): [<ffffffffabe25cb4>] _raw_spin_unlock_irq+0x24/0x50
hardirqs last disabled at (549813369): [<ffffffffabe2594e>] _raw_spin_lock_irq+0x5e/0x90
softirqs last enabled at (549813356): [<ffffffffabe28c91>] __do_softirq+0x621/0x9c2
softirqs last disabled at (549813363): [<ffffffffa9a44665>] __irq_exit_rcu+0x185/0x230
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&recinf->lock);
<Interrupt>
lock(&recinf->lock);
*** DEADLOCK ***
```
Signed-off-by: Auke Kok <auke.kok@versity.com>
Scoutfs has supported posix ACLs through the xattr handler table,
which allowed NFS to fetch them through this sideband, which worked
for older kernels.
With recent changes we've pulled in .get_acl because the mainline
kernel is changing how ACL ops are called. But we still left .set_acl
unreachable. This meant that on el9.7 nfs clients could now reach
.get_acl, but still not set them.
With this change, we're finally exposing .set_acl consistently
across all el releases and allowing nfs clients to both get and set
posix ACLs.
Signed-off-by: Auke Kok <auke.kok@versity.com>
El9.8 backported the upstream v6.15.* rename of from_timer to
timer_container_of. Switch the two callers in fence.c and recov.c
to the new style and add a simple kcompat define for older kernels.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Ramps up data preallocation based on the number of online
blocks. This results in a simple 2<<n block allocation pattern
until n=11 (2048) - the default value of data_prealloc_blocks.
Signed-off-by: Auke Kok <auke.kok@versity.com>
When scoutfs_setattr truncates a file with offline extents, it unlocks
the inode lock before calling scoutfs_data_wait to wait for the data
to be staged. If data_wait returns any error, the code jumps to 'goto
out' which calls scoutfs_unlock again, thus double-unlocking the lock.
Signed-off-by: Auke Kok <auke.kok@versity.com>
A quota check holds the quota cluster lock for READ and marks the
cached ruleset EBUSY while loading rules. A quota mod on the same
mount holds the lock for WRITE (compatible with the local READ)
and calls scoutfs_quota_invalidate(), tripping
BUG_ON(rs == ERR_PTR(-EBUSY)).
Make invalidate skip EBUSY so the reader's claim is preserved, and
have scoutfs_quota_mod_rule wait for the reader to finish before
calling invalidate. Without the wait, the in-flight reader would
publish its stale ruleset after invalidate runs, leaving the cache
stale until the next invalidation.
Signed-off-by: Auke Kok <auke.kok@versity.com>
When searching for the next least merge range we need to sweep all the
stored items because they're interleaved with respect to key sorting
because we've clobbered the zone.
To search all of them we need to start from 0, not from the caller's
start key after setting the zone. If the caller happens to provide a
start key with a small zone but large other fields (totl keys with
sufficiently large identifiers) we can miss ranges.
Signed-off-by: Zach Brown <zab@zabbo.net>
I'm seeing consistent CPU soft lockups in block_free_work on
my bare metal system that aren't reached by VM instances. The
reason is that the bare metal machine has a ton more memory
available causing the block free work queue to grow much
larger in size, and then it has so much work that it can take 30+
seconds before it goes through it all.
This is all with a debug kernel. A non debug kernel will likely
zoom through the outstanding work here at a much faster rate.
Signed-off-by: Auke Kok <auke.kok@versity.com>
block_submit_bio will return -ENOLINK if called during a forced
shutdown, the bio is never submitted, and thus no completion callback
will fire to set BLOCK_BIT_ERROR. Any other task waiting for this
specific bp will end up waiting forever.
To fix, fall through to the existing block_end_io call on the
error path instead of returning directly. That means moving
the forcing_unmount check past the setup calls so block_end_io's
bookkeeping stays balanced. block_end_io then sets BLOCK_BIT_ERROR
and wakes up waiters just as it would on a failed async completion.
Signed-off-by: Auke Kok <auke.kok@versity.com>
scoutfs_quota_mod_rule calls scoutfs_item_create/delete which use
the transaction allocator but it never held it. Without the hold,
a concurrent transaction commit can call scoutfs_alloc_init to
reinitialize the allocator while dirty_alloc_blocks is in the middle
of setting up the freed list block. This overwrites alloc->freed with
the server's fresh (empty) state, causing a blkno mismatch BUG_ON
in list_block_add.
Reproduced by stressing concurrent quota add/del operations across
mounts. Crashdump analysis confirms dirty_list_block COW'd a freed
block (fr_old=9842, new blkno=9852) but by the time list_block_add
ran, freed.ref.blkno was 0 with first_nr=0 and total_nr=0: the freed
list head had been zeroed by a concurrent alloc_init.
Fix by adding scoutfs_hold_trans/scoutfs_release_trans around the
item modification in scoutfs_quota_mod_rule, preventing transaction
commit from racing with the allocator use.
Rename the 'unlock' label to 'release' since 'out' now directly
does the unlock. The unlock safely handles a NULL lock.
Signed-off-by: Auke Kok <auke.kok@versity.com>
A malformed message encountered here increases the counter, but doesn't
tear down the connection because of the nested for loops. The comments
indicate that that is the expected behavior - a misbehaving client
should not be tolerated.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Clang's scan-build found this leak when we get an invalidation
for a lock we no longer have. Free ireq to fix.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Clang flow analysis flags resp_data in process_response as possibly
uninitialized when find_request returns NULL.
kmod/src/net.c:533:6: error: variable 'resp_data' is used uninitialized
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
In practice the read is harmless because resp_func stays NULL in that
path and call_resp_func only dereferences resp_data when resp_func is
non-NULL. Initialize at declaration.
Signed-off-by: Auke Kok <auke.kok@versity.com>
The gcc version in el7 can't determine that scoutfs_block_check_stale
won't return ret = 0 when the input ret value is < 0, and
errors because we might call alloc_wpage with an uninitialized
read_seq. Initialize it to 0 to avoid it.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Readers currently accumulate all finalized log tree deltas into
a single bucket for deciding whether they are already in fs_root
or not, but, finalized trees that aren't inputs to a current merge
will have higher seqs, and thus we may be double applying deltas
already merged into fs_root.
To distinguish, scoutfs_totl_merge_contribute() needs to know the
merge status item seq. We change wkic's get_roots() from using the
SCOUTFS_NET_CMD_GET_ROOTS RPC to reading the superblock directly.
This is needed because totl merge resolution has to use the same data
as the btree roots it is operating on, thus we can't grab it from a
SCOUTFS_NET_CMD_GET_ROOTS packet - it likely is different.
Signed-off-by: Auke Kok <auke.kok@versity.com>
These mislabeled members and enums were clearly not describing
the actual data being handled and obfuscating the intent of
avoiding mixing merge input items with non-merge input items.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Before deltas were added this code path was correct, but with
deltas we can't just retry this without clearing &root, since
it would potentially double count.
The condition where this could happen is when there are deltas in
several finalized log trees, and we've made progress towards reading
some of them, and then encounter a stale btree block. The retry
would not clear the collected trees, apply the same delta as was
already applied before the retry, and thus double count.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Two different clients can write delta's for totl indexes at the same
time, recording their changes. When merged, a reader should apply both
in order, and only once. To do so, the seq determines whether the delta
has been applied already.
The code fails to update the seq while walking the trees for deltas to
apply. Subsequently, when processing subsequent trees, it could
re-process deltas already applied. In case of a large negative delta
(e.g. removal of large amounts of files), the totl value could become
negative, resulting in quota lockout.
The fix is simple: advance the seq when reading partial delta merges
to avoid double counting.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Add a trigger that forces btree_merge() to return -ERANGE after
modifying a leaf's worth of items, causing many small partial merges
per merge cycle. This is used by tests to reliably reproduce races
that depend on partial merges splicing items into fs_root while
finalized logs still exist.
The trigger check lives inside btree_merge() where it can observe
actual item modification progress, rather than overriding the
caller's dirty byte limit argument which applies to the whole
writer context.
Signed-off-by: Auke Kok <auke.kok@versity.com>
During inode deletion, scoutfs_xattr_drop forgot to set the xid
of the xattr after calling parse_indx_key, which hardcodes xid=0, and it
is the callers' responsibility. delete_force then deletes the wrong
key, and returns no errors on nonexistant keys.
So now there is a pending deletion for a non-existant indx and an
orphan indx entry in the tree. Subsequent calls to `scoutfs
read-xattr-index` will thus return entries for deleted inodes.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Add a reclaim_skip_finalize trigger that prevents reclaim from
setting FINALIZED on log_trees entries. The test arms this trigger,
force-unmounts a client to create an orphan, and verifies the log
merge succeeds without timeout and the orphan reclaim message
appears in dmesg.
Signed-off-by: Auke Kok <auke.kok@versity.com>
An unfinalized log_trees entry whose rid is not in mounted_clients
is an orphan left behind by incomplete reclaim. Previously this
permanently blocked log merges because the finalize loop treated it
as an active client that would never commit.
Call reclaim_open_log_tree for orphaned rids before starting a log
merge. Once reclaimed, the existing merge and freeing paths include
them normally.
Also skip orphans in get_stable_trans_seq so their open transaction
doesn't artificially lower the stable sequence.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Add an archive layer ioctl for converting offline extents into sparse
extents without relying on or modifying data_version. This is helpful
when working with files with very large sparse regions.
Signed-off-by: Zach Brown <zab@versity.com>
Signed-off-by: Auke Kok <auke.kok@versity.com>
The initialization here avoids clearing __pad[], which leaks
to disk. Use a struct initializer to avoid it.
Signed-off-by: Auke Kok <auke.kok@versity.com>
This allocation here currently leaks through __pad[7] which
is written to disk. Use the initializer to enforce zeroing
the pad. The name member is written right after.
Signed-off-by: Auke Kok <auke.kok@versity.com>
The caller sends the return value of this inline as u8. If we return
-EINVAL, it maps to (234) which is outside of our enum range. Assume
this was meant to return SCOUTFS_NET_ERR_EINVAL which is a defined
constant.
Signed-off-by: Auke Kok <auke.kok@versity.com>
These boolean checks are all mutually exclusive, meaning this
check will always succeed due to the negative. Instead of && it
needs to use ||.
Signed-off-by: Auke Kok <auke.kok@versity.com>
The exact 2 lines here are repeated. It suggests that there may
have been the intent of an additional check, but, there isn't
anything left from what I can see that needs checking here.
Signed-off-by: Auke Kok <auke.kok@versity.com>
This setup function always returned 0, even on error, causing
initialization to continue despite the error.
Signed-off-by: Auke Kok <auke.kok@versity.com>
In el9.6, the kernel VFS no longer goes through xattr handlers to
retreive ACLs, but instead calls the FS drivers' .get_{inode_}acl
method. In the initial compat version we hooked up to .get_acl given
the identical name that was used in the past.
However, this results in caching issues, as was encountered by customers
and exposed in the added test case `basic-acl-consistency`. The result
is that some group ACL entries may appear randomly missing. Dropping
caches may temporarily fix the issue.
The root cause of the issue is that the VFS now has 2 separate paths to
retreive ACL's from the FS driver, and, they have conflicting
implications for caching. `.get_acl` is purely meant for filesystems
like overlay/ecryptfs where no caching should ever go on as they are
fully passthrough only. Filesystems with dentries (i.e. all normal
filesystems should not expose this interface, and instead expose the
.get_inode_acl method. And indeed, in introducing the new interface, the
upstream kernel converts all but a few fs's to use .get_inode_acl().
The functional change in the driver is to detach KC_GET_ACL_DENTRY and
introduce KC_GET_INODE_ACL to handle the new (and required) interface.
KC_SET_ACL_DENTRY is detached due to it being a different changeset in
the kernel and we should separate these for good measure now.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Errors from lock server calls typically shut the server down.
During normal unmount a client's locks are reclaimed before the
connection is disconnected. The lock server won't try to send to
unmounting clients.
Clients whose connections time out can cause ENOTCONN errors. Their
connection is freed before they're fenced and their locks are reclaimed.
The server can try to send to the client for a lock that's disconnected
and get a send error.
These errors shouldn't shut down the server. The client is either going
to be fenced and have the locks reclaimed, ensuring forward progress, or
the server is going to shutdown if it can't fence.
This was seen in testing as multiple clients were timed out.
Signed-off-by: Zach Brown <zab@versity.com>
Lock invalidation has assertions for critical errors, but it doesn't
allow the synthetic errors that come from forced unmount severing the
client's connection to the world.
Signed-off-by: Zach Brown <zab@versity.com>
The net layer was initially built around send queue lists with the
presumption that there wouldn't be many messages in flight and that
responses would be sent roughly in order.
In the modern era, we can have 10s of thousands of lock request messages
in flight. This lead to o(n^2) processing in quite a few places as recv
processing searched for either requests to complete or responses to
free.
This adds messages to two rbtrees, indexing either requests by their id
or responses by their send sequence. Recv processing can find messages
in o(log n). This patch intends to be minimally disruptive. It's only
replacing the search of the send and resend queues in the recv path with
rbtrees. Other uses of the two queue lists are untouched.
On a single node, with ~40k lock shrink attempts in flight, we go from
processing ~800 total request/grant request/response pairs per second to
~60,000 per second.
Signed-off-by: Zach Brown <zab@versity.com>
The use of the VM shrinker was a bad fit for locks. Shrinking a lock
requires a round trip with the server to request a null mode. The VM
treats the locks like a cache, as expected, which leads to huge amounts
of locks accumulating and then being shrank in bulk. This creates a
huge backlog of locks making their way through the network conversation
with the server that implements invalidating to a null mode and freeing.
It starves other network and lock processing, possibly for minutes.
This removes the VM shrinker and instead introduces an option that sets
a limit on the number of idle locks. As the number of locks exceeds the
count we only try to free an oldest lock at each lock call. This
results in a lock freeing pace that is proportional to the allocation of
new locks by callers and so is throttled by the work done while callers
hold locks. It avoids the bulk shrinking of 10s of thousands of locks
that we see in the field.
Signed-off-by: Zach Brown <zab@versity.com>
Due to an iput race, the "unlink wait for open on other mount"
subtest can fail. If the unlink happens inline, then the test
passes. But if the orphan scanner has to complete the unlink
work, it's possible that there won't be enough log merge work
for the scanner to do the cleanup before we look at the seq index.
Add SCOUTFS_TRIGGER_LOG_MERGE_FORCE_FINALIZE_OURS, to allow
forcing a log merge. Add new counters, log_merges_start and
log_merge_complete, so that tests can see that a merge has happened.
Then we have to wait for the orphan scanner to do its work.
Add a new counter, orphan_scan_empty, that increments each time
the scanner walks the entire inode space without finding any
orphans. Once the test sees that counter increment, it should be
safe to check the seq index and see that the unlinked inode is gone.
Signed-off-by: Chris Kirby <ckirby@versity.com>
A few callers of alloc_move_empty in the server were providing a budget
that was too small. Recent changes to extent_mod_blocks increased the
max budget that is necessary to move extents between btrees. The
existing WAG of 100 was too small for trees of height 2 and 3. This
caused looping in production.
We can increase the move budget to half the overall commit budget, which
leaves room for a height of around 7 each. This is much greater than we
see in practice because the size of the per-mount btrees is effectiely
limited by both watermarks and thresholds to commit and drain.
Signed-off-by: Zach Brown <zab@versity.com>
Tests that cause client retries can fail with this error
from server_commit_log_merge():
error -2 committing log merge: getting merge status item
This can happen if the server has already committed and resolved
the log merge that is being retried. We can safely ignore ENOENT here
just like we do a few lines later.
Signed-off-by: Chris Kirby <ckirby@versity.com>
Auke Kok
Zach Brown
Chris Kirby