From 70b80695cb77032f6ddbf17052a900ff40909ddc Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Mon, 15 Jun 2026 10:53:18 -0700 Subject: [PATCH 1/5] Guard log_merge, srch_root, and mounted_client item printers Scoutfs print segfaults walking a log_merge btree that has more than one level. print_btree_block() prints parent (level > 0) items via print_block_ref(), which invokes the item callback with a NULL value to print the key portion before printing the child ref: func(key, 0, 0, NULL, 0, arg); print_log_merge_item immediately casts val and reads a field, dereferencing NULL. A log_merge of a single leaf block (height 1) never hits the parent path; one with height > 1 crashes on the first parent item: scoutfs[22043]: segfault at 8 ip 0000000000408ef0 sp 00007fffc5edd8b0 error 4 #0 0x0000000000408ef0 in print_log_merge_item () #1 0x000000000040958d in print_btree_block.constprop.0.isra () #2 0x000000000040a471 in print_cmd () #3 0x0000000000404264 in cmd_execute () #4 0x00000000004025c9 in main () print_mounted_client_entry has the same bug: it casts val and reads mcv->addr / mcv->flags with no NULL guard, so a mounted_clients btree of height > 1 segfaults the same way. print_srch_root_item guards NULL but casts to scoutfs_srch_compact or scoutfs_srch_file without bounds checking, so a short or malformed item reads past its end. Fix all three: return early when val is NULL (printing just the key for the parent ref where applicable), and bounds-check val_len before each cast so a short item is reported instead of read past its end. Signed-off-by: Auke Kok --- utils/src/print.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/utils/src/print.c b/utils/src/print.c index c17eb425..f3d7ae6a 100644 --- a/utils/src/print.c +++ b/utils/src/print.c @@ -375,6 +375,11 @@ static int print_srch_root_item(struct scoutfs_key *key, u64 seq, u8 flags, void if (val) { if (key->sk_type == SCOUTFS_SRCH_PENDING_TYPE || key->sk_type == SCOUTFS_SRCH_BUSY_TYPE) { + if (val_len < sizeof(*sc)) { + printf(" (short srch compact value: val_len %u)\n", + val_len); + return 0; + } sc = val; printf(" compact %s: nr %u flags 0x%x\n", key->sk_type == SCOUTFS_SRCH_PENDING_TYPE ? @@ -387,6 +392,11 @@ static int print_srch_root_item(struct scoutfs_key *key, u64 seq, u8 flags, void SRF_A(&sc->in[i].sfl)); } } else { + if (val_len < sizeof(*sfl)) { + printf(" (short srch file value: val_len %u)\n", + val_len); + return 0; + } sfl = val; printf(" "SRF_FMT"\n", SRF_A(sfl)); } @@ -398,9 +408,25 @@ static int print_srch_root_item(struct scoutfs_key *key, u64 seq, u8 flags, void static int print_mounted_client_entry(struct scoutfs_key *key, u64 seq, u8 flags, void *val, unsigned val_len, void *arg) { - struct scoutfs_mounted_client_btree_val *mcv = val; + struct scoutfs_mounted_client_btree_val *mcv; struct in_addr in; + /* + * Parent block items reference child blocks and have no value; + * print_block_ref() calls us with a NULL val just to print the key. + */ + if (!val) { + printf(" rid %016llx\n", le64_to_cpu(key->skmc_rid)); + return 0; + } + + if (val_len < sizeof(*mcv)) { + printf(" rid %016llx (short mounted client value: val_len %u)\n", + le64_to_cpu(key->skmc_rid), val_len); + return 0; + } + + mcv = val; memset(&in, 0, sizeof(in)); in.s_addr = htonl(le32_to_cpu(mcv->addr.v4.addr)); @@ -419,8 +445,17 @@ static int print_log_merge_item(struct scoutfs_key *key, u64 seq, u8 flags, void struct scoutfs_log_merge_complete *comp; struct scoutfs_log_merge_freeing *fr; + /* + * Parent block items reference child blocks and have no value; + * print_block_ref() calls us with a NULL val just to print the key. + */ + if (!val) + return 0; + switch (key->sk_zone) { case SCOUTFS_LOG_MERGE_STATUS_ZONE: + if (val_len < sizeof(*stat)) + goto bad_len; stat = val; printf(" status: next_range_key "SK_FMT" nr_req %llu nr_comp %llu seq %llu\n", SK_ARG(&stat->next_range_key), @@ -429,12 +464,16 @@ static int print_log_merge_item(struct scoutfs_key *key, u64 seq, u8 flags, void le64_to_cpu(stat->seq)); break; case SCOUTFS_LOG_MERGE_RANGE_ZONE: + if (val_len < sizeof(*rng)) + goto bad_len; rng = val; printf(" range: start "SK_FMT" end "SK_FMT"\n", SK_ARG(&rng->start), SK_ARG(&rng->end)); break; case SCOUTFS_LOG_MERGE_REQUEST_ZONE: + if (val_len < sizeof(*req)) + goto bad_len; req = val; printf(" request: logs_root "BTROOT_F" logs_root "BTROOT_F" start "SK_FMT " end "SK_FMT" input_seq %llu rid %016llx seq %llu flags 0x%llx\n", @@ -448,6 +487,8 @@ static int print_log_merge_item(struct scoutfs_key *key, u64 seq, u8 flags, void le64_to_cpu(req->flags)); break; case SCOUTFS_LOG_MERGE_COMPLETE_ZONE: + if (val_len < sizeof(*comp)) + goto bad_len; comp = val; printf(" complete: root "BTROOT_F" start "SK_FMT" end "SK_FMT " remain "SK_FMT" rid %016llx seq %llu flags %llx\n", @@ -460,6 +501,8 @@ static int print_log_merge_item(struct scoutfs_key *key, u64 seq, u8 flags, void le64_to_cpu(comp->flags)); break; case SCOUTFS_LOG_MERGE_FREEING_ZONE: + if (val_len < sizeof(*fr)) + goto bad_len; fr = val; printf(" freeing: root "BTROOT_F" key "SK_FMT" seq %llu\n", BTROOT_A(&fr->root), @@ -472,6 +515,11 @@ static int print_log_merge_item(struct scoutfs_key *key, u64 seq, u8 flags, void } return 0; + +bad_len: + printf(" (short log merge value: zone %u val_len %u)\n", + key->sk_zone, val_len); + return 0; } static int print_alloc_item(struct scoutfs_key *key, u64 seq, u8 flags, void *val, From 205cfbdf4ae19a5cf50fc4c790900d3b26da9f44 Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Mon, 15 Jun 2026 11:37:48 -0700 Subject: [PATCH 2/5] Don't shut down the server when fencing a rid twice 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 Check the list for the rid and skip the duplicate before creating sysfs. A pending fence can be freed once it is on the list, so a new fi->mutex serializes creation against the freeing path: the duplicate check, sysfs create, and list insert run as a unit, and a fence becomes visible on the list only once it is fully built. scoutfs_fence_free() and scoutfs_fence_stop() take the same mutex around removing a fence and tearing it down, and scoutfs_fence_destroy() drains through fence_stop() rather than walking the list unlocked. Signed-off-by: Auke Kok --- kmod/src/fence.c | 46 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/kmod/src/fence.c b/kmod/src/fence.c index 60799917..a3579228 100644 --- a/kmod/src/fence.c +++ b/kmod/src/fence.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "super.h" @@ -65,6 +66,7 @@ struct fence_info { struct kobject fence_dir_kobj; struct workqueue_struct *wq; wait_queue_head_t waitq; + struct mutex mutex; spinlock_t lock; struct list_head list; }; @@ -235,8 +237,10 @@ static void fence_timeout(struct timer_list *timer) int scoutfs_fence_start(struct super_block *sb, u64 rid, __be32 ipv4_addr, int reason) { DECLARE_FENCE_INFO(sb, fi); + struct pending_fence *existing; struct pending_fence *fence; - int ret; + bool duplicate = false; + int ret = 0; fence = kzalloc(sizeof(struct pending_fence), GFP_NOFS); if (!fence) { @@ -246,6 +250,7 @@ int scoutfs_fence_start(struct super_block *sb, u64 rid, __be32 ipv4_addr, int r fence->sb = sb; scoutfs_sysfs_init_attrs(sb, &fence->ssa); + timer_setup(&fence->timer, fence_timeout, 0); fence->start_kt = ktime_get(); fence->ipv4_addr = ipv4_addr; @@ -254,22 +259,39 @@ int scoutfs_fence_start(struct super_block *sb, u64 rid, __be32 ipv4_addr, int r fence->reason = reason; fence->rid = rid; + mutex_lock(&fi->mutex); + + spin_lock(&fi->lock); + list_for_each_entry(existing, &fi->list, entry) { + if (existing->rid == rid) { + duplicate = true; + break; + } + } + spin_unlock(&fi->lock); + + if (duplicate) + goto unlock; + ret = scoutfs_sysfs_create_attrs_parent(sb, &fi->kset->kobj, &fence->ssa, fence_attrs, "%016llx", rid); - if (ret < 0) { - kfree(fence); - goto out; - } + if (ret < 0) + goto unlock; - timer_setup(&fence->timer, fence_timeout, 0); fence->timer.expires = jiffies + msecs_to_jiffies(FENCE_TIMEOUT_MS); add_timer(&fence->timer); spin_lock(&fi->lock); list_add_tail(&fence->entry, &fi->list); spin_unlock(&fi->lock); + + fence = NULL; +unlock: + mutex_unlock(&fi->mutex); out: + if (fence) + destroy_fence(fence); return ret; } @@ -324,6 +346,8 @@ int scoutfs_fence_free(struct super_block *sb, u64 rid) struct pending_fence *fence; int ret = -ENOENT; + mutex_lock(&fi->mutex); + spin_lock(&fi->lock); list_for_each_entry(fence, &fi->list, entry) { if (fence->rid == rid) { @@ -339,6 +363,8 @@ int scoutfs_fence_free(struct super_block *sb, u64 rid) wake_up(&fi->waitq); } + mutex_unlock(&fi->mutex); + return ret; } @@ -413,6 +439,7 @@ int scoutfs_fence_setup(struct super_block *sb) } init_waitqueue_head(&fi->waitq); + mutex_init(&fi->mutex); spin_lock_init(&fi->lock); INIT_LIST_HEAD(&fi->list); @@ -446,6 +473,7 @@ void scoutfs_fence_stop(struct super_block *sb) DECLARE_FENCE_INFO(sb, fi); struct pending_fence *fence; + mutex_lock(&fi->mutex); do { spin_lock(&fi->lock); fence = list_first_entry_or_null(&fi->list, struct pending_fence, entry); @@ -458,20 +486,18 @@ void scoutfs_fence_stop(struct super_block *sb) wake_up(&fi->waitq); } } while (fence); + mutex_unlock(&fi->mutex); } void scoutfs_fence_destroy(struct super_block *sb) { struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); struct fence_info *fi = SCOUTFS_SB(sb)->fence_info; - struct pending_fence *fence; - struct pending_fence *tmp; if (fi) { if (fi->wq) destroy_workqueue(fi->wq); - list_for_each_entry_safe(fence, tmp, &fi->list, entry) - destroy_fence(fence); + scoutfs_fence_stop(sb); if (fi->kset) kset_unregister(fi->kset); kfree(fi); From cebe46544353617587bfd8c92beb8844db40bcb6 Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Mon, 15 Jun 2026 11:40:10 -0700 Subject: [PATCH 3/5] Abort pending net requests when a mount fails 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 --- kmod/src/super.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/kmod/src/super.c b/kmod/src/super.c index 3c837160..f2e1420f 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -627,6 +627,19 @@ static int scoutfs_fill_super(struct super_block *sb, void *data, int silent) scoutfs_trans_restart_sync_deadline(sb); ret = 0; out: + if (ret) { + /* + * The mount failed and we're about to tear down, either here + * or via generic_shutdown_super if s_root was set. Any worker + * started during fill_super can be blocked in an uninterruptible + * net request to a server that will never respond. Force the + * client connection down so those pending requests abort with + * -ECONNABORTED before teardown. + */ + SCOUTFS_SB(sb)->forced_unmount = true; + scoutfs_client_net_shutdown(sb); + } + /* on error, generic_shutdown_super calls put_super if s_root */ if (ret && !sb->s_root) scoutfs_put_super(sb); From a87c317931a14ddf47e3a027589d5911f4cebdd6 Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Tue, 16 Jun 2026 21:55:03 -0700 Subject: [PATCH 4/5] Account for the pending freed-head rotation in the commit room gates The server commit deadlocks when its freed allocator list head block fills to near capacity. Both gates that decide whether a transaction has room measure it as free slots in the clean freed head block: - hold_commit() admits a holder only if scoutfs_alloc_meta_remaining() reports enough freed room (2 * COMMIT_HOLD_ALLOC_BUDGET slots). - empty_list()/fill_list()'s list_has_blocks() proceed only if the head has extent_mod_blocks() slots. The clean head is not what a transaction gets: the first dirtying allocation runs dirty_alloc_blocks(), which rotates in a fresh head block when the current head is under EMPTY_FREED_THRESH. A clean, nearly-full head has a full block's worth of room as soon as it's touched. With the gates refusing on the clean full head, no holder is admitted and the drains never start, so the rotation in dirty_alloc_blocks() is never reached. The server spins applying empty commits and the filesystem can't mount or recover (observed at ~11k empty commits/sec; freed head first_nr 8148 of an 8184 capacity). Fix the accounting in scoutfs_alloc_meta_remaining(): when the freed list isn't dirtied yet and the clean head is under EMPTY_FREED_THRESH, report the room the pending rotation will give, SCOUTFS_ALLOC_LIST_MAX_BLOCKS - 2 (a fresh block, less the old avail and freed head blocks the rotation frees into it). list_has_blocks() routes through the same function so fill_list()/empty_list() use identical accounting; otherwise an avail-low, freed-full commit could still wedge because fill_list() couldn't refill avail past the clean full freed head. hold_commit() then admits a holder (or a drain starts) and the first allocation rotates the full head. The avail gate and the meta_low() loop-stop are left conservative, so genuine ENOSPC still fails and freeing loops still commit before overflowing a head. Signed-off-by: Auke Kok --- kmod/src/alloc.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/kmod/src/alloc.c b/kmod/src/alloc.c index 0ceaf3b8..f6a5be0c 100644 --- a/kmod/src/alloc.c +++ b/kmod/src/alloc.c @@ -1150,13 +1150,20 @@ static bool list_has_blocks(struct super_block *sb, struct scoutfs_alloc *alloc, { u32 tree_blocks = extent_mod_blocks(root->root.height) * extents; u32 most = 1 + tree_blocks + addl_blocks; + u32 avail; + u32 freed; - if (le32_to_cpu(alloc->avail.first_nr) < most) { + /* use the same room accounting as the commit hold gate, including the + * pending freed-head rotation, so a clean nearly-full freed head can't + * stop fill_list()/empty_list() from making progress */ + scoutfs_alloc_meta_remaining(alloc, &avail, &freed); + + if (avail < most) { scoutfs_inc_counter(sb, alloc_list_avail_lo); return false; } - if (list_block_space(alloc->freed.first_nr) < most) { + if (freed < most) { scoutfs_inc_counter(sb, alloc_list_freed_hi); return false; } @@ -1384,14 +1391,35 @@ bool scoutfs_alloc_meta_low(struct super_block *sb, return lo; } +/* + * Report the metadata allocator room a transaction will actually have. + * + * If the freed list hasn't been dirtied yet, the first dirtying allocation + * rotates in a fresh head block when the current head is under + * EMPTY_FREED_THRESH (see dirty_alloc_blocks()). A clean but nearly-full + * head then has a fresh block's worth of room as soon as it's touched, so + * report that; otherwise the commit hold gate and the fill/empty drains read + * it as no room and refuse to make progress. The rotation frees the old + * avail and freed head blocks into the fresh block, so the room it leaves is + * MAX - 2. + * + * dirty_freed_bl isn't covered by the seqlock, but it only transitions + * NULL->set on a transaction's first allocation and back to NULL at + * prepare_commit; a stale read predicts the rotation one allocation early or + * late, which still gates correctly. + */ void scoutfs_alloc_meta_remaining(struct scoutfs_alloc *alloc, u32 *avail_total, u32 *freed_space) { unsigned int seq; + u32 fr; do { seq = read_seqbegin(&alloc->seqlock); *avail_total = le32_to_cpu(alloc->avail.first_nr); - *freed_space = list_block_space(alloc->freed.first_nr); + fr = list_block_space(alloc->freed.first_nr); + if (!alloc->dirty_freed_bl && fr < EMPTY_FREED_THRESH) + fr = SCOUTFS_ALLOC_LIST_MAX_BLOCKS - 2; + *freed_space = fr; } while (read_seqretry(&alloc->seqlock, seq)); } From 687f1041e20ad80ead410318819e6fe7d26b7eae Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Wed, 17 Jun 2026 14:27:28 -0700 Subject: [PATCH 5/5] Test case for wedged full meta allocators. Reproduce the freed-list commit deadlock and show a fixed server recovers. The alloc_fill_freed_list trigger stuffs both server_meta_freed heads to near-full in a single commit. It claims runs of free blocks and appends them straight into the head blocks, leaking whatever isn't used. Filling both heads in one commit makes the wedge reproducible. There's no window for the drain to empty one head before the other fills. An unfixed server is left with both heads full and wedges on the next commit. A fixed server drains them and stays live. Signed-off-by: Auke Kok --- kmod/src/alloc.c | 57 +++++++++++++++++++++++++++++++++ kmod/src/alloc.h | 4 +++ kmod/src/counters.h | 1 + kmod/src/server.c | 10 ++++++ kmod/src/triggers.c | 1 + kmod/src/triggers.h | 1 + tests/golden/freed-list-wedge | 8 +++++ tests/sequence | 1 + tests/tests/freed-list-wedge.sh | 39 ++++++++++++++++++++++ 9 files changed, 122 insertions(+) create mode 100644 tests/golden/freed-list-wedge create mode 100644 tests/tests/freed-list-wedge.sh diff --git a/kmod/src/alloc.c b/kmod/src/alloc.c index f6a5be0c..78191e4b 100644 --- a/kmod/src/alloc.c +++ b/kmod/src/alloc.c @@ -25,6 +25,7 @@ #include "alloc.h" #include "counters.h" #include "scoutfs_trace.h" +#include "triggers.h" /* * The core allocator uses extent items in btrees rooted in the super. @@ -1241,6 +1242,62 @@ out: return ret; } +/* + * Test: stuff a freed list head to nearly full with real free blocks. + * + * Destructive: bypass filesystem consistency. Claim free blocks, append + * them into the head block via list_block_add (bypassing free_meta and + * its active-head-only path), and leak whatever we don't use. + * + * The caller can stuff both meta_freed heads in a single commits, allowing + * to reproduce the wedge condition. A counter validates we did the stuffing + * on both heads (increased twice - once for each head). An unfixed kernel + * will hang on mount. + */ +int scoutfs_alloc_fill_freed_list(struct super_block *sb, struct scoutfs_alloc *alloc, + struct scoutfs_block_writer *wri, + struct scoutfs_alloc_root *root, + struct scoutfs_alloc_list_head *lh) +{ + struct alloc_ext_args args = { + .alloc = alloc, + .wri = wri, + .root = root, + .zone = SCOUTFS_FREE_EXTENT_ORDER_ZONE, + }; + const u32 target = SCOUTFS_ALLOC_LIST_MAX_BLOCKS - 8; + struct scoutfs_alloc_list_block *lblk; + struct scoutfs_block *bl = NULL; + struct scoutfs_extent ext; + int ret = 0; + u64 old; /* leaked */ + u64 i; + u32 want; + + while (le32_to_cpu(lh->first_nr) < target) { + want = target - le32_to_cpu(lh->first_nr) + 1; + ret = scoutfs_ext_alloc(sb, &alloc_ext_ops, &args, 0, 0, want, &ext); + if (ret < 0) + break; + + ret = dirty_list_block(sb, alloc, wri, &lh->ref, ext.start, &old, &bl); + if (ret < 0) + break; + lblk = bl->data; + + for (i = 1; i < ext.len && le32_to_cpu(lh->first_nr) < target; i++) + list_block_add(lh, lblk, ext.start + i); + + scoutfs_block_put(sb, bl); + bl = NULL; + } + + scoutfs_block_put(sb, bl); + if (ret == 0 && le32_to_cpu(lh->first_nr) >= target) + scoutfs_inc_counter(sb, alloc_freed_fill); + return ret < 0 ? ret : 0; +} + /* * Move blknos from all the blocks in the list into extents in the root, * removing empty blocks as we go. This can return success and leave blocks diff --git a/kmod/src/alloc.h b/kmod/src/alloc.h index 70d39c5e..0ee181a2 100644 --- a/kmod/src/alloc.h +++ b/kmod/src/alloc.h @@ -152,6 +152,10 @@ int scoutfs_alloc_splice_list(struct super_block *sb, struct scoutfs_block_writer *wri, struct scoutfs_alloc_list_head *dst, struct scoutfs_alloc_list_head *src); +int scoutfs_alloc_fill_freed_list(struct super_block *sb, struct scoutfs_alloc *alloc, + struct scoutfs_block_writer *wri, + struct scoutfs_alloc_root *root, + struct scoutfs_alloc_list_head *lh); bool scoutfs_alloc_meta_low(struct super_block *sb, struct scoutfs_alloc *alloc, u32 nr); diff --git a/kmod/src/counters.h b/kmod/src/counters.h index 9088496c..cb7a2e18 100644 --- a/kmod/src/counters.h +++ b/kmod/src/counters.h @@ -16,6 +16,7 @@ EXPAND_COUNTER(alloc_alloc_meta) \ EXPAND_COUNTER(alloc_free_data) \ EXPAND_COUNTER(alloc_free_meta) \ + EXPAND_COUNTER(alloc_freed_fill) \ EXPAND_COUNTER(alloc_list_avail_lo) \ EXPAND_COUNTER(alloc_list_freed_hi) \ EXPAND_COUNTER(alloc_move) \ diff --git a/kmod/src/server.c b/kmod/src/server.c index ed97f556..73a8bfa7 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -623,6 +623,16 @@ static void scoutfs_server_commit_func(struct work_struct *work) goto out; } + /* test-only: manufacture the deadlock by stuffing both freed heads full + * in this one commit, so there's no window for empty_list to drain one + * before the other fills */ + if (scoutfs_trigger(sb, ALLOC_FILL_FREED_LIST)) { + scoutfs_alloc_fill_freed_list(sb, &server->alloc, &server->wri, + server->meta_avail, &server->alloc.freed); + scoutfs_alloc_fill_freed_list(sb, &server->alloc, &server->wri, + server->meta_avail, server->other_freed); + } + /* make sure next avail has sufficient blocks */ ret = scoutfs_alloc_fill_list(sb, &server->alloc, &server->wri, server->other_avail, diff --git a/kmod/src/triggers.c b/kmod/src/triggers.c index 15ddf907..c05465ac 100644 --- a/kmod/src/triggers.c +++ b/kmod/src/triggers.c @@ -47,6 +47,7 @@ static char *names[] = { [SCOUTFS_TRIGGER_STATFS_LOCK_PURGE] = "statfs_lock_purge", [SCOUTFS_TRIGGER_RECLAIM_SKIP_FINALIZE] = "reclaim_skip_finalize", [SCOUTFS_TRIGGER_LOG_MERGE_FORCE_PARTIAL] = "log_merge_force_partial", + [SCOUTFS_TRIGGER_ALLOC_FILL_FREED_LIST] = "alloc_fill_freed_list", }; bool scoutfs_trigger_test_and_clear(struct super_block *sb, unsigned int t) diff --git a/kmod/src/triggers.h b/kmod/src/triggers.h index 6d70cbea..3b5d8e1c 100644 --- a/kmod/src/triggers.h +++ b/kmod/src/triggers.h @@ -10,6 +10,7 @@ enum scoutfs_trigger { SCOUTFS_TRIGGER_STATFS_LOCK_PURGE, SCOUTFS_TRIGGER_RECLAIM_SKIP_FINALIZE, SCOUTFS_TRIGGER_LOG_MERGE_FORCE_PARTIAL, + SCOUTFS_TRIGGER_ALLOC_FILL_FREED_LIST, SCOUTFS_TRIGGER_NR, }; diff --git a/tests/golden/freed-list-wedge b/tests/golden/freed-list-wedge new file mode 100644 index 00000000..836b82b8 --- /dev/null +++ b/tests/golden/freed-list-wedge @@ -0,0 +1,8 @@ +== make throwaway scratch fs +== stuff both server freed heads full in one commit +== confirm both heads were stuffed (not a no-op) +both freed heads stuffed +== a fixed server drains them and keeps making progress +one +two +== cleanup scratch fs diff --git a/tests/sequence b/tests/sequence index 7e73df03..401b36a4 100644 --- a/tests/sequence +++ b/tests/sequence @@ -62,6 +62,7 @@ client-unmount-recovery.sh createmany-parallel-mounts.sh archive-light-cycle.sh block-stale-reads.sh +freed-list-wedge.sh inode-deletion.sh renameat2-noreplace.sh xfstests.sh diff --git a/tests/tests/freed-list-wedge.sh b/tests/tests/freed-list-wedge.sh new file mode 100644 index 00000000..79eb20c8 --- /dev/null +++ b/tests/tests/freed-list-wedge.sh @@ -0,0 +1,39 @@ +# +# Destructive: the alloc_fill_freed_list trigger claims free blocks and leaks +# them into both server_meta_freed heads in one commit, filling them near-full. +# Runs on a scratch fs; mkfs before re-use. +# +# A fixed server drains the full heads and stays live; an unfixed server wedges +# on the next commit and hangs until the harness times it out. +# + +scr_counter() { + cat "$(t_sysfs_path_from_mnt "$T_MSCR")/counters/$1" +} + +echo "== make throwaway scratch fs" +t_scratch_mkfs +t_scratch_mount + +echo "== stuff both server freed heads full in one commit" +old=$(scr_counter alloc_freed_fill) + +echo 1 > "/sys/kernel/debug/scoutfs/$(t_ident_from_mnt "$T_MSCR")/trigger/alloc_fill_freed_list" +echo one > "$T_MSCR/one"; sync + +echo "== confirm both heads were stuffed (not a no-op)" +filled=$(($(scr_counter alloc_freed_fill) - old)) +test "$filled" -ge 2 && echo "both freed heads stuffed" || \ + echo "stuff was a no-op ($filled heads)" + +echo "== a fixed server drains them and keeps making progress" +# an unfixed server wedges on this commit +echo two > "$T_MSCR/two"; sync +cat "$T_MSCR/one" "$T_MSCR/two" + +rm -f "$T_MSCR/one" "$T_MSCR/two"; sync + +echo "== cleanup scratch fs" +t_scratch_umount + +t_pass