diff --git a/kmod/src/alloc.c b/kmod/src/alloc.c index 0ceaf3b8..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. @@ -1150,13 +1151,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; } @@ -1234,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 @@ -1384,14 +1448,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)); } 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/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); 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/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); 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 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,