Merge tag 'bcachefs-2024-10-05' of git://evilpiepirate.org/bcachefs

Pull bcachefs fixes from Kent Overstreet:
 "A lot of little fixes, bigger ones include:

   - bcachefs's __wait_on_freeing_inode() was broken in rc1 due to vfs
     changes, now fixed along with another lost wakeup

   - fragmentation LRU fixes; fsck now repairs successfully (this is the
     data structure copygc uses); along with some nice simplification.

   - Rework logged op error handling, so that if logged op replay errors
     (due to another filesystem error) we delete the logged op instead
     of going into an infinite loop)

   - Various small filesystem connectivitity repair fixes"

* tag 'bcachefs-2024-10-05' of git://evilpiepirate.org/bcachefs:
  bcachefs: Rework logged op error handling
  bcachefs: Add warn param to subvol_get_snapshot, peek_inode
  bcachefs: Kill snapshot arg to fsck_write_inode()
  bcachefs: Check for unlinked, non-empty dirs in check_inode()
  bcachefs: Check for unlinked inodes with dirents
  bcachefs: Check for directories with no backpointers
  bcachefs: Kill alloc_v4.fragmentation_lru
  bcachefs: minor lru fsck fixes
  bcachefs: Mark more errors AUTOFIX
  bcachefs: Make sure we print error that causes fsck to bail out
  bcachefs: bkey errors are only AUTOFIX during read
  bcachefs: Create lost+found in correct snapshot
  bcachefs: Fix reattach_inode()
  bcachefs: Add missing wakeup to bch2_inode_hash_remove()
  bcachefs: Fix trans_commit disk accounting revert
  bcachefs: Fix bch2_inode_is_open() check
  bcachefs: Fix return type of dirent_points_to_inode_nowarn()
  bcachefs: Fix bad shift in bch2_read_flag_list()

20 files changed, 342 insertions, 209 deletions

diff --git a/fs/bcachefs/alloc_background.c b/fs/bcachefs/alloc_background.c
index 645b5ed4babb..4e4a448f6931 100644
--- a/fs/bcachefs/alloc_background.c
+++ b/fs/bcachefs/alloc_background.c
@@ -332,7 +332,6 @@ void bch2_alloc_v4_swab(struct bkey_s k)
 	a->io_time[1]		= swab64(a->io_time[1]);
 	a->stripe		= swab32(a->stripe);
 	a->nr_external_backpointers = swab32(a->nr_external_backpointers);
-	a->fragmentation_lru	= swab64(a->fragmentation_lru);
 	a->stripe_sectors	= swab32(a->stripe_sectors);
 
 	bps = alloc_v4_backpointers(a);
@@ -347,6 +346,7 @@ void bch2_alloc_to_text(struct printbuf *out, struct bch_fs *c, struct bkey_s_c
 {
 	struct bch_alloc_v4 _a;
 	const struct bch_alloc_v4 *a = bch2_alloc_to_v4(k, &_a);
+	struct bch_dev *ca = c ? bch2_dev_bucket_tryget_noerror(c, k.k->p) : NULL;
 
 	prt_newline(out);
 	printbuf_indent_add(out, 2);
@@ -364,9 +364,13 @@ void bch2_alloc_to_text(struct printbuf *out, struct bch_fs *c, struct bkey_s_c
 	prt_printf(out, "stripe_redundancy %u\n",	a->stripe_redundancy);
 	prt_printf(out, "io_time[READ]     %llu\n",	a->io_time[READ]);
 	prt_printf(out, "io_time[WRITE]    %llu\n",	a->io_time[WRITE]);
-	prt_printf(out, "fragmentation     %llu\n",	a->fragmentation_lru);
+
+	if (ca)
+		prt_printf(out, "fragmentation     %llu\n",	alloc_lru_idx_fragmentation(*a, ca));
 	prt_printf(out, "bp_start          %llu\n", BCH_ALLOC_V4_BACKPOINTERS_START(a));
 	printbuf_indent_sub(out, 2);
+
+	bch2_dev_put(ca);
 }
 
 void __bch2_alloc_to_v4(struct bkey_s_c k, struct bch_alloc_v4 *out)
@@ -882,12 +886,13 @@ int bch2_trigger_alloc(struct btree_trans *trans,
 				goto err;
 		}
 
-		new_a->fragmentation_lru = alloc_lru_idx_fragmentation(*new_a, ca);
-		if (old_a->fragmentation_lru != new_a->fragmentation_lru) {
+		old_lru = alloc_lru_idx_fragmentation(*old_a, ca);
+		new_lru = alloc_lru_idx_fragmentation(*new_a, ca);
+		if (old_lru != new_lru) {
 			ret = bch2_lru_change(trans,
 					BCH_LRU_FRAGMENTATION_START,
 					bucket_to_u64(new.k->p),
-					old_a->fragmentation_lru, new_a->fragmentation_lru);
+					old_lru, new_lru);
 			if (ret)
 				goto err;
 		}
@@ -1629,18 +1634,22 @@ static int bch2_check_alloc_to_lru_ref(struct btree_trans *trans,
 	if (ret)
 		return ret;
 
+	struct bch_dev *ca = bch2_dev_tryget_noerror(c, alloc_k.k->p.inode);
+	if (!ca)
+		return 0;
+
 	a = bch2_alloc_to_v4(alloc_k, &a_convert);
 
-	if (a->fragmentation_lru) {
+	u64 lru_idx = alloc_lru_idx_fragmentation(*a, ca);
+	if (lru_idx) {
 		ret = bch2_lru_check_set(trans, BCH_LRU_FRAGMENTATION_START,
-					 a->fragmentation_lru,
-					 alloc_k, last_flushed);
+					 lru_idx, alloc_k, last_flushed);
 		if (ret)
-			return ret;
+			goto err;
 	}
 
 	if (a->data_type != BCH_DATA_cached)
-		return 0;
+		goto err;
 
 	if (fsck_err_on(!a->io_time[READ],
 			trans, alloc_key_cached_but_read_time_zero,
@@ -1669,6 +1678,7 @@ static int bch2_check_alloc_to_lru_ref(struct btree_trans *trans,
 		goto err;
 err:
 fsck_err:
+	bch2_dev_put(ca);
 	printbuf_exit(&buf);
 	return ret;
 }
diff --git a/fs/bcachefs/alloc_background_format.h b/fs/bcachefs/alloc_background_format.h
index f754a2951d8a..befdaa95c515 100644
--- a/fs/bcachefs/alloc_background_format.h
+++ b/fs/bcachefs/alloc_background_format.h
@@ -70,7 +70,7 @@ struct bch_alloc_v4 {
 	__u32			stripe;
 	__u32			nr_external_backpointers;
 	/* end of fields in original version of alloc_v4 */
-	__u64			fragmentation_lru;
+	__u64			_fragmentation_lru; /* obsolete */
 	__u32			stripe_sectors;
 	__u32			pad;
 } __packed __aligned(8);
diff --git a/fs/bcachefs/btree_gc.c b/fs/bcachefs/btree_gc.c
index 660d2fa02da2..771154e3a291 100644
--- a/fs/bcachefs/btree_gc.c
+++ b/fs/bcachefs/btree_gc.c
@@ -828,8 +828,6 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
 			return ret;
 	}
 
-	gc.fragmentation_lru = alloc_lru_idx_fragmentation(gc, ca);
-
 	if (fsck_err_on(new.data_type != gc.data_type,
 			trans, alloc_key_data_type_wrong,
 			"bucket %llu:%llu gen %u has wrong data_type"
@@ -857,7 +855,6 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
 	copy_bucket_field(alloc_key_cached_sectors_wrong,	cached_sectors);
 	copy_bucket_field(alloc_key_stripe_wrong,		stripe);
 	copy_bucket_field(alloc_key_stripe_redundancy_wrong,	stripe_redundancy);
-	copy_bucket_field(alloc_key_fragmentation_lru_wrong,	fragmentation_lru);
 #undef copy_bucket_field
 
 	if (!bch2_alloc_v4_cmp(*old, new))
diff --git a/fs/bcachefs/btree_trans_commit.c b/fs/bcachefs/btree_trans_commit.c
index 1a74a1a252ee..9bf471fa4361 100644
--- a/fs/bcachefs/btree_trans_commit.c
+++ b/fs/bcachefs/btree_trans_commit.c
@@ -832,7 +832,8 @@ revert_fs_usage:
 	for (struct jset_entry *entry2 = trans->journal_entries;
 	     entry2 != entry;
 	     entry2 = vstruct_next(entry2))
-		if (jset_entry_is_key(entry2) && entry2->start->k.type == KEY_TYPE_accounting) {
+		if (entry2->type == BCH_JSET_ENTRY_write_buffer_keys &&
+		    entry2->start->k.type == KEY_TYPE_accounting) {
 			struct bkey_s_accounting a = bkey_i_to_s_accounting(entry2->start);
 
 			bch2_accounting_neg(a);
diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c
index 3a16b535b6c3..7a79f695ba2e 100644
--- a/fs/bcachefs/error.c
+++ b/fs/bcachefs/error.c
@@ -393,6 +393,14 @@ int __bch2_fsck_err(struct bch_fs *c,
 	     !(flags & FSCK_CAN_IGNORE)))
 		ret = -BCH_ERR_fsck_errors_not_fixed;
 
+	bool exiting =
+		test_bit(BCH_FS_fsck_running, &c->flags) &&
+		(ret != -BCH_ERR_fsck_fix &&
+		 ret != -BCH_ERR_fsck_ignore);
+
+	if (exiting)
+		print = true;
+
 	if (print) {
 		if (bch2_fs_stdio_redirect(c))
 			bch2_print(c, "%s\n", out->buf);
@@ -400,9 +408,7 @@ int __bch2_fsck_err(struct bch_fs *c,
 			bch2_print_string_as_lines(KERN_ERR, out->buf);
 	}
 
-	if (test_bit(BCH_FS_fsck_running, &c->flags) &&
-	    (ret != -BCH_ERR_fsck_fix &&
-	     ret != -BCH_ERR_fsck_ignore))
+	if (exiting)
 		bch_err(c, "Unable to continue, halting");
 	else if (suppressing)
 		bch_err(c, "Ratelimiting new instances of previous error");
@@ -430,10 +436,17 @@ err:
 
 int __bch2_bkey_fsck_err(struct bch_fs *c,
 			 struct bkey_s_c k,
-			 enum bch_fsck_flags flags,
+			 enum bch_validate_flags validate_flags,
 			 enum bch_sb_error_id err,
 			 const char *fmt, ...)
 {
+	if (validate_flags & BCH_VALIDATE_silent)
+		return -BCH_ERR_fsck_delete_bkey;
+
+	unsigned fsck_flags = 0;
+	if (!(validate_flags & (BCH_VALIDATE_write|BCH_VALIDATE_commit)))
+		fsck_flags |= FSCK_AUTOFIX|FSCK_CAN_FIX;
+
 	struct printbuf buf = PRINTBUF;
 	va_list args;
 
@@ -445,7 +458,7 @@ int __bch2_bkey_fsck_err(struct bch_fs *c,
 	va_end(args);
 	prt_str(&buf, ": delete?");
 
-	int ret = __bch2_fsck_err(c, NULL, flags, err, "%s", buf.buf);
+	int ret = __bch2_fsck_err(c, NULL, fsck_flags, err, "%s", buf.buf);
 	printbuf_exit(&buf);
 	return ret;
 }
diff --git a/fs/bcachefs/error.h b/fs/bcachefs/error.h
index 21ee7211b03e..6551ada926b6 100644
--- a/fs/bcachefs/error.h
+++ b/fs/bcachefs/error.h
@@ -167,10 +167,11 @@ void bch2_flush_fsck_errs(struct bch_fs *);
 #define fsck_err_on(cond, c, _err_type, ...)				\
 	__fsck_err_on(cond, c, FSCK_CAN_FIX|FSCK_CAN_IGNORE, _err_type, __VA_ARGS__)
 
+enum bch_validate_flags;
 __printf(5, 6)
 int __bch2_bkey_fsck_err(struct bch_fs *,
 			 struct bkey_s_c,
-			 enum bch_fsck_flags,
+			 enum bch_validate_flags,
 			 enum bch_sb_error_id,
 			 const char *, ...);
 
@@ -180,11 +181,7 @@ int __bch2_bkey_fsck_err(struct bch_fs *,
  */
 #define bkey_fsck_err(c, _err_type, _err_msg, ...)			\
 do {									\
-	if ((flags & BCH_VALIDATE_silent)) {				\
-		ret = -BCH_ERR_fsck_delete_bkey;			\
-		goto fsck_err;						\
-	}								\
-	int _ret = __bch2_bkey_fsck_err(c, k, FSCK_CAN_FIX|FSCK_AUTOFIX,\
+	int _ret = __bch2_bkey_fsck_err(c, k, flags,			\
 				BCH_FSCK_ERR_##_err_type,		\
 				_err_msg, ##__VA_ARGS__);		\
 	if (_ret != -BCH_ERR_fsck_fix &&				\
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 4a1bb07a2574..5bfc26d58270 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -174,20 +174,24 @@ static const struct rhashtable_params bch2_vfs_inodes_params = {
 	.automatic_shrinking	= true,
 };
 
-static void __wait_on_freeing_inode(struct inode *inode)
+struct bch_inode_info *__bch2_inode_hash_find(struct bch_fs *c, subvol_inum inum)
 {
-	wait_queue_head_t *wq;
-	DEFINE_WAIT_BIT(wait, &inode->i_state, __I_NEW);
-	wq = bit_waitqueue(&inode->i_state, __I_NEW);
-	prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
-	spin_unlock(&inode->i_lock);
-	schedule();
-	finish_wait(wq, &wait.wq_entry);
+	return rhashtable_lookup_fast(&c->vfs_inodes_table, &inum, bch2_vfs_inodes_params);
 }
 
-struct bch_inode_info *__bch2_inode_hash_find(struct bch_fs *c, subvol_inum inum)
+static void __wait_on_freeing_inode(struct bch_fs *c,
+				    struct bch_inode_info *inode,
+				    subvol_inum inum)
 {
-	return rhashtable_lookup_fast(&c->vfs_inodes_table, &inum, bch2_vfs_inodes_params);
+	wait_queue_head_t *wq;
+	DEFINE_WAIT_BIT(wait, &inode->v.i_state, __I_NEW);
+	wq = inode_bit_waitqueue(&wait, &inode->v, __I_NEW);
+	prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
+	spin_unlock(&inode->v.i_lock);
+
+	if (__bch2_inode_hash_find(c, inum) == inode)
+		schedule_timeout(HZ * 10);
+	finish_wait(wq, &wait.wq_entry);
 }
 
 static struct bch_inode_info *bch2_inode_hash_find(struct bch_fs *c, struct btree_trans *trans,
@@ -204,10 +208,10 @@ repeat:
 		}
 		if ((inode->v.i_state & (I_FREEING|I_WILL_FREE))) {
 			if (!trans) {
-				__wait_on_freeing_inode(&inode->v);
+				__wait_on_freeing_inode(c, inode, inum);
 			} else {
 				bch2_trans_unlock(trans);
-				__wait_on_freeing_inode(&inode->v);
+				__wait_on_freeing_inode(c, inode, inum);
 				int ret = bch2_trans_relock(trans);
 				if (ret)
 					return ERR_PTR(ret);
@@ -232,6 +236,11 @@ static void bch2_inode_hash_remove(struct bch_fs *c, struct bch_inode_info *inod
 					&inode->hash, bch2_vfs_inodes_params);
 		BUG_ON(ret);
 		inode->v.i_hash.pprev = NULL;
+		/*
+		 * This pairs with the bch2_inode_hash_find() ->
+		 * __wait_on_freeing_inode() path
+		 */
+		inode_wake_up_bit(&inode->v, __I_NEW);
 	}
 }
 
diff --git a/fs/bcachefs/fsck.c b/fs/bcachefs/fsck.c
index 0d8b782b63fb..b8a6ceb0cc7a 100644
--- a/fs/bcachefs/fsck.c
+++ b/fs/bcachefs/fsck.c
@@ -28,8 +28,8 @@ static bool inode_points_to_dirent(struct bch_inode_unpacked *inode,
 		inode->bi_dir_offset	== d.k->p.offset;
 }
 
-static bool dirent_points_to_inode_nowarn(struct bkey_s_c_dirent d,
-				   struct bch_inode_unpacked *inode)
+static int dirent_points_to_inode_nowarn(struct bkey_s_c_dirent d,
+					 struct bch_inode_unpacked *inode)
 {
 	if (d.v->d_type == DT_SUBVOL
 	    ? le32_to_cpu(d.v->d_child_subvol)	== inode->bi_subvol
@@ -137,16 +137,15 @@ found:
 	return ret;
 }
 
-static int lookup_inode(struct btree_trans *trans, u64 inode_nr,
-			struct bch_inode_unpacked *inode,
-			u32 *snapshot)
+static int lookup_inode(struct btree_trans *trans, u64 inode_nr, u32 snapshot,
+			struct bch_inode_unpacked *inode)
 {
 	struct btree_iter iter;
 	struct bkey_s_c k;
 	int ret;
 
 	k = bch2_bkey_get_iter(trans, &iter, BTREE_ID_inodes,
-			       SPOS(0, inode_nr, *snapshot), 0);
+			       SPOS(0, inode_nr, snapshot), 0);
 	ret = bkey_err(k);
 	if (ret)
 		goto err;
@@ -154,8 +153,6 @@ static int lookup_inode(struct btree_trans *trans, u64 inode_nr,
 	ret = bkey_is_inode(k.k)
 		? bch2_inode_unpack(k, inode)
 		: -BCH_ERR_ENOENT_inode;
-	if (!ret)
-		*snapshot = iter.pos.snapshot;
 err:
 	bch2_trans_iter_exit(trans, &iter);
 	return ret;
@@ -250,8 +247,7 @@ static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
 
 	struct bch_inode_unpacked root_inode;
 	struct bch_hash_info root_hash_info;
-	u32 root_inode_snapshot = snapshot;
-	ret = lookup_inode(trans, root_inum.inum, &root_inode, &root_inode_snapshot);
+	ret = lookup_inode(trans, root_inum.inum, snapshot, &root_inode);
 	bch_err_msg(c, ret, "looking up root inode %llu for subvol %u",
 		    root_inum.inum, le32_to_cpu(st.master_subvol));
 	if (ret)
@@ -277,17 +273,23 @@ static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
 	 * The bch2_check_dirents pass has already run, dangling dirents
 	 * shouldn't exist here:
 	 */
-	ret = lookup_inode(trans, inum, lostfound, &snapshot);
+	ret = lookup_inode(trans, inum, snapshot, lostfound);
 	bch_err_msg(c, ret, "looking up lost+found %llu:%u in (root inode %llu, snapshot root %u)",
 		    inum, snapshot, root_inum.inum, bch2_snapshot_root(c, snapshot));
 	return ret;
 
 create_lostfound:
 	/*
+	 * we always create lost+found in the root snapshot; we don't want
+	 * different branches of the snapshot tree to have different lost+found
+	 */
+	snapshot = le32_to_cpu(st.root_snapshot);
+	/*
 	 * XXX: we could have a nicer log message here  if we had a nice way to
 	 * walk backpointers to print a path
 	 */
-	bch_notice(c, "creating lost+found in snapshot %u", le32_to_cpu(st.root_snapshot));
+	bch_notice(c, "creating lost+found in subvol %llu snapshot %u",
+		   root_inum.subvol, le32_to_cpu(st.root_snapshot));
 
 	u64 now = bch2_current_time(c);
 	struct btree_iter lostfound_iter = { NULL };
@@ -296,6 +298,7 @@ create_lostfound:
 	bch2_inode_init_early(c, lostfound);
 	bch2_inode_init_late(lostfound, now, 0, 0, S_IFDIR|0700, 0, &root_inode);
 	lostfound->bi_dir = root_inode.bi_inum;
+	lostfound->bi_snapshot = le32_to_cpu(st.root_snapshot);
 
 	root_inode.bi_nlink++;
 
@@ -323,9 +326,7 @@ err:
 	return ret;
 }
 
-static int reattach_inode(struct btree_trans *trans,
-			  struct bch_inode_unpacked *inode,
-			  u32 inode_snapshot)
+static int reattach_inode(struct btree_trans *trans, struct bch_inode_unpacked *inode)
 {
 	struct bch_fs *c = trans->c;
 	struct bch_hash_info dir_hash;
@@ -333,7 +334,7 @@ static int reattach_inode(struct btree_trans *trans,
 	char name_buf[20];
 	struct qstr name;
 	u64 dir_offset = 0;
-	u32 dirent_snapshot = inode_snapshot;
+	u32 dirent_snapshot = inode->bi_snapshot;
 	int ret;
 
 	if (inode->bi_subvol) {
@@ -354,14 +355,18 @@ static int reattach_inode(struct btree_trans *trans,
 	if (ret)
 		return ret;
 
-	if (S_ISDIR(inode->bi_mode)) {
-		lostfound.bi_nlink++;
+	lostfound.bi_nlink += S_ISDIR(inode->bi_mode);
 
-		ret = __bch2_fsck_write_inode(trans, &lostfound, U32_MAX);
-		if (ret)
-			return ret;
+	/* ensure lost+found inode is also present in inode snapshot */
+	if (!inode->bi_subvol) {
+		BUG_ON(!bch2_snapshot_is_ancestor(c, inode->bi_snapshot, lostfound.bi_snapshot));
+		lostfound.bi_snapshot = inode->bi_snapshot;
 	}
 
+	ret = __bch2_fsck_write_inode(trans, &lostfound);
+	if (ret)
+		return ret;
+
 	dir_hash = bch2_hash_info_init(c, &lostfound);
 
 	name = (struct qstr) QSTR(name_buf);
@@ -383,7 +388,7 @@ static int reattach_inode(struct btree_trans *trans,
 	inode->bi_dir		= lostfound.bi_inum;
 	inode->bi_dir_offset	= dir_offset;
 
-	return __bch2_fsck_write_inode(trans, inode, inode_snapshot);
+	return __bch2_fsck_write_inode(trans, inode);
 }
 
 static int remove_backpointer(struct btree_trans *trans,
@@ -422,7 +427,7 @@ static int reattach_subvol(struct btree_trans *trans, struct bkey_s_c_subvolume
 	if (ret)
 		return ret;
 
-	ret = reattach_inode(trans, &inode, le32_to_cpu(s.v->snapshot));
+	ret = reattach_inode(trans, &inode);
 	bch_err_msg(c, ret, "reattaching inode %llu", inode.bi_inum);
 	return ret;
 }
@@ -540,8 +545,9 @@ static int reconstruct_inode(struct btree_trans *trans, enum btree_id btree, u32
 	bch2_inode_init_late(&new_inode, bch2_current_time(c), 0, 0, i_mode|0600, 0, NULL);
 	new_inode.bi_size = i_size;
 	new_inode.bi_inum = inum;
+	new_inode.bi_snapshot = snapshot;
 
-	return __bch2_fsck_write_inode(trans, &new_inode, snapshot);
+	return __bch2_fsck_write_inode(trans, &new_inode);
 }
 
 struct snapshots_seen {
@@ -1024,6 +1030,7 @@ static int check_inode(struct btree_trans *trans,
 		       bool full)
 {
 	struct bch_fs *c = trans->c;
+	struct printbuf buf = PRINTBUF;
 	struct bch_inode_unpacked u;
 	bool do_update = false;
 	int ret;
@@ -1057,7 +1064,41 @@ static int check_inode(struct btree_trans *trans,
 			trans, inode_snapshot_mismatch,
 			"inodes in different snapshots don't match")) {
 		bch_err(c, "repair not implemented yet");
-		return -BCH_ERR_fsck_repair_unimplemented;
+		ret = -BCH_ERR_fsck_repair_unimplemented;
+		goto err_noprint;
+	}
+
+	if (u.bi_dir || u.bi_dir_offset) {
+		ret = check_inode_dirent_inode(trans, &u, &do_update);
+		if (ret)
+			goto err;
+	}
+
+	if (fsck_err_on(u.bi_dir && (u.bi_flags & BCH_INODE_unlinked),
+			trans, inode_unlinked_but_has_dirent,
+			"inode unlinked but has dirent\n%s",
+			(printbuf_reset(&buf),
+			 bch2_inode_unpacked_to_text(&buf, &u),
+			 buf.buf))) {
+		u.bi_flags &= ~BCH_INODE_unlinked;
+		do_update = true;
+	}
+
+	if (S_ISDIR(u.bi_mode) && (u.bi_flags & BCH_INODE_unlinked)) {
+		/* Check for this early so that check_unreachable_inode() will reattach it */
+
+		ret = bch2_empty_dir_snapshot(trans, k.k->p.offset, 0, k.k->p.snapshot);
+		if (ret && ret != -BCH_ERR_ENOTEMPTY_dir_not_empty)
+			goto err;
+
+		fsck_err_on(ret, trans, inode_dir_unlinked_but_not_empty,
+			    "dir unlinked but not empty\n%s",
+			    (printbuf_reset(&buf),
+			     bch2_inode_unpacked_to_text(&buf, &u),
+			     buf.buf));
+		u.bi_flags &= ~BCH_INODE_unlinked;
+		do_update = true;
+		ret = 0;
 	}
 
 	if ((u.bi_flags & (BCH_INODE_i_size_dirty|BCH_INODE_unlinked)) &&
@@ -1070,15 +1111,15 @@ static int check_inode(struct btree_trans *trans,
 
 		u.bi_flags &= ~BCH_INODE_i_size_dirty|BCH_INODE_unlinked;
 
-		ret = __bch2_fsck_write_inode(trans, &u, iter->pos.snapshot);
+		ret = __bch2_fsck_write_inode(trans, &u);
 
 		bch_err_msg(c, ret, "in fsck updating inode");
 		if (ret)
-			return ret;
+			goto err_noprint;
 
 		if (!bpos_eq(new_min_pos, POS_MIN))
 			bch2_btree_iter_set_pos(iter, bpos_predecessor(new_min_pos));
-		return 0;
+		goto err_noprint;
 	}
 
 	if (u.bi_flags & BCH_INODE_unlinked) {
@@ -1095,7 +1136,7 @@ static int check_inode(struct btree_trans *trans,
 			 */
 			ret = check_inode_deleted_list(trans, k.k->p);
 			if (ret < 0)
-				return ret;
+				goto err_noprint;
 
 			fsck_err_on(!ret,
 				    trans, unlinked_inode_not_on_deleted_list,
@@ -1106,13 +1147,13 @@ static int check_inode(struct btree_trans *trans,
 			if (ret)
 				goto err;
 		} else {
-			if (fsck_err_on(bch2_inode_is_open(c, k.k->p),
+			if (fsck_err_on(!bch2_inode_is_open(c, k.k->p),
 					trans, inode_unlinked_and_not_open,
 				      "inode %llu%u unlinked and not open",
 				      u.bi_inum, u.bi_snapshot)) {
 				ret = bch2_inode_rm_snapshot(trans, u.bi_inum, iter->pos.snapshot);
 				bch_err_msg(c, ret, "in fsck deleting inode");
-				return ret;
+				goto err_noprint;
 			}
 		}
 	}
@@ -1177,12 +1218,6 @@ static int check_inode(struct btree_trans *trans,
 		do_update = true;
 	}
 
-	if (u.bi_dir || u.bi_dir_offset) {
-		ret = check_inode_dirent_inode(trans, &u, &do_update);
-		if (ret)
-			goto err;
-	}
-
 	if (fsck_err_on(u.bi_parent_subvol &&
 			(u.bi_subvol == 0 ||
 			 u.bi_subvol == BCACHEFS_ROOT_SUBVOL),
@@ -1224,14 +1259,16 @@ static int check_inode(struct btree_trans *trans,
 	}
 do_update:
 	if (do_update) {
-		ret = __bch2_fsck_write_inode(trans, &u, iter->pos.snapshot);
+		ret = __bch2_fsck_write_inode(trans, &u);
 		bch_err_msg(c, ret, "in fsck updating inode");
 		if (ret)
-			return ret;
+			goto err_noprint;
 	}
 err:
 fsck_err:
 	bch_err_fn(c, ret);
+err_noprint:
+	printbuf_exit(&buf);
 	return ret;
 }
 
@@ -1347,7 +1384,7 @@ static int check_i_sectors_notnested(struct btree_trans *trans, struct inode_wal
 				w->last_pos.inode, i->snapshot,
 				i->inode.bi_sectors, i->count)) {
 			i->inode.bi_sectors = i->count;
-			ret = bch2_fsck_write_inode(trans, &i->inode, i->snapshot);
+			ret = bch2_fsck_write_inode(trans, &i->inode);
 			if (ret)
 				break;
 		}
@@ -1789,7 +1826,7 @@ static int check_subdir_count_notnested(struct btree_trans *trans, struct inode_
 				"directory %llu:%u with wrong i_nlink: got %u, should be %llu",
 				w->last_pos.inode, i->snapshot, i->inode.bi_nlink, i->count)) {
 			i->inode.bi_nlink = i->count;
-			ret = bch2_fsck_write_inode(trans, &i->inode, i->snapshot);
+			ret = bch2_fsck_write_inode(trans, &i->inode);
 			if (ret)
 				break;
 		}
@@ -1810,8 +1847,7 @@ noinline_for_stack
 static int check_dirent_inode_dirent(struct btree_trans *trans,
 				   struct btree_iter *iter,
 				   struct bkey_s_c_dirent d,
-				   struct bch_inode_unpacked *target,
-				   u32 target_snapshot)
+				   struct bch_inode_unpacked *target)
 {
 	struct bch_fs *c = trans->c;
 	struct printbuf buf = PRINTBUF;
@@ -1821,6 +1857,32 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
 	if (inode_points_to_dirent(target, d))
 		return 0;
 
+	if (!target->bi_dir &&
+	    !target->bi_dir_offset) {
+		fsck_err_on(S_ISDIR(target->bi_mode),
+			    trans, inode_dir_missing_backpointer,
+			    "directory with missing backpointer\n%s",
+			    (printbuf_reset(&buf),
+			     bch2_bkey_val_to_text(&buf, c, d.s_c),
+			     prt_printf(&buf, "\n"),
+			     bch2_inode_unpacked_to_text(&buf, target),
+			     buf.buf));
+
+		fsck_err_on(target->bi_flags & BCH_INODE_unlinked,
+			    trans, inode_unlinked_but_has_dirent,
+			    "inode unlinked but has dirent\n%s",
+			    (printbuf_reset(&buf),
+			     bch2_bkey_val_to_text(&buf, c, d.s_c),
+			     prt_printf(&buf, "\n"),
+			     bch2_inode_unpacked_to_text(&buf, target),
+			     buf.buf));
+
+		target->bi_flags &= ~BCH_INODE_unlinked;
+		target->bi_dir		= d.k->p.inode;
+		target->bi_dir_offset	= d.k->p.offset;
+		return __bch2_fsck_write_inode(trans, target);
+	}
+
 	if (bch2_inode_should_have_bp(target) &&
 	    !fsck_err(trans, inode_wrong_backpointer,
 		      "dirent points to inode that does not point back:\n  %s",
@@ -1830,15 +1892,8 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
 		       buf.buf)))
 		goto err;
 
-	if (!target->bi_dir &&
-	    !target->bi_dir_offset) {
-		target->bi_dir		= d.k->p.inode;
-		target->bi_dir_offset	= d.k->p.offset;
-		return __bch2_fsck_write_inode(trans, target, target_snapshot);
-	}
-
 	struct bkey_s_c_dirent bp_dirent = dirent_get_by_pos(trans, &bp_iter,
-			      SPOS(target->bi_dir, target->bi_dir_offset, target_snapshot));
+			      SPOS(target->bi_dir, target->bi_dir_offset, target->bi_snapshot));
 	ret = bkey_err(bp_dirent);
 	if (ret && !bch2_err_matches(ret, ENOENT))
 		goto err;
@@ -1851,14 +1906,14 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
 			"inode %llu:%u has wrong backpointer:\n"
 			"got       %llu:%llu\n"
 			"should be %llu:%llu",
-			target->bi_inum, target_snapshot,
+			target->bi_inum, target->bi_snapshot,
 			target->bi_dir,
 			target->bi_dir_offset,
 			d.k->p.inode,
 			d.k->p.offset)) {
 		target->bi_dir		= d.k->p.inode;
 		target->bi_dir_offset	= d.k->p.offset;
-		ret = __bch2_fsck_write_inode(trans, target, target_snapshot);
+		ret = __bch2_fsck_write_inode(trans, target);
 		goto out;
 	}
 
@@ -1873,7 +1928,7 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
 			trans, inode_dir_multiple_links,
 			"%s %llu:%u with multiple links\n%s",
 			S_ISDIR(target->bi_mode) ? "directory" : "subvolume",
-			target->bi_inum, target_snapshot, buf.buf)) {
+			target->bi_inum, target->bi_snapshot, buf.buf)) {
 		ret = __remove_dirent(trans, d.k->p);
 		goto out;
 	}
@@ -1886,10 +1941,10 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
 	if (fsck_err_on(backpointer_exists && !target->bi_nlink,
 			trans, inode_multiple_links_but_nlink_0,
 			"inode %llu:%u type %s has multiple links but i_nlink 0\n%s",
-			target->bi_inum, target_snapshot, bch2_d_types[d.v->d_type], buf.buf)) {
+			target->bi_inum, target->bi_snapshot, bch2_d_types[d.v->d_type], buf.buf)) {
 		target->bi_nlink++;
 		target->bi_flags &= ~BCH_INODE_unlinked;
-		ret = __bch2_fsck_write_inode(trans, target, target_snapshot);
+		ret = __bch2_fsck_write_inode(trans, target);
 		if (ret)
 			goto err;
 	}
@@ -1906,15 +1961,14 @@ noinline_for_stack
 static int check_dirent_target(struct btree_trans *trans,
 			       struct btree_iter *iter,
 			       struct bkey_s_c_dirent d,
-			       struct bch_inode_unpacked *target,
-			       u32 target_snapshot)
+			       struct bch_inode_unpacked *target)
 {
 	struct bch_fs *c = trans->c;
 	struct bkey_i_dirent *n;
 	struct printbuf buf = PRINTBUF;
 	int ret = 0;
 
-	ret = check_dirent_inode_dirent(trans, iter, d, target, target_snapshot);
+	ret = check_dirent_inode_dirent(trans, iter, d, target);
 	if (ret)
 		goto err;
 
@@ -2073,7 +2127,7 @@ static int check_dirent_to_subvol(struct btree_trans *trans, struct btree_iter *
 	u64 target_inum = le64_to_cpu(s.v->inode);
 	u32 target_snapshot = le32_to_cpu(s.v->snapshot);
 
-	ret = lookup_inode(trans, target_inum, &subvol_root, &target_snapshot);
+	ret = lookup_inode(trans, target_inum, target_snapshot, &subvol_root);
 	if (ret && !bch2_err_matches(ret, ENOENT))
 		goto err;
 
@@ -2089,13 +2143,13 @@ static int check_dirent_to_subvol(struct btree_trans *trans, struct btree_iter *
 			target_inum,
 			subvol_root.bi_parent_subvol, parent_subvol)) {
 		subvol_root.bi_parent_subvol = parent_subvol;
-		ret = __bch2_fsck_write_inode(trans, &subvol_root, target_snapshot);
+		subvol_root.bi_snapshot = le32_to_cpu(s.v->snapshot);
+		ret = __bch2_fsck_write_inode(trans, &subvol_root);
 		if (ret)
 			goto err;
 	}
 
-	ret = check_dirent_target(trans, iter, d, &subvol_root,
-				  target_snapshot);
+	ret = check_dirent_target(trans, iter, d, &subvol_root);
 	if (ret)
 		goto err;
 out:
@@ -2188,8 +2242,7 @@ static int check_dirent(struct btree_trans *trans, struct btree_iter *iter,
 		}
 
 		darray_for_each(target->inodes, i) {
-			ret = check_dirent_target(trans, iter, d,
-						  &i->inode, i->snapshot);
+			ret = check_dirent_target(trans, iter, d, &i->inode);
 			if (ret)
 				goto err;
 		}
@@ -2330,7 +2383,7 @@ static int check_root_trans(struct btree_trans *trans)
 			goto err;
 	}
 
-	ret = lookup_inode(trans, BCACHEFS_ROOT_INO, &root_inode, &snapshot);
+	ret = lookup_inode(trans, BCACHEFS_ROOT_INO, snapshot, &root_inode);
 	if (ret && !bch2_err_matches(ret, ENOENT))
 		return ret;
 
@@ -2343,8 +2396,9 @@ static int check_root_trans(struct btree_trans *trans)
 		bch2_inode_init(c, &root_inode, 0, 0, S_IFDIR|0755,
 				0, NULL);
 		root_inode.bi_inum = inum;
+		root_inode.bi_snapshot = snapshot;
 
-		ret = __bch2_fsck_write_inode(trans, &root_inode, snapshot);
+		ret = __bch2_fsck_write_inode(trans, &root_inode);
 		bch_err_msg(c, ret, "writing root inode");
 	}
 err:
@@ -2511,7 +2565,7 @@ static int check_path(struct btree_trans *trans, pathbuf *p, struct bkey_s_c ino
 				     (printbuf_reset(&buf),
 				      bch2_bkey_val_to_text(&buf, c, inode_k),
 				      buf.buf)))
-				ret = reattach_inode(trans, &inode, snapshot);
+				ret = reattach_inode(trans, &inode);
 			goto out;
 		}
 
@@ -2557,7 +2611,7 @@ static int check_path(struct btree_trans *trans, pathbuf *p, struct bkey_s_c ino
 				if (ret)
 					break;
 
-				ret = reattach_inode(trans, &inode, snapshot);
+				ret = reattach_inode(trans, &inode);
 				bch_err_msg(c, ret, "reattaching inode %llu", inode.bi_inum);
 			}
 			break;
@@ -2787,7 +2841,7 @@ static int check_nlinks_update_inode(struct btree_trans *trans, struct btree_ite
 			u.bi_inum, bch2_d_types[mode_to_type(u.bi_mode)],
 			bch2_inode_nlink_get(&u), link->count)) {
 		bch2_inode_nlink_set(&u, link->count);
-		ret = __bch2_fsck_write_inode(trans, &u, k.k->p.snapshot);
+		ret = __bch2_fsck_write_inode(trans, &u);
 	}
 fsck_err:
 	return ret;
diff --git a/fs/bcachefs/inode.c b/fs/bc