Update bcachefs sources to e027cf9aa0 fixup! bcachefs: Defer checking of alloc -> lru refs until after RW

32 files changed, 839 insertions, 549 deletions

diff --git a/.bcachefs_revision b/.bcachefs_revision
index a4341deb..3cdd136f 100644
--- a/.bcachefs_revision
+++ b/.bcachefs_revision
@@ -1 +1 @@
-91e6c3e0d5ac0d29a9c97e71a1ba7abb346b4991
+e027cf9aa0e18b688d76cd6c2702491b8d06f48f
diff --git a/libbcachefs/alloc_background.c b/libbcachefs/alloc_background.c
index e8a34ecc..6d6798ae 100644
--- a/libbcachefs/alloc_background.c
+++ b/libbcachefs/alloc_background.c
@@ -302,71 +302,57 @@ static unsigned bch_alloc_v1_val_u64s(const struct bch_alloc *a)
 	return DIV_ROUND_UP(bytes, sizeof(u64));
 }
 
-const char *bch2_alloc_v1_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_alloc_v1_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
 {
 	struct bkey_s_c_alloc a = bkey_s_c_to_alloc(k);
 
-	if (k.k->p.inode >= c->sb.nr_devices ||
-	    !c->devs[k.k->p.inode])
-		return "invalid device";
-
 	/* allow for unknown fields */
-	if (bkey_val_u64s(a.k) < bch_alloc_v1_val_u64s(a.v))
-		return "incorrect value size";
+	if (bkey_val_u64s(a.k) < bch_alloc_v1_val_u64s(a.v)) {
+		pr_buf(err, "incorrect value size (%zu < %u)",
+		       bkey_val_u64s(a.k), bch_alloc_v1_val_u64s(a.v));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
-const char *bch2_alloc_v2_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_alloc_v2_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
 {
 	struct bkey_alloc_unpacked u;
 
-	if (k.k->p.inode >= c->sb.nr_devices ||
-	    !c->devs[k.k->p.inode])
-		return "invalid device";
-
-	if (bch2_alloc_unpack_v2(&u, k))
-		return "unpack error";
+	if (bch2_alloc_unpack_v2(&u, k)) {
+		pr_buf(err, "unpack error");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
-const char *bch2_alloc_v3_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_alloc_v3_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
 {
 	struct bkey_alloc_unpacked u;
-	struct bch_dev *ca;
-
-	if (k.k->p.inode >= c->sb.nr_devices ||
-	    !c->devs[k.k->p.inode])
-		return "invalid device";
-
-	ca = bch_dev_bkey_exists(c, k.k->p.inode);
-
-	if (k.k->p.offset < ca->mi.first_bucket ||
-	    k.k->p.offset >= ca->mi.nbuckets)
-		return "invalid bucket";
 
-	if (bch2_alloc_unpack_v3(&u, k))
-		return "unpack error";
+	if (bch2_alloc_unpack_v3(&u, k)) {
+		pr_buf(err, "unpack error");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
-const char *bch2_alloc_v4_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_alloc_v4_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
 {
-	struct bch_dev *ca;
-
-	if (k.k->p.inode >= c->sb.nr_devices ||
-	    !c->devs[k.k->p.inode])
-		return "invalid device";
-
-	ca = bch_dev_bkey_exists(c, k.k->p.inode);
-
-	if (k.k->p.offset < ca->mi.first_bucket ||
-	    k.k->p.offset >= ca->mi.nbuckets)
-		return "invalid bucket";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_alloc_v4)) {
+		pr_buf(err, "bad val size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_alloc_v4));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_alloc_v4_swab(struct bkey_s k)
@@ -562,7 +548,8 @@ static int bch2_check_alloc_key(struct btree_trans *trans,
 				struct btree_iter *alloc_iter)
 {
 	struct bch_fs *c = trans->c;
-	struct btree_iter discard_iter, freespace_iter, lru_iter;
+	struct bch_dev *ca;
+	struct btree_iter discard_iter, freespace_iter;
 	struct bch_alloc_v4 a;
 	unsigned discard_key_type, freespace_key_type;
 	struct bkey_s_c alloc_k, k;
@@ -578,7 +565,16 @@ static int bch2_check_alloc_key(struct btree_trans *trans,
 	if (ret)
 		return ret;
 
+	if (fsck_err_on(!bch2_dev_bucket_exists(c, alloc_k.k->p), c,
+			"alloc key for invalid device or bucket"))
+		return bch2_btree_delete_at(trans, alloc_iter, 0);
+
+	ca = bch_dev_bkey_exists(c, alloc_k.k->p.inode);
+	if (!ca->mi.freespace_initialized)
+		return 0;
+
 	bch2_alloc_to_v4(alloc_k, &a);
+
 	discard_key_type = bucket_state(a) == BUCKET_need_discard
 		? KEY_TYPE_set : 0;
 	freespace_key_type = bucket_state(a) == BUCKET_free
@@ -588,8 +584,6 @@ static int bch2_check_alloc_key(struct btree_trans *trans,
 			     alloc_k.k->p, 0);
 	bch2_trans_iter_init(trans, &freespace_iter, BTREE_ID_freespace,
 			     alloc_freespace_pos(alloc_k.k->p, a), 0);
-	bch2_trans_iter_init(trans, &lru_iter, BTREE_ID_lru,
-			     POS(alloc_k.k->p.inode, a.io_time[READ]), 0);
 
 	k = bch2_btree_iter_peek_slot(&discard_iter);
 	ret = bkey_err(k);
@@ -613,8 +607,7 @@ static int bch2_check_alloc_key(struct btree_trans *trans,
 		update->k.type	= discard_key_type;
 		update->k.p	= discard_iter.pos;
 
-		ret =   bch2_trans_update(trans, &discard_iter, update, 0) ?:
-			bch2_trans_commit(trans, NULL, NULL, 0);
+		ret = bch2_trans_update(trans, &discard_iter, update, 0);
 		if (ret)
 			goto err;
 	}
@@ -643,65 +636,12 @@ static int bch2_check_alloc_key(struct btree_trans *trans,
 		update->k.p	= freespace_iter.pos;
 		bch2_key_resize(&update->k, 1);
 
-		ret   = bch2_trans_update(trans, &freespace_iter, update, 0) ?:
-			bch2_trans_commit(trans, NULL, NULL, 0);
+		ret = bch2_trans_update(trans, &freespace_iter, update, 0);
 		if (ret)
 			goto err;
 	}
-
-	if (bucket_state(a) == BUCKET_cached) {
-		k = bch2_btree_iter_peek_slot(&lru_iter);
-		ret = bkey_err(k);
-		if (ret)
-			goto err;
-
-		if (fsck_err_on(!a.io_time[READ], c,
-				"cached bucket with read_time 0\n"
-				"  %s",
-			(printbuf_reset(&buf),
-			 bch2_bkey_val_to_text(&buf, c, alloc_k), buf.buf)) ||
-		    fsck_err_on(k.k->type != KEY_TYPE_lru ||
-				le64_to_cpu(bkey_s_c_to_lru(k).v->idx) != alloc_k.k->p.offset, c,
-				"incorrect/missing lru entry\n"
-				"  %s\n"
-				"  %s",
-				(printbuf_reset(&buf),
-				 bch2_bkey_val_to_text(&buf, c, alloc_k), buf.buf),
-				(bch2_bkey_val_to_text(&buf2, c, k), buf2.buf))) {
-			u64 read_time = a.io_time[READ];
-
-			if (!a.io_time[READ])
-				a.io_time[READ] = atomic64_read(&c->io_clock[READ].now);
-
-			ret   = bch2_lru_change(trans,
-						alloc_k.k->p.inode,
-						alloc_k.k->p.offset,
-						0, &a.io_time[READ]);
-			if (ret)
-				goto err;
-
-			if (a.io_time[READ] != read_time) {
-				struct bkey_i_alloc_v4 *a_mut =
-					bch2_alloc_to_v4_mut(trans, alloc_k);
-				ret = PTR_ERR_OR_ZERO(a_mut);
-				if (ret)
-					goto err;
-
-				a_mut->v.io_time[READ] = a.io_time[READ];
-				ret = bch2_trans_update(trans, alloc_iter,
-							&a_mut->k_i, BTREE_TRIGGER_NORUN);
-				if (ret)
-					goto err;
-			}
-
-			ret = bch2_trans_commit(trans, NULL, NULL, 0);
-			if (ret)
-				goto err;
-		}
-	}
 err:
 fsck_err:
-	bch2_trans_iter_exit(trans, &lru_iter);
 	bch2_trans_iter_exit(trans, &freespace_iter);
 	bch2_trans_iter_exit(trans, &discard_iter);
 	printbuf_exit(&buf2);
@@ -709,21 +649,8 @@ fsck_err:
 	return ret;
 }
 
-static inline bool bch2_dev_bucket_exists(struct bch_fs *c, struct bpos pos)
-{
-	struct bch_dev *ca;
-
-	if (pos.inode >= c->sb.nr_devices || !c->devs[pos.inode])
-		return false;
-
-	ca = bch_dev_bkey_exists(c, pos.inode);
-	return pos.offset >= ca->mi.first_bucket &&
-		pos.offset < ca->mi.nbuckets;
-}
-
-static int bch2_check_freespace_key(struct btree_trans *trans,
-				    struct btree_iter *freespace_iter,
-				    bool initial)
+static int bch2_check_discard_freespace_key(struct btree_trans *trans,
+					    struct btree_iter *iter)
 {
 	struct bch_fs *c = trans->c;
 	struct btree_iter alloc_iter;
@@ -732,10 +659,13 @@ static int bch2_check_freespace_key(struct btree_trans *trans,
 	u64 genbits;
 	struct bpos pos;
 	struct bkey_i *update;
+	enum bucket_state state = iter->btree_id == BTREE_ID_need_discard
+		? BUCKET_need_discard
+		: BUCKET_free;
 	struct printbuf buf = PRINTBUF;
 	int ret;
 
-	freespace_k = bch2_btree_iter_peek(freespace_iter);
+	freespace_k = bch2_btree_iter_peek(iter);
 	if (!freespace_k.k)
 		return 1;
 
@@ -743,15 +673,16 @@ static int bch2_check_freespace_key(struct btree_trans *trans,
 	if (ret)
 		return ret;
 
-	pos = freespace_iter->pos;
+	pos = iter->pos;
 	pos.offset &= ~(~0ULL << 56);
-	genbits = freespace_iter->pos.offset & (~0ULL << 56);
+	genbits = iter->pos.offset & (~0ULL << 56);
 
 	bch2_trans_iter_init(trans, &alloc_iter, BTREE_ID_alloc, pos, 0);
 
 	if (fsck_err_on(!bch2_dev_bucket_exists(c, pos), c,
-			"%llu:%llu set in freespace btree but device or bucket does not exist",
-			pos.inode, pos.offset))
+			"%llu:%llu set in %s btree but device or bucket does not exist",
+			pos.inode, pos.offset,
+			bch2_btree_ids[iter->btree_id]))
 		goto delete;
 
 	k = bch2_btree_iter_peek_slot(&alloc_iter);
@@ -761,11 +692,13 @@ static int bch2_check_freespace_key(struct btree_trans *trans,
 
 	bch2_alloc_to_v4(k, &a);
 
-	if (fsck_err_on(bucket_state(a) != BUCKET_free ||
-			genbits != alloc_freespace_genbits(a), c,
-			"%s\n  incorrectly set in freespace index (free %u, genbits %llu should be %llu)",
+	if (fsck_err_on(bucket_state(a) != state ||
+			(state == BUCKET_free &&
+			 genbits != alloc_freespace_genbits(a)), c,
+			"%s\n  incorrectly set in %s index (free %u, genbits %llu should be %llu)",
 			(bch2_bkey_val_to_text(&buf, c, k), buf.buf),
-			bucket_state(a) == BUCKET_free,
+			bch2_btree_ids[iter->btree_id],
+			bucket_state(a) == state,
 			genbits >> 56, alloc_freespace_genbits(a) >> 56))
 		goto delete;
 out:
@@ -775,46 +708,54 @@ fsck_err:
 	printbuf_exit(&buf);
 	return ret;
 delete:
-	update = bch2_trans_kmalloc(trans, sizeof(*update));
-	ret = PTR_ERR_OR_ZERO(update);
-	if (ret)
-		goto err;
+	if (iter->btree_id == BTREE_ID_freespace) {
+		/* should probably add a helper for deleting extents */
+		update = bch2_trans_kmalloc(trans, sizeof(*update));
+		ret = PTR_ERR_OR_ZERO(update);
+		if (ret)
+			goto err;
 
-	bkey_init(&update->k);
-	update->k.p = freespace_iter->pos;
-	bch2_key_resize(&update->k, 1);
+		bkey_init(&update->k);
+		update->k.p = iter->pos;
+		bch2_key_resize(&update->k, 1);
 
-	ret   = bch2_trans_update(trans, freespace_iter, update, 0) ?:
-		bch2_trans_commit(trans, NULL, NULL, 0);
+		ret = bch2_trans_update(trans, iter, update, 0);
+	} else {
+		ret = bch2_btree_delete_at(trans, iter, 0);
+	}
 	goto out;
 }
 
-int bch2_check_alloc_info(struct bch_fs *c, bool initial)
+int bch2_check_alloc_info(struct bch_fs *c)
 {
 	struct btree_trans trans;
 	struct btree_iter iter;
 	struct bkey_s_c k;
-	int ret = 0, last_dev = -1;
+	int ret = 0;
 
 	bch2_trans_init(&trans, c, 0, 0);
 
 	for_each_btree_key(&trans, iter, BTREE_ID_alloc, POS_MIN,
 			   BTREE_ITER_PREFETCH, k, ret) {
-		if (k.k->p.inode != last_dev) {
-			struct bch_dev *ca = bch_dev_bkey_exists(c, k.k->p.inode);
-
-			if (!ca->mi.freespace_initialized) {
-				bch2_btree_iter_set_pos(&iter, POS(k.k->p.inode + 1, 0));
-				continue;
-			}
+		ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+			bch2_check_alloc_key(&trans, &iter));
+		if (ret)
+			break;
+	}
+	bch2_trans_iter_exit(&trans, &iter);
 
-			last_dev = k.k->p.inode;
-		}
+	if (ret)
+		goto err;
 
+	bch2_trans_iter_init(&trans, &iter, BTREE_ID_need_discard, POS_MIN,
+			     BTREE_ITER_PREFETCH);
+	while (1) {
 		ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-			bch2_check_alloc_key(&trans, &iter));
+			bch2_check_discard_freespace_key(&trans, &iter));
 		if (ret)
 			break;
+
+		bch2_btree_iter_set_pos(&iter, bpos_nosnap_successor(iter.pos));
 	}
 	bch2_trans_iter_exit(&trans, &iter);
 
@@ -825,7 +766,7 @@ int bch2_check_alloc_info(struct bch_fs *c, bool initial)
 			     BTREE_ITER_PREFETCH);
 	while (1) {
 		ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-			bch2_check_freespace_key(&trans, &iter, initial));
+			bch2_check_discard_freespace_key(&trans, &iter));
 		if (ret)
 			break;
 
@@ -837,6 +778,109 @@ err:
 	return ret < 0 ? ret : 0;
 }
 
+static int bch2_check_alloc_to_lru_ref(struct btree_trans *trans,
+				       struct btree_iter *alloc_iter)
+{
+	struct bch_fs *c = trans->c;
+	struct btree_iter lru_iter;
+	struct bch_alloc_v4 a;
+	struct bkey_s_c alloc_k, k;
+	struct printbuf buf = PRINTBUF;
+	struct printbuf buf2 = PRINTBUF;
+	int ret;
+
+	alloc_k = bch2_btree_iter_peek(alloc_iter);
+	if (!alloc_k.k)
+		return 0;
+
+	ret = bkey_err(alloc_k);
+	if (ret)
+		return ret;
+
+	bch2_alloc_to_v4(alloc_k, &a);
+
+	if (bucket_state(a) != BUCKET_cached)
+		return 0;
+
+	bch2_trans_iter_init(trans, &lru_iter, BTREE_ID_lru,
+			     POS(alloc_k.k->p.inode, a.io_time[READ]), 0);
+
+	k = bch2_btree_iter_peek_slot(&lru_iter);
+	ret = bkey_err(k);
+	if (ret)
+		goto err;
+
+	if (fsck_err_on(!a.io_time[READ], c,
+			"cached bucket with read_time 0\n"
+			"  %s",
+		(printbuf_reset(&buf),
+		 bch2_bkey_val_to_text(&buf, c, alloc_k), buf.buf)) ||
+	    fsck_err_on(k.k->type != KEY_TYPE_lru ||
+			le64_to_cpu(bkey_s_c_to_lru(k).v->idx) != alloc_k.k->p.offset, c,
+			"incorrect/missing lru entry\n"
+			"  %s\n"
+			"  %s",
+			(printbuf_reset(&buf),
+			 bch2_bkey_val_to_text(&buf, c, alloc_k), buf.buf),
+			(bch2_bkey_val_to_text(&buf2, c, k), buf2.buf))) {
+		u64 read_time = a.io_time[READ];
+
+		if (!a.io_time[READ])
+			a.io_time[READ] = atomic64_read(&c->io_clock[READ].now);
+
+		ret = bch2_lru_change(trans,
+				      alloc_k.k->p.inode,
+				      alloc_k.k->p.offset,
+				      0, &a.io_time[READ]);
+		if (ret)
+			goto err;
+
+		if (a.io_time[READ] != read_time) {
+			struct bkey_i_alloc_v4 *a_mut =
+				bch2_alloc_to_v4_mut(trans, alloc_k);
+			ret = PTR_ERR_OR_ZERO(a_mut);
+			if (ret)
+				goto err;
+
+			a_mut->v.io_time[READ] = a.io_time[READ];
+			ret = bch2_trans_update(trans, alloc_iter,
+						&a_mut->k_i, BTREE_TRIGGER_NORUN);
+			if (ret)
+				goto err;
+		}
+	}
+err:
+fsck_err:
+	bch2_trans_iter_exit(trans, &lru_iter);
+	printbuf_exit(&buf2);
+	printbuf_exit(&buf);
+	return ret;
+}
+
+int bch2_check_alloc_to_lru_refs(struct bch_fs *c)
+{
+	struct btree_trans trans;
+	struct btree_iter iter;
+	struct bkey_s_c k;
+	int ret = 0;
+
+	bch2_trans_init(&trans, c, 0, 0);
+
+	for_each_btree_key(&trans, iter, BTREE_ID_alloc, POS_MIN,
+			   BTREE_ITER_PREFETCH, k, ret) {
+		ret = __bch2_trans_do(&trans, NULL, NULL,
+				      BTREE_INSERT_NOFAIL|
+				      BTREE_INSERT_LAZY_RW,
+			bch2_check_alloc_to_lru_ref(&trans, &iter));
+		if (ret)
+			break;
+	}
+	bch2_trans_iter_exit(&trans, &iter);
+
+	bch2_trans_exit(&trans);
+	return ret < 0 ? ret : 0;
+}
+
 static int bch2_clear_need_discard(struct btree_trans *trans, struct bpos pos,
 				   struct bch_dev *ca, bool *discard_done)
 {
diff --git a/libbcachefs/alloc_background.h b/libbcachefs/alloc_background.h
index da1b650e..9c6a590f 100644
--- a/libbcachefs/alloc_background.h
+++ b/libbcachefs/alloc_background.h
@@ -11,6 +11,18 @@
 /* How out of date a pointer gen is allowed to be: */
 #define BUCKET_GC_GEN_MAX	96U
 
+static inline bool bch2_dev_bucket_exists(struct bch_fs *c, struct bpos pos)
+{
+	struct bch_dev *ca;
+
+	if (!bch2_dev_exists2(c, pos.inode))
+		return false;
+
+	ca = bch_dev_bkey_exists(c, pos.inode);
+	return pos.offset >= ca->mi.first_bucket &&
+		pos.offset < ca->mi.nbuckets;
+}
+
 static inline u8 alloc_gc_gen(struct bch_alloc_v4 a)
 {
 	return a.gen - a.oldest_gen;
@@ -66,10 +78,10 @@ int bch2_bucket_io_time_reset(struct btree_trans *, unsigned, size_t, int);
 
 #define ALLOC_SCAN_BATCH(ca)		max_t(size_t, 1, (ca)->mi.nbuckets >> 9)
 
-const char *bch2_alloc_v1_invalid(const struct bch_fs *, struct bkey_s_c);
-const char *bch2_alloc_v2_invalid(const struct bch_fs *, struct bkey_s_c);
-const char *bch2_alloc_v3_invalid(const struct bch_fs *, struct bkey_s_c);
-const char *bch2_alloc_v4_invalid(const struct bch_fs *, struct bkey_s_c k);
+int bch2_alloc_v1_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
+int bch2_alloc_v2_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
+int bch2_alloc_v3_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
+int bch2_alloc_v4_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_alloc_v4_swab(struct bkey_s);
 void bch2_alloc_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
@@ -113,7 +125,8 @@ int bch2_alloc_read(struct bch_fs *);
 
 int bch2_trans_mark_alloc(struct btree_trans *, struct bkey_s_c,
 			  struct bkey_i *, unsigned);
-int bch2_check_alloc_info(struct bch_fs *, bool);
+int bch2_check_alloc_info(struct bch_fs *);
+int bch2_check_alloc_to_lru_refs(struct bch_fs *);
 void bch2_do_discards(struct bch_fs *);
 
 static inline bool should_invalidate_buckets(struct bch_dev *ca)
diff --git a/libbcachefs/bkey_methods.c b/libbcachefs/bkey_methods.c
index 0eac86e5..574c668a 100644
--- a/libbcachefs/bkey_methods.c
+++ b/libbcachefs/bkey_methods.c
@@ -22,10 +22,10 @@ const char * const bch2_bkey_types[] = {
 	NULL
 };
 
-static const char *deleted_key_invalid(const struct bch_fs *c,
-					struct bkey_s_c k)
+static int deleted_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			       int rw, struct printbuf *err)
 {
-	return NULL;
+	return 0;
 }
 
 #define bch2_bkey_ops_deleted (struct bkey_ops) {	\
@@ -36,25 +36,32 @@ static const char *deleted_key_invalid(const struct bch_fs *c,
 	.key_invalid = deleted_key_invalid,		\
 }
 
-static const char *empty_val_key_invalid(const struct bch_fs *c, struct bkey_s_c k)
+static int empty_val_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
+				 int rw, struct printbuf *err)
 {
-	if (bkey_val_bytes(k.k))
-		return "value size should be zero";
+	if (bkey_val_bytes(k.k)) {
+		pr_buf(err, "incorrect value size (%zu != 0)",
+		       bkey_val_bytes(k.k));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 #define bch2_bkey_ops_error (struct bkey_ops) {		\
 	.key_invalid = empty_val_key_invalid,		\
 }
 
-static const char *key_type_cookie_invalid(const struct bch_fs *c,
-					   struct bkey_s_c k)
+static int key_type_cookie_invalid(const struct bch_fs *c, struct bkey_s_c k,
+				   int rw, struct printbuf *err)
 {
-	if (bkey_val_bytes(k.k) != sizeof(struct bch_cookie))
-		return "incorrect value size";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_cookie)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_cookie));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 #define bch2_bkey_ops_cookie (struct bkey_ops) {	\
@@ -65,10 +72,10 @@ static const char *key_type_cookie_invalid(const struct bch_fs *c,
 	.key_invalid = empty_val_key_invalid,		\
 }
 
-static const char *key_type_inline_data_invalid(const struct bch_fs *c,
-					   struct bkey_s_c k)
+static int key_type_inline_data_invalid(const struct bch_fs *c, struct bkey_s_c k,
+					int rw, struct printbuf *err)
 {
-	return NULL;
+	return 0;
 }
 
 static void key_type_inline_data_to_text(struct printbuf *out, struct bch_fs *c,
@@ -86,11 +93,16 @@ static void key_type_inline_data_to_text(struct printbuf *out, struct bch_fs *c,
 	.val_to_text	= key_type_inline_data_to_text,	\
 }
 
-static const char *key_type_set_invalid(const struct bch_fs *c, struct bkey_s_c k)
+static int key_type_set_invalid(const struct bch_fs *c, struct bkey_s_c k,
+				int rw, struct printbuf *err)
 {
-	if (bkey_val_bytes(k.k))
-		return "nonempty value";
-	return NULL;
+	if (bkey_val_bytes(k.k)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_cookie));
+		return -EINVAL;
+	}
+
+	return 0;
 }
 
 static bool key_type_set_merge(struct bch_fs *c, struct bkey_s l, struct bkey_s_c r)
@@ -110,12 +122,15 @@ const struct bkey_ops bch2_bkey_ops[] = {
 #undef x
 };
 
-const char *bch2_bkey_val_invalid(struct bch_fs *c, struct bkey_s_c k)
+int bch2_bkey_val_invalid(struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
 {
-	if (k.k->type >= KEY_TYPE_MAX)
-		return "invalid type";
+	if (k.k->type >= KEY_TYPE_MAX) {
+		pr_buf(err, "invalid type (%u >= %u)", k.k->type, KEY_TYPE_MAX);
+		return -EINVAL;
+	}
 
-	return bch2_bkey_ops[k.k->type].key_invalid(c, k);
+	return bch2_bkey_ops[k.k->type].key_invalid(c, k, rw, err);
 }
 
 static unsigned bch2_key_types_allowed[] = {
@@ -182,63 +197,84 @@ static unsigned bch2_key_types_allowed[] = {
 		(1U << KEY_TYPE_btree_ptr_v2),
 };
 
-const char *__bch2_bkey_invalid(struct bch_fs *c, struct bkey_s_c k,
-				enum btree_node_type type)
+int __bch2_bkey_invalid(struct bch_fs *c, struct bkey_s_c k,
+			enum btree_node_type type,
+			int rw, struct printbuf *err)
 {
-	if (k.k->u64s < BKEY_U64s)
-		return "u64s too small";
-
-	if (!(bch2_key_types_allowed[type] & (1U << k.k->type)))
-		return "invalid key type for this btree";
+	if (k.k->u64s < BKEY_U64s) {
+		pr_buf(err, "u64s too small (%u < %zu)", k.k->u64s, BKEY_U64s);
+		return -EINVAL;
+	}
 
-	if (type == BKEY_TYPE_btree &&
-	    bkey_val_u64s(k.k) > BKEY_BTREE_PTR_VAL_U64s_MAX)
-		return "value too big";
+	if (!(bch2_key_types_allowed[type] & (1U << k.k->type))) {
+		pr_buf(err, "invalid key type for this btree (%s)",
+		       bch2_bkey_types[type]);
+		return -EINVAL;
+	}
 
 	if (btree_node_type_is_extents(type) && !bkey_whiteout(k.k)) {
-		if (k.k->size == 0)
-			return "bad size field";
+		if (k.k->size == 0) {
+			pr_buf(err, "size == 0");
+			return -EINVAL;
+		}
 
-		if (k.k->size > k.k->p.offset)
-			return "size greater than offset";
+		if (k.k->size > k.k->p.offset) {
+			pr_buf(err, "size greater than offset (%u > %llu)",
+			       k.k->size, k.k->p.offset);
+			return -EINVAL;
+		}
 	} else {
-		if (k.k->size)
-			return "nonzero size field";
+		if (k.k->size) {
+			pr_buf(err, "size != 0");
+			return -EINVAL;
+		}
 	}
 
 	if (type != BKEY_TYPE_btree &&
 	    !btree_type_has_snapshots(type) &&
-	    k.k->p.snapshot)
-		return "nonzero snapshot";
+	    k.k->p.snapshot) {
+		pr_buf(err, "nonzero snapshot");
+		return -EINVAL;
+	}
 
 	if (type != BKEY_TYPE_btree &&
 	    btree_type_has_snapshots(type) &&
-	    !k.k->p.snapshot)
-		return "invalid snapshot field";
+	    !k.k->p.snapshot) {
+		pr_buf(err, "snapshot == 0");
+		return -EINVAL;
+	}
 
 	if (type != BKEY_TYPE_btree &&
-	    !bkey_cmp(k.k->p, POS_MAX))
-		return "POS_MAX key";
+	    !bkey_cmp(k.k->p, POS_MAX)) {
+		pr_buf(err, "key at POS_MAX");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
-const char *bch2_bkey_invalid(struct bch_fs *c, struct bkey_s_c k,
-			      enum btree_node_type type)
+int bch2_bkey_invalid(struct bch_fs *c, struct bkey_s_c k,
+		      enum btree_node_type type,
+		      int rw, struct printbuf *err)
 {
-	return __bch2_bkey_invalid(c, k, type) ?:
-		bch2_bkey_val_invalid(c, k);
+	return __bch2_bkey_invalid(c, k, type, rw, err) ?:
+		bch2_bkey_val_invalid(c, k, rw, err);
 }
 
-const char *bch2_bkey_in_btree_node(struct btree *b, struct bkey_s_c k)
+int bch2_bkey_in_btree_node(struct btree *b, struct bkey_s_c k,
+			    struct printbuf *err)
 {
-	if (bpos_cmp(k.k->p, b->data->min_key) < 0)
-		return "key before start of btree node";
+	if (bpos_cmp(k.k->p, b->data->min_key) < 0) {
+		pr_buf(err, "key before start of btree node");
+		return -EINVAL;
+	}
 
-	if (bpos_cmp(k.k->p, b->data->max_key) > 0)
-		return "key past end of btree node";
+	if (bpos_cmp(k.k->p, b->data->max_key) > 0) {
+		pr_buf(err, "key past end of btree node");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_bpos_to_text(struct printbuf *out, struct bpos pos)
diff --git a/libbcachefs/bkey_methods.h b/libbcachefs/bkey_methods.h
index 2289a09d..48891775 100644
--- a/libbcachefs/bkey_methods.h
+++ b/libbcachefs/bkey_methods.h
@@ -12,10 +12,16 @@ enum btree_node_type;
 
 extern const char * const bch2_bkey_types[];
 
+/*
+ * key_invalid: checks validity of @k, returns 0 if good or -EINVAL if bad. If
+ * invalid, entire key will be deleted.
+ *
+ * When invalid, error string is returned via @err. @rw indicates whether key is
+ * being read or written; more aggressive checks can be enabled when rw == WRITE.
+*/
 struct bkey_ops {
-	/* Returns reason for being invalid if invalid, else NULL: */
-	const char *	(*key_invalid)(const struct bch_fs *,
-				       struct bkey_s_c);
+	int		(*key_invalid)(const struct bch_fs *c, struct bkey_s_c k,
+				       int rw, struct printbuf *err);
 	void		(*val_to_text)(struct printbuf *, struct bch_fs *,
 				       struct bkey_s_c);
 	void		(*swab)(struct bkey_s);
@@ -32,12 +38,12 @@ struct bkey_ops {
 
 extern const struct bkey_ops bch2_bkey_ops[];
 
-const char *bch2_bkey_val_invalid(struct bch_fs *, struct bkey_s_c);
-const char *__bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
-				enum btree_node_type);
-const char *bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
-			      enum btree_node_type);
-const char *bch2_bkey_in_btree_node(struct btree *, struct bkey_s_c);
+int bch2_bkey_val_invalid(struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
+int __bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
+			enum btree_node_type, int, struct printbuf *);
+int bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
+		      enum btree_node_type, int, struct printbuf *);
+int bch2_bkey_in_btree_node(struct btree *, struct bkey_s_c, struct printbuf *);
 
 void bch2_bpos_to_text(struct printbuf *, struct bpos);
 void bch2_bkey_to_text(struct printbuf *, const struct bkey *);
diff --git a/libbcachefs/btree_io.c b/libbcachefs/btree_io.c
index 4b880ea5..a38561c7 100644
--- a/libbcachefs/btree_io.c
+++ b/libbcachefs/btree_io.c
@@ -767,14 +767,23 @@ fsck_err:
 	return ret;
 }
 
+static int bset_key_invalid(struct bch_fs *c, struct btree *b,
+			    struct bkey_s_c k,
+			    bool updated_range, int rw,
+			    struct printbuf *err)
+{
+	return __bch2_bkey_invalid(c, k, btree_node_type(b), rw, err) ?:
+		(!updated_range ? bch2_bkey_in_btree_node(b, k, err) : 0) ?:
+		(rw == WRITE ? bch2_bkey_val_invalid(c, k, rw, err) : 0);
+}
+
 static int validate_bset_keys(struct bch_fs *c, struct btree *b,
 			 struct bset *i, unsigned *whiteout_u64s,
 			 int write, bool have_retry)
 {
 	unsigned version = le16_to_cpu(i->version);
 	struct bkey_packed *k, *prev = NULL;
-	struct printbuf buf1 = PRINTBUF;
-	struct printbuf buf2 = PRINTBUF;
+	struct printbuf buf = PRINTBUF;
 	bool updated_range = b->key.k.type == KEY_TYPE_btree_ptr_v2 &&
 		BTREE_PTR_RANGE_UPDATED(&bkey_i_to_btree_ptr_v2(&b->key)->v);
 	int ret = 0;
@@ -783,7 +792,6 @@ static int validate_bset_keys(struct bch_fs *c, struct btree *b,
 	     k != vstruct_last(i);) {
 		struct bkey_s u;
 		struct bkey tmp;
-		const char *invalid;
 
 		if (btree_err_on(bkey_next(k) > vstruct_last(i),
 				 BTREE_ERR_FIXABLE, c, NULL, b, i,
@@ -809,14 +817,15 @@ static int validate_bset_keys(struct bch_fs *c, struct btree *b,
 
 		u = __bkey_disassemble(b, k, &tmp);
 
-		invalid = __bch2_bkey_invalid(c, u.s_c, btree_node_type(b)) ?:
-			(!updated_range ?  bch2_bkey_in_btree_node(b, u.s_c) : NULL) ?:
-			(write ? bch2_bkey_val_invalid(c, u.s_c) : NULL);
-		if (invalid) {
-			printbuf_reset(&buf1);
-			bch2_bkey_val_to_text(&buf1, c, u.s_c);
-			btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i,
-				  "invalid bkey: %s\n%s", invalid, buf1.buf);
+		printbuf_reset(&buf);
+		if (bset_key_invalid(c, b, u.s_c, updated_range, write, &buf)) {
+			printbuf_reset(&buf);
+			pr_buf(&buf, "invalid bkey:\n  ");
+			bch2_bkey_val_to_text(&buf, c, u.s_c);
+			pr_buf(&buf, "  \n");
+			bset_key_invalid(c, b, u.s_c, updated_range, write, &buf);
+
+			btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i, "%s", buf.buf);
 
 			i->u64s = cpu_to_le16(le16_to_cpu(i->u64s) - k->u64s);
 			memmove_u64s_down(k, bkey_next(k),
@@ -832,16 +841,15 @@ static int validate_bset_keys(struct bch_fs *c, struct btree *b,
 		if (prev && bkey_iter_cmp(b, prev, k) > 0) {
 			struct bkey up = bkey_unpack_key(b, prev);
 
-			printbuf_reset(&buf1);
-			bch2_bkey_to_text(&buf1, &up);
-			printbuf_reset(&buf2);
-			bch2_bkey_to_text(&buf2, u.k);
+			printbuf_reset(&buf);
+			pr_buf(&buf, "keys out of order: ");
+			bch2_bkey_to_text(&buf, &up);
+			pr_buf(&buf, " > ");
+			bch2_bkey_to_text(&buf, u.k);
 
 			bch2_dump_bset(c, b, i, 0);
 
-			if (btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i,
-				      "keys out of order: %s > %s",
-				      buf1.buf, buf2.buf)) {
+			if (btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i, "%s", buf.buf)) {
 				i->u64s = cpu_to_le16(le16_to_cpu(i->u64s) - k->u64s);
 				memmove_u64s_down(k, bkey_next(k),
 						  (u64 *) vstruct_end(i) - (u64 *) k);
@@ -853,8 +861,7 @@ static int validate_bset_keys(struct bch_fs *c, struct btree *b,
 		k = bkey_next(k);
 	}
 fsck_err:
-	printbuf_exit(&buf2);
-	printbuf_exit(&buf1);
+	printbuf_exit(&buf);
 	return ret;
 }
 
@@ -873,6 +880,7 @@ int bch2_btree_node_read_done(struct bch_fs *c, struct bch_dev *ca,
 	unsigned u64s;
 	unsigned blacklisted_written, nonblacklisted_written = 0;
 	unsigned ptr_written = btree_ptr_sectors_written(&b->key);
+	struct printbuf buf = PRINTBUF;
 	int ret, retry_read = 0, write = READ;
 
 	b->version_ondisk = U16_MAX;
@@ -1065,17 +1073,20 @@ int bch2_btree_node_read_done(struct bch_fs *c, struct bch_dev *ca,
 	for (k = i->start; k != vstruct_last(i);) {
 		struct bkey tmp;
 		struct bkey_s u = __bkey_disassemble(b, k, &tmp);
-		const char *invalid = bch2_bkey_val_invalid(c, u.s_c);
 
-		if (invalid ||
+		printbuf_reset(&buf);
+
+		if (bch2_bkey_val_invalid(c, u.s_c, READ, &buf) ||
 		    (bch2_inject_invalid_keys &&
 		     !bversion_cmp(u.k->version, MAX_VERSION))) {
-			struct printbuf buf = PRINTBUF;
+			printbuf_reset(&buf);
 
+			pr_buf(&buf, "invalid bkey\n  ");
 			bch2_bkey_val_to_text(&buf, c, u.s_c);
-			btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i,
-				  "invalid bkey %s: %s", buf.buf, invalid);
-			printbuf_exit(&buf);
+			pr_buf(&buf, "\n  ");
+			bch2_bkey_val_invalid(c, u.s_c, READ, &buf);
+
+			btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i, "%s", buf.buf);
 
 			btree_keys_account_key_drop(&b->nr, 0, k);
 
@@ -1112,6 +1123,7 @@ int bch2_btree_node_read_done(struct bch_fs *c, struct bch_dev *ca,
 		set_btree_node_need_rewrite(b);
 out:
 	mempool_free(iter, &c->fill_iter);
+	printbuf_exit(&buf);
 	return retry_read;
 fsck_err:
 	if (ret == BTREE_RETRY_READ) {
@@ -1719,10 +1731,17 @@ static int validate_bset_for_write(struct bch_fs *c, struct btree *b,
 				   struct bset *i, unsigned sectors)
 {
 	unsigned whiteout_u64s = 0;
+	struct printbuf buf = PRINTBUF;
 	int ret;
 
-	if (bch2_bkey_invalid(c, bkey_i_to_s_c(&b->key), BKEY_TYPE_btree))
-		return -1;
+	ret = bch2_bkey_invalid(c, bkey_i_to_s_c(&b->key),
+				BKEY_TYPE_btree, WRITE, &buf);
+
+	if (ret)
+		bch2_fs_inconsistent(c, "invalid btree node key before write: %s", buf.buf);
+	printbuf_exit(&buf);
+	if (ret)
+		return ret;
 
 	ret = validate_bset_keys(c, b, i, &whiteout_u64s, WRITE, false) ?:
 		validate_bset(c, NULL, b, i, b->written, sectors, WRITE, false);
diff --git a/libbcachefs/btree_update_interior.c b/libbcachefs/btree_update_interior.c
index 42ae3b0c..b1aa77b8 100644
--- a/libbcachefs/btree_update_interior.c
+++ b/libbcachefs/btree_update_interior.c
@@ -1167,7 +1167,7 @@ static void bch2_insert_fixup_btree_ptr(struct btree_update *as,
 {
 	struct bch_fs *c = as->c;
 	struct bkey_packed *k;
-	const char *invalid;
+	struct printbuf buf = PRINTBUF;
 
 	BUG_ON(insert->k.type == KEY_TYPE_btree_ptr_v2 &&
 	       !btree_ptr_sectors_written(insert));
@@ -1175,14 +1175,18 @@ static void bch2_insert_fixup_btree_ptr(struct btree_update *as,
 	if (unlikely(!test_bit(JOURNAL_REPLAY_DONE, &c->journal.flags)))
 		bch2_journal_key_overwritten(c, b->c.btree_id, b->c.level, insert->k.p);
 
-	invalid = bch2_bkey_invalid(c, bkey_i_to_s_c(insert), btree_node_type(b)) ?:
-		bch2_bkey_in_btree_node(b, bkey_i_to_s_c(insert));
-	if (invalid) {
-		struct printbuf buf = PRINTBUF;
-
+	if (bch2_bkey_invalid(c, bkey_i_to_s_c(insert),
+			      btree_node_type(b), WRITE, &buf) ?:
+	    bch2_bkey_in_btree_node(b, bkey_i_to_s_c(insert), &buf)) {
+		printbuf_reset(&buf);
+		pr_buf(&buf, "inserting invalid bkey\n  ");
 		bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(insert));
-		bch2_fs_inconsistent(c, "inserting invalid bkey %s: %s", buf.buf, invalid);
-		printbuf_exit(&buf);
+		pr_buf(&buf, "\n  ");
+		bch2_bkey_invalid(c, bkey_i_to_s_c(insert),
+				  btree_node_type(b), WRITE, &buf);
+		bch2_bkey_in_btree_node(b, bkey_i_to_s_c(insert), &buf);
+
+		bch2_fs_inconsistent(c, "%s", buf.buf);
 		dump_stack();
 	}
 
@@ -1202,6 +1206,8 @@ static void bch2_insert_fixup_btree_ptr(struct btree_update *as,
 	bch2_btree_bset_insert_key(trans, path, b, node_iter, insert);
 	set_btree_node_dirty_acct(c, b);
 	set_btree_node_need_write(b);
+
+	printbuf_exit(&buf);
 }
 
 static void
diff --git a/libbcachefs/btree_update_leaf.c b/libbcachefs/btree_update_leaf.c
index a0480c63..5427d0bd 100644
--- a/libbcachefs/btree_update_leaf.c
+++ b/libbcachefs/btree_update_leaf.c
@@ -856,23 +856,33 @@ static inline int do_bch2_trans_commit(struct btree_trans *trans,
 {
 	struct bch_fs *c = trans->c;
 	struct btree_insert_entry *i;
+	struct printbuf buf = PRINTBUF;
 	int ret, u64s_delta = 0;
 
 	trans_for_each_update(trans, i) {
-		const char *invalid = bch2_bkey_invalid(c,
-				bkey_i_to_s_c(i->k), i->bkey_type);
-		if (invalid) {
-			struct printbuf buf = PRINTBUF;
+		if (bch2_bkey_invalid(c, bkey_i_to_s_c(i->k),
+				      i->bkey_type, WRITE, &buf)) {
+			printbuf_reset(&buf);
+			pr_buf(&buf, "invalid bkey on insert from %s -> %ps",
+			       trans->fn, (void *) i->ip_allocated);
+			pr_newline(&buf);
+			pr_indent_push(&buf, 2);
 
 			bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(i->k));
-			bch2_fs_fatal_error(c, "invalid bkey %s on insert from %s -> %ps: %s\n",
-					    buf.buf, trans->fn, (void *) i->ip_allocated, invalid);
+			pr_newline(&buf);
+
+			bch2_bkey_invalid(c, bkey_i_to_s_c(i->k),
+					  i->bkey_type, WRITE, &buf);
+
+			bch2_fs_fatal_error(c, "%s", buf.buf);
 			printbuf_exit(&buf);
 			return -EINVAL;
 		}
 		btree_insert_entry_checks(trans, i);
 	}
 
+	printbuf_exit(&buf);
+
 	trans_for_each_update(trans, i) {
 		if (i->cached)
 			continue;
@@ -1695,10 +1705,9 @@ retry:
 				break;
 		}
 
-		ret   = bch2_trans_update(trans, &iter, &delete, 0) ?:
+		ret   = bch2_trans_update(trans, &iter, &delete, update_flags) ?:
 			bch2_trans_commit(trans, &disk_res, journal_seq,
-					  BTREE_INSERT_NOFAIL|
-					  update_flags);
+					  BTREE_INSERT_NOFAIL);
 		bch2_disk_reservation_put(trans->c, &disk_res);
 		if (ret)
 			break;
diff --git a/libbcachefs/buckets.c b/libbcachefs/buckets.c
index 51ed9609..9513ee34 100644
--- a/libbcachefs/buckets.c
+++ b/libbcachefs/buckets.c
@@ -507,14 +507,9 @@ int bch2_mark_alloc(struct btree_trans *trans,
 	u64 journal_seq = trans->journal_res.seq;
 	struct bch_fs *c = trans->c;
 	struct bch_alloc_v4 old_a, new_a;
-	struct bch_dev *ca = bch_dev_bkey_exists(c, new.k->p.inode);
+	struct bch_dev *ca;
 	int ret = 0;
 
-	if (bch2_trans_inconsistent_on(new.k->p.offset < ca->mi.first_bucket ||
-				       new.k->p.offset >= ca->mi.nbuckets, trans,
-				       "alloc key outside range of device's buckets"))
-		return -EIO;
-
 	/*
 	 * alloc btree is read in by bch2_alloc_read, not gc:
 	 */
@@ -522,6 +517,12 @@ int bch2_mark_alloc(struct btree_trans *trans,
 	    !(flags & BTREE_TRIGGER_BUCKET_INVALIDATE))
 		return 0;
 
+	if (bch2_trans_inconsistent_on(!bch2_dev_bucket_exists(c, new.k->p), trans,
+				       "alloc key for invalid device or bucket"))
+		return -EIO;
+
+	ca = bch_dev_bkey_exists(c, new.k->p.inode);
+
 	bch2_alloc_to_v4(old, &old_a);
 	bch2_alloc_to_v4(new, &new_a);
 
diff --git a/libbcachefs/buckets.h b/libbcachefs/buckets.h
index 656a04b5..85e86ded 100644
--- a/libbcachefs/buckets.h
+++ b/libbcachefs/buckets.h
@@ -9,6 +9,7 @@
 #define _BUCKETS_H
 
 #include "buckets_types.h"
+#include "extents.h"
 #include "super.h"
 
 #define for_each_bucket(_b, _buckets)				\
@@ -83,8 +84,7 @@ static inline struct bucket *PTR_GC_BUCKET(struct bch_dev *ca,
 static inline enum bch_data_type ptr_data_type(const struct bkey *k,
 					       const struct bch_extent_ptr *ptr)
 {
-	if (k->type == KEY_TYPE_btree_ptr ||
-	    k->type == KEY_TYPE_btree_ptr_v2)
+	if (bkey_is_btree_ptr(k))
 		return BCH_DATA_btree;
 
 	return ptr->cached ? BCH_DATA_cached : BCH_DATA_user;
diff --git a/libbcachefs/dirent.c b/libbcachefs/dirent.c
index 760e4f74..28195988 100644
--- a/libbcachefs/dirent.c
+++ b/libbcachefs/dirent.c
@@ -83,38 +83,58 @@ const struct bch_hash_desc bch2_dirent_hash_desc = {
 	.is_visible	= dirent_is_visible,
 };
 
-const char *bch2_dirent_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_dirent_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			int rw, struct printbuf *err)
 {
 	struct bkey_s_c_dirent d = bkey_s_c_to_dirent(k);
 	unsigned len;
 
-	if (bkey_val_bytes(k.k) < sizeof(struct bch_dirent))
-		return "value too small";
+	if (bkey_val_bytes(k.k) < sizeof(struct bch_dirent)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(*d.v));
+		return -EINVAL;
+	}
 
 	len = bch2_dirent_name_bytes(d);
-	if (!len)
-		return "empty name";
+	if (!len) {
+		pr_buf(err, "empty name");
+		return -EINVAL;
+	}
 
-	if (bkey_val_u64s(k.k) > dirent_val_u64s(len))
-		return "value too big";
+	if (bkey_val_u64s(k.k) > dirent_val_u64s(len)) {
+		pr_buf(err, "value too big (%zu > %u)",
+		       bkey_val_u64s(k.k),dirent_val_u64s(len));
+		return -EINVAL;
+	}
 
-	if (len > BCH_NAME_MAX)
-		return "dirent name too big";
+	if (len > BCH_NAME_MAX) {
+		pr_buf(err, "dirent name too big (%u > %lu)",
+		       len, BCH_NAME_MAX);
+		return -EINVAL;
+	}
 
-	if (len == 1 && !memcmp(d.v->d_name, ".", 1))
-		return "invalid name";
+	if (len == 1 && !memcmp(d.v->d_name, ".", 1)) {
+		pr_buf(err, "invalid name");
+		return -EINVAL;
+	}
 
-	if (len == 2 && !memcmp(d.v->d_name, "..", 2))
-		return "invalid name";
+	if (len == 2 && !memcmp(d.v->d_name, "..", 2)) {
+		pr_buf(err, "invalid name");
+		return -EINVAL;
+	}
 
-	if (memchr(d.v->d_name, '/', len))
-		return "invalid name";
+	if (memchr(d.v->d_name, '/', len)) {
+		pr_buf(err, "invalid name");
+		return -EINVAL;
+	}
 
 	if (d.v->d_type != DT_SUBVOL &&
-	    le64_to_cpu(d.v->d_inum) == d.k->p.inode)
-		return "dirent points to own directory";
+	    le64_to_cpu(d.v->d_inum) == d.k->p.inode) {
+		pr_buf(err, "dirent points to own directory");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_dirent_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/dirent.h b/libbcachefs/dirent.h
index 1bb4d802..b1466932 100644
--- a/libbcachefs/dirent.h
+++ b/libbcachefs/dirent.h
@@ -6,7 +6,7 @@
 
 extern const struct bch_hash_desc bch2_dirent_hash_desc;
 
-const char *bch2_dirent_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_dirent_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_dirent_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_dirent (struct bkey_ops) {	\
diff --git a/libbcachefs/ec.c b/libbcachefs/ec.c
index 616a5512..a86b9748 100644
--- a/libbcachefs/ec.c
+++ b/libbcachefs/ec.c
@@ -102,24 +102,34 @@ struct ec_bio {
 
 /* Stripes btree keys: */
 
-const char *bch2_stripe_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_stripe_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			int rw, struct printbuf *err)
 {
 	const struct bch_stripe *s = bkey_s_c_to_stripe(k).v;
 
-	if (!bkey_cmp(k.k->p, POS_MIN))
-		return "stripe at pos 0";
+	if (!bkey_cmp(k.k->p, POS_MIN)) {
+		pr_buf(err, "stripe at POS_MIN");
+		return -EINVAL;
+	}
 
-	if (k.k->p.inode)
-		return "invalid stripe key";
+	if (k.k->p.inode) {
+		pr_buf(err, "nonzero inode field");
+		return -EINVAL;
+	}
 
-	if (bkey_val_bytes(k.k) < sizeof(*s))
-		return "incorrect value size";
+	if (bkey_val_bytes(k.k) < sizeof(*s)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(*s));
+		return -EINVAL;
+	}
 
-	if (bkey_val_bytes(k.k) < sizeof(*s) ||
-	    bkey_val_u64s(k.k) < stripe_val_u64s(s))
-		return "incorrect value size";
+	if (bkey_val_u64s(k.k) < stripe_val_u64s(s)) {
+		pr_buf(err, "incorrect value size (%zu < %u)",
+		       bkey_val_u64s(k.k), stripe_val_u64s(s));
+		return -EINVAL;
+	}
 
-	return bch2_bkey_ptrs_invalid(c, k);
+	return bch2_bkey_ptrs_invalid(c, k, rw, err);
 }
 
 void bch2_stripe_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/ec.h b/libbcachefs/ec.h
index 9d508a2f..af7f8eee 100644
--- a/libbcachefs/ec.h
+++ b/libbcachefs/ec.h
@@ -6,7 +6,8 @@
 #include "buckets_types.h"
 #include "keylist_types.h"
 
-const char *bch2_stripe_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_stripe_invalid(const struct bch_fs *, struct bkey_s_c,
+			int rw, struct printbuf *);
 void bch2_stripe_to_text(struct printbuf *, struct bch_fs *,
 			 struct bkey_s_c);
 
diff --git a/libbcachefs/extents.c b/libbcachefs/extents.c
index 77a0d49a..dffbcffa 100644
--- a/libbcachefs/extents.c
+++ b/libbcachefs/extents.c
@@ -156,12 +156,16 @@ int bch2_bkey_pick_read_device(struct bch_fs *c, struct bkey_s_c k,
 
 /* KEY_TYPE_btree_ptr: */
 
-const char *bch2_btree_ptr_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_btree_ptr_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			   int rw, struct printbuf *err)
 {
-	if (bkey_val_u64s(k.k) > BCH_REPLICAS_MAX)
-		return "value too big";
+	if (bkey_val_u64s(k.k) > BCH_REPLICAS_MAX) {
+		pr_buf(err, "value too big (%zu > %u)",
+		       bkey_val_u64s(k.k), BCH_REPLICAS_MAX);
+		return -EINVAL;
+	}
 
-	return bch2_bkey_ptrs_invalid(c, k);
+	return bch2_bkey_ptrs_invalid(c, k, rw, err);
 }
 
 void bch2_btree_ptr_to_text(struct printbuf *out, struct bch_fs *c,
@@ -170,25 +174,35 @@ void bch2_btree_ptr_to_text(struct printbuf *out, struct bch_fs *c,
 	bch2_bkey_ptrs_to_text(out, c, k);
 }
 
-const char *bch2_btree_ptr_v2_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_btree_ptr_v2_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			      int rw, struct printbuf *err)
 {
 	struct bkey_s_c_btree_ptr_v2 bp = bkey_s_c_to_btree_ptr_v2(k);
 
-	if (bkey_val_bytes(k.k) <= sizeof(*bp.v))
-		return "value too small";
+	if (bkey_val_bytes(k.k) <= sizeof(*bp.v)) {
+		pr_buf(err, "value too small (%zu <= %zu)",
+		       bkey_val_bytes(k.k), sizeof(*bp.v));
+		return -EINVAL;
+	}
 
-	if (bkey_val_u64s(k.k) > BKEY_BTREE_PTR_VAL_U64s_MAX)
-		return "value too big";
+	if (bkey_val_u64s(k.k) > BKEY_BTREE_PTR_VAL_U64s_MAX) {
+		pr_buf(err, "value too big (%zu > %zu)",
+		       bkey_val_u64s(k.k), BKEY_BTREE_PTR_VAL_U64s_MAX);
+		return -EINVAL;
+	}
 
 	if (c->sb.version < bcachefs_metadata_version_snapshot &&
-	    bp.v->min_key.snapshot)
-		return "invalid min_key.snapshot";
+	    bp.v->min_key.snapshot) {
+		pr_buf(err, "invalid min_key.snapshot (%u != 0)",
+		       bp.v->min_key.snapshot);
+		return -EINVAL;
+	}
 
-	return bch2_bkey_ptrs_invalid(c, k);
+	return bch2_bkey_ptrs_invalid(c, k, rw, err);
 }
 
 void bch2_btree_ptr_v2_to_text(struct printbuf *out, struct bch_fs *c,
-			    struct bkey_s_c k)
+			       struct bkey_s_c k)
 {
 	struct bkey_s_c_btree_ptr_v2 bp = bkey_s_c_to_btree_ptr_v2(k);
 
@@ -220,17 +234,6 @@ void bch2_btree_ptr_v2_compat(enum btree_id btree_id, unsigned version,
 
 /* KEY_TYPE_extent: */
 
-const char *bch2_extent_invalid(const struct bch_fs *c, struct bkey_s_c k)
-{
-	return bch2_bkey_ptrs_invalid(c, k);
-}
-
-void bch2_extent_to_text(struct printbuf *out, struct bch_fs *c,
-			 struct bkey_s_c k)
-{
-	bch2_bkey_ptrs_to_text(out, c, k);
-}
-
 bool bch2_extent_merge(struct bch_fs *c, struct bkey_s l, struct bkey_s_c r)
 {
 	struct bkey_ptrs   l_ptrs = bch2_bkey_ptrs(l);
@@ -363,17 +366,24 @@ bool bch2_extent_merge(struct bch_fs *c, struct bkey_s l, struct bkey_s_c r)
 
 /* KEY_TYPE_reservation: */
 
-const char *bch2_reservation_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_reservation_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			     int rw, struct printbuf *err)
 {
 	struct bkey_s_c_reservation r = bkey_s_c_to_reservation(k);
 
-	if (bkey_val_bytes(k.k) != sizeof(struct bch_reservation))
-		return "incorrect value size";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_reservation)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(*r.v));
+		return -EINVAL;
+	}
 
-	if (!r.v->nr_replicas || r.v->nr_replicas > BCH_REPLICAS_MAX)
-		return "invalid nr_replicas";
+	if (!r.v->nr_replicas || r.v->nr_replicas > BCH_REPLICAS_MAX) {
+		pr_buf(err, "invalid nr_replicas (%u)",
+		       r.v->nr_replicas);
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_reservation_to_text(struct printbuf *out, struct bch_fs *c,
@@ -1001,69 +1011,86 @@ void bch2_bkey_ptrs_to_text(struct printbuf *out, struct bch_fs *c,
 	}
 }
 
-static const char *extent_ptr_invalid(const struct bch_fs *c,
-				      struct bkey_s_c k,
-				      const struct bch_extent_ptr *ptr,
-				      unsigned size_ondisk,
-				      bool metadata)
+static int extent_ptr_invalid(const struct bch_fs *c,
+			      struct bkey_s_c k,
+			      const struct bch_extent_ptr *ptr,
+			      unsigned size_ondisk,
+			      bool metadata,
+			      struct printbuf *err)
 {
 	struct bkey_ptrs_c ptrs = bch2_bkey_ptrs_c(k);
 	const struct bch_extent_ptr *ptr2;
+	u64 bucket;
+	u32 bucket_offset;
 	struct bch_dev *ca;
 
-	if (!bch2_dev_exists2(c, ptr->dev))
-		return "pointer to invalid device";
+	if (!bch2_dev_exists2(c, ptr->dev)) {
+		pr_buf(err, "pointer to invalid device (%u)", ptr->dev);
+		return -EINVAL;
+	}
 
 	ca = bch_dev_bkey_exists(c, ptr->dev);
-	if (!ca)
-		return "pointer to invalid device";
-
 	bkey_for_each_ptr(ptrs, ptr2)
-		if (ptr != ptr2 && ptr->dev == ptr2->dev)
-			return "multiple pointers to same device";
+		if (ptr != ptr2 && ptr->dev == ptr2->dev) {
+			pr_buf(err, "multiple pointers to same device (%u)", ptr->dev);
+			return -EINVAL;
+		}
+
+	bucket = sector_to_bucket_and_offset(ca, ptr->offset, &bucket_offset);
 
-	if (ptr->offset + size_ondisk > bucket_to_sector(ca, ca->mi.nbuckets))
-		return "offset past end of device";
+	if (bucket >= ca->mi.nbuckets) {
+		pr_buf(err, "pointer past last bucket (%llu > %llu)",
+		       bucket, ca->mi.nbuckets);
+		return -EINVAL;
+	}
 
-	if (ptr->offset < bucket_to_sector(ca, ca->mi.first_bucket))
-		return "offset before first bucket";
+	if (ptr->offset < bucket_to_sector(ca, ca->mi.first_bucket)) {
+		pr_buf(err, "pointer before first bucket (%llu < %u)",
+		       bucket, ca->mi.first_bucket);
+		return -EINVAL;
+	}
 
-	if (bucket_remainder(ca, ptr->offset) +
-	    size_ondisk > ca->mi.bucket_size)
-		return "spans multiple buckets";
+	if (bucket_offset + size_ondisk > ca->mi.bucket_size) {
+		pr_buf(err, "pointer spans multiple buckets (%u + %u > %u)",
+		       bucket_offset, size_ondisk, ca->mi.bucket_size);
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
-const char *bch2_bkey_ptrs_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_bkey_ptrs_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			   int rw, struct printbuf *err)
 {
 	struct bkey_ptrs_c ptrs = bch2_bkey_ptrs_c(k);
-	struct bch_devs_list devs;
 	const union bch_extent_entry *entry;
 	struct bch_extent_crc_unpacked crc;
 	unsigned size_ondisk = k.k->size;
-	const char *reason;
 	unsigned nonce = UINT_MAX;
-	unsigned i;
+	int ret;
 
-	if (k.k->type == KEY_TYPE_btree_ptr ||
-	    k.k->type == KEY_TYPE_btree_ptr_v2)
+	if (bkey_is_btree_ptr(k.k))
 		size_ondisk = btree_sectors(c);
 
 	bkey_extent_entry_for_each(ptrs, entry) {
-		if (__extent_entry_type(entry) >= BCH_EXTENT_ENTRY_MAX)
-			return "invalid extent entry type";
+		if (__extent_entry_type(entry) >= BCH_EXTENT_ENTRY_MAX) {
+			pr_buf(err, "invalid extent entry type (got %u, max %u)",
+			       __extent_entry_type(entry), BCH_EXTENT_ENTRY_MAX);
+			return -EINVAL;
+		}
 
-		if (k.k->type == KEY_TYPE_btree_ptr &&
-		    !extent_entry_is_ptr(entry))
-			return "has non ptr field";
+		if (bkey_is_btree_ptr(k.k) &&
+		    !extent_entry_is_ptr(entry)) {
+			pr_buf(err, "has non ptr field");
+			return -EINVAL;
+		}
 
 		switch (extent_entry_type(entry)) {
 		case BCH_EXTENT_ENTRY_ptr:
-			reason = extent_ptr_invalid(c, k, &entry->ptr,
-						    size_ondisk, false);
-			if (reason)
-				return reason;
+			ret = extent_ptr_invalid(c, k, &entry->ptr, size_ondisk,
+						 false, err);
+			if (ret)
+				return ret;
 			break;
 		case BCH_EXTENT_ENTRY_crc32:
 		case BCH_EXTENT_ENTRY_crc64:
@@ -1071,22 +1098,30 @@ const char *bch2_bkey_ptrs_invalid(const struct bch_fs *c, struct bkey_s_c k)
 			crc = bch2_extent_crc_unpack(k.k, entry_to_crc(entry));
 
 			if (crc.offset + crc.live_size >
-			    crc.uncompressed_size)
-				return "checksum offset + key size > uncompressed size";
+			    crc.uncompressed_size) {
+				pr_buf(err, "checksum offset + key size > uncompressed size");
+				return -EINVAL;
+			}
 
 			size_ondisk = crc.compressed_size;
 
-			if (!bch2_checksum_type_valid(c, crc.csum_type))
-				return "invalid checksum type";
+			if (!bch2_checksum_type_valid(c, crc.csum_type)) {
+				pr_buf(err, "invalid checksum type");
+				return -EINVAL;
+			}
 
-			if (crc.compression_type >= BCH_COMPRESSION_TYPE_NR)
-				return "invalid compression type";
+			if (crc.compression_type >= BCH_COMPRESSION_TYPE_NR) {
+				pr_buf(err, "invalid compression type");
+				return -EINVAL;
+			}
 
 			if (bch2_csum_type_is_encryption(crc.csum_type)) {
 				if (nonce == UINT_MAX)
 					nonce = crc.offset + crc.nonce;
-				else if (nonce != crc.offset + crc.nonce)
-					return "incorrect nonce";
+				else if (nonce != crc.offset + crc.nonce) {
+					pr_buf(err, "incorrect nonce");
+					return -EINVAL;
+				}
 			}
 			break;
 		case BCH_EXTENT_ENTRY_stripe_ptr:
@@ -1094,13 +1129,7 @@ const char *bch2_bkey_ptrs_invalid(const struct bch_fs *c, struct bkey_s_c k)
 		}
 	}
 
-	devs = bch2_bkey_devs(k);
-	bubble_sort(devs.devs, devs.nr, u8_cmp);
-	for (i = 0; i + 1 < devs.nr; i++)
-		if (devs.devs[i] == devs.devs[i + 1])
-			return "multiple ptrs to same device";
-
-	return NULL;
+	return 0;
 }
 
 void bch2_ptr_swab(struct bkey_s k)
diff --git a/libbcachefs/extents.h b/libbcachefs/extents.h
index ae650849..4f41f0fd 100644
--- a/libbcachefs/extents.h
+++ b/libbcachefs/extents.h
@@ -367,13 +367,12 @@ int bch2_bkey_pick_read_device(struct bch_fs *, struct bkey_s_c,
 
 /* KEY_TYPE_btree_ptr: */
 
-const char *bch2_btree_ptr_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_btree_ptr_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_btree_ptr_to_text(struct printbuf *, struct bch_fs *,
 			    struct bkey_s_c);
 
-const char *bch2_btree_ptr_v2_invalid(const struct bch_fs *, struct bkey_s_c);
-void bch2_btree_ptr_v2_to_text(struct printbuf *, struct bch_fs *,
-			    struct bkey_s_c);
+int bch2_btree_ptr_v2_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
+void bch2_btree_ptr_v2_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 void bch2_btree_ptr_v2_compat(enum btree_id, unsigned, unsigned,
 			      int, struct bkey_s);
 
@@ -396,13 +395,11 @@ void bch2_btree_ptr_v2_compat(enum btree_id, unsigned, unsigned,
 
 /* KEY_TYPE_extent: */
 
-const char *bch2_extent_invalid(const struct bch_fs *, struct bkey_s_c);
-void bch2_extent_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 bool bch2_extent_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
 #define bch2_bkey_ops_extent (struct bkey_ops) {		\
-	.key_invalid	= bch2_extent_invalid,			\
-	.val_to_text	= bch2_extent_to_text,			\
+	.key_invalid	= bch2_bkey_ptrs_invalid,		\
+	.val_to_text	= bch2_bkey_ptrs_to_text,		\
 	.swab		= bch2_ptr_swab,			\
 	.key_normalize	= bch2_extent_normalize,		\
 	.key_merge	= bch2_extent_merge,			\
@@ -412,7 +409,8 @@ bool bch2_extent_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
 /* KEY_TYPE_reservation: */
 
-const char *bch2_reservation_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_reservation_invalid(const struct bch_fs *, struct bkey_s_c,
+			     int, struct printbuf *);
 void bch2_reservation_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 bool bch2_reservation_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
@@ -618,7 +616,8 @@ bool bch2_bkey_matches_ptr(struct bch_fs *, struct bkey_s_c,
 bool bch2_extent_normalize(struct bch_fs *, struct bkey_s);
 void bch2_bkey_ptrs_to_text(struct printbuf *, struct bch_fs *,
 			    struct bkey_s_c);
-const char *bch2_bkey_ptrs_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_bkey_ptrs_invalid(const struct bch_fs *, struct bkey_s_c,
+			   int, struct printbuf *);
 
 void bch2_ptr_swab(struct bkey_s);
 
diff --git a/libbcachefs/inode.c b/libbcachefs/inode.c
index 14b0b595..8a824897 100644
--- a/libbcachefs/inode.c
+++ b/libbcachefs/inode.c
@@ -293,76 +293,89 @@ int bch2_inode_write(struct btree_trans *trans,
 	return bch2_trans_update(trans, iter, &inode_p->inode.k_i, 0);
 }
 
-const char *bch2_inode_invalid(const struct bch_fs *c, struct bkey_s_c k)
+static int __bch2_inode_invalid(struct bkey_s_c k, struct printbuf *err)
 {
-	struct bkey_s_c_inode inode = bkey_s_c_to_inode(k);
 	struct bch_inode_unpacked unpacked;
 
-	if (k.k->p.inode)
-		return "nonzero k.p.inode";
-
-	if (bkey_val_bytes(k.k) < sizeof(struct bch_inode))
-		return "incorrect value size";
-
-	if (k.k->p.offset < BLOCKDEV_INODE_MAX)
-		return "fs inode in blockdev range";
+	if (k.k->p.inode) {
+		pr_buf(err, "nonzero k.p.inode");
+		return -EINVAL;
+	}
 
-	if (INODE_STR_HASH(inode.v) >= BCH_STR_HASH_NR)
-		return "invalid str hash type";
+	if (k.k->p.offset < BLOCKDEV_INODE_MAX) {
+		pr_buf(err, "fs inode in blockdev range");
+		return -EINVAL;
+	}
 
-	if (bch2_inode_unpack(k, &unpacked))
-		return "invalid variable length fields";
+	if (bch2_inode_unpack(k, &unpacked)){
+		pr_buf(err, "invalid variable length fields");
+		return -EINVAL;
+	}
 
-	if (unpacked.bi_data_checksum >= BCH_CSUM_OPT_NR + 1)
-		return "invalid data checksum type";
+	if (unpacked.bi_data_checksum >= BCH_CSUM_OPT_NR + 1) {
+		pr_buf(err, "invalid data checksum type (%u >= %u",
+			unpacked.bi_data_checksum, BCH_CSUM_OPT_NR + 1);
+		return -EINVAL;
+	}
 
-	if (unpacked.bi_compression >= BCH_COMPRESSION_OPT_NR + 1)
-		return "invalid data checksum type";
+	if (unpacked.bi_compression >= BCH_COMPRESSION_OPT_NR + 1) {
+		pr_buf(err, "invalid data checksum type (%u >= %u)",
+		       unpacked.bi_compression, BCH_COMPRESSION_OPT_NR + 1);
+		return -EINVAL;
+	}
 
 	if ((unpacked.bi_flags & BCH_INODE_UNLINKED) &&
-	    unpacked.bi_nlink != 0)
-		return "flagged as unlinked but bi_nlink != 0";
+	    unpacked.bi_nlink != 0) {
+		pr_buf(err, "flagged as unlinked but bi_nlink != 0");
+		return -EINVAL;
+	}
 
-	if (unpacked.bi_subvol && !S_ISDIR(unpacked.bi_mode))
-		return "subvolume root but not a directory";
+	if (unpacked.bi_subvol && !S_ISDIR(unpacked.bi_mode)) {
+		pr_buf(err, "subvolume root but not a directory");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
-const char *bch2_inode_v2_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_inode_invalid(const struct bch_fs *c, struct bkey_s_c k,
+		       int rw, struct printbuf *err)
 {
-	struct bkey_s_c_inode_v2 inode = bkey_s_c_to_inode_v2(k);
-	struct bch_inode_unpacked unpacked;
-
-	if (k.k->p.inode)
-		return "nonzero k.p.inode";
-
-	if (bkey_val_bytes(k.k) < sizeof(struct bch_inode))
-		return "incorrect value size";
-
-	if (k.k->p.offset < BLOCKDEV_INODE_MAX)
-		return "fs inode in blockdev range";
+	struct bkey_s_c_inode inode = bkey_s_c_to_inode(k);
 
-	if (INODEv2_STR_HASH(inode.v) >= BCH_STR_HASH_NR)
-		return "invalid str hash type";
+	if (bkey_val_bytes(k.k) < sizeof(*inode.v)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(*inode.v));
+		return -EINVAL;
+	}
 
-	if (bch2_inode_unpack(k, &unpacked))
-		return "invalid variable length fields";
+	if (INODE_STR_HASH(inode.v) >= BCH_STR_HASH_NR) {
+		pr_buf(err, "invalid str hash type (%llu >= %u)",
+		       INODE_STR_HASH(inode.v), BCH_STR_HASH_NR);
+		return -EINVAL;
+	}
 
-	if (unpacked.bi_data_checksum >= BCH_CSUM_OPT_NR + 1)
-		return "invalid data checksum type";
+	return __bch2_inode_invalid(k, err);
+}
 
-	if (unpacked.bi_compression >= BCH_COMPRESSION_OPT_NR + 1)
-		return "invalid data checksum type";
+int bch2_inode_v2_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
+{
+	struct bkey_s_c_inode_v2 inode = bkey_s_c_to_inode_v2(k);
 
-	if ((unpacked.bi_flags & BCH_INODE_UNLINKED) &&
-	    unpacked.bi_nlink != 0)
-		return "flagged as unlinked but bi_nlink != 0";
+	if (bkey_val_bytes(k.k) < sizeof(*inode.v)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(*inode.v));
+		return -EINVAL;
+	}
 
-	if (unpacked.bi_subvol && !S_ISDIR(unpacked.bi_mode))
-		return "subvolume root but not a directory";
+	if (INODEv2_STR_HASH(inode.v) >= BCH_STR_HASH_NR) {
+		pr_buf(err, "invalid str hash type (%llu >= %u)",
+		       INODEv2_STR_HASH(inode.v), BCH_STR_HASH_NR);
+		return -EINVAL;
+	}
 
-	return NULL;
+	return __bch2_inode_invalid(k, err);
 }
 
 static void __bch2_inode_unpacked_to_text(struct printbuf *out, struct bch_inode_unpacked *inode)
@@ -396,16 +409,21 @@ void bch2_inode_to_text(struct printbuf *out, struct bch_fs *c,
 	__bch2_inode_unpacked_to_text(out, &inode);
 }
 
-const char *bch2_inode_generation_invalid(const struct bch_fs *c,
-					  struct bkey_s_c k)
+int bch2_inode_generation_invalid(const struct bch_fs *c, struct bkey_s_c k,
+				  int rw, struct printbuf *err)
 {
-	if (k.k->p.inode)
-		return "nonzero k.p.inode";
+	if (k.k->p.inode) {
+		pr_buf(err, "nonzero k.p.inode");
+		return -EINVAL;
+	}
 
-	if (bkey_val_bytes(k.k) != sizeof(struct bch_inode_generation))
-		return "incorrect value size";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_inode_generation)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_inode_generation));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_inode_generation_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/inode.h b/libbcachefs/inode.h
index 2337ecfc..9442600a 100644
--- a/libbcachefs/inode.h
+++ b/libbcachefs/inode.h
@@ -6,8 +6,8 @@
 
 extern const char * const bch2_inode_opts[];
 
-const char *bch2_inode_invalid(const struct bch_fs *, struct bkey_s_c);
-const char *bch2_inode_v2_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_inode_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
+int bch2_inode_v2_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_inode_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_inode (struct bkey_ops) {		\
@@ -30,10 +30,9 @@ static inline bool bkey_is_inode(const struct bkey *k)
 		k->type == KEY_TYPE_inode_v2;
 }
 
-const char *bch2_inode_generation_invalid(const struct bch_fs *,
-					  struct bkey_s_c);
-void bch2_inode_generation_to_text(struct printbuf *, struct bch_fs *,
-				   struct bkey_s_c);
+int bch2_inode_generation_invalid(const struct bch_fs *, struct bkey_s_c,
+				  int, struct printbuf *);
+void bch2_inode_generation_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_inode_generation (struct bkey_ops) {	\
 	.key_invalid	= bch2_inode_generation_invalid,	\
diff --git a/libbcachefs/journal_io.c b/libbcachefs/journal_io.c
index e61b8893..cbde21a4 100644
--- a/libbcachefs/journal_io.c
+++ b/libbcachefs/journal_io.c
@@ -210,7 +210,7 @@ static int journal_validate_key(struct bch_fs *c, const char *where,
 				unsigned version, int big_endian, int write)
 {
 	void *next = vstruct_next(entry);
-	const char *invalid;
+	struct printbuf buf = PRINTBUF;
 	int ret = 0;
 
 	if (journal_entry_err_on(!k->k.u64s, c,
@@ -250,22 +250,28 @@ static int journal_validate_key(struct bch_fs *c, const char *where,
 		bch2_bkey_compat(level, btree_id, version, big_endian,
 				 write, NULL, bkey_to_packed(k));
 
-	invalid = bch2_bkey_invalid(c, bkey_i_to_s_c(k),
-				    __btree_node_type(level, btree_id));
-	if (invalid) {
-		struct printbuf buf = PRINTBUF;
+	if (bch2_bkey_invalid(c, bkey_i_to_s_c(k),
+			      __btree_node_type(level, btree_id), write, &buf)) {
+		printbuf_reset(&buf);
+		pr_buf(&buf, "invalid %s in %s entry offset %zi/%u:",
+		       type, where,
+		       (u64 *) k - entry->_data,
+		       le16_to_cpu(entry->u64s));
+		pr_newline(&buf);
+		pr_indent_push(&buf, 2);
 
 		bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(k));
-		mustfix_fsck_err(c, "invalid %s in %s entry offset %zi/%u: %s\n%s",
-				 type, where,
-				 (u64 *) k - entry->_data,
-				 le16_to_cpu(entry->u64s),
-				 invalid, buf.buf);
-		printbuf_exit(&buf);
+		pr_newline(&buf);
+		bch2_bkey_invalid(c, bkey_i_to_s_c(k),
+				  __btree_node_type(level, btree_id), write, &buf);
+
+		mustfix_fsck_err(c, "%s", buf.buf);
 
 		le16_add_cpu(&entry->u64s, -((u16) k->k.u64s));
 		memmove(k, bkey_next(k), next - (void *) bkey_next(k));
 		journal_entry_null_range(vstruct_next(entry), next);
+
+		printbuf_exit(&buf);
 		return FSCK_DELETED_KEY;
 	}
 
@@ -273,6 +279,7 @@ static int journal_validate_key(struct bch_fs *c, const char *where,
 		bch2_bkey_compat(level, btree_id, version, big_endian,
 				 write, NULL, bkey_to_packed(k));
 fsck_err:
+	printbuf_exit(&buf);
 	return ret;
 }
 
diff --git a/libbcachefs/lru.c b/libbcachefs/lru.c
index 4f0e6960..c6f43315 100644
--- a/libbcachefs/lru.c
+++ b/libbcachefs/lru.c
@@ -8,14 +8,18 @@
 #include "lru.h"
 #include "recovery.h"
 
-const char *bch2_lru_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_lru_invalid(const struct bch_fs *c, struct bkey_s_c k,
+		     int rw, struct printbuf *err)
 {
 	const struct bch_lru *lru = bkey_s_c_to_lru(k).v;
 
-	if (bkey_val_bytes(k.k) < sizeof(*lru))
-		return "incorrect value size";
+	if (bkey_val_bytes(k.k) < sizeof(*lru)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(*lru));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_lru_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/lru.h b/libbcachefs/lru.h
index 4db6a839..e8f50817 100644
--- a/libbcachefs/lru.h
+++ b/libbcachefs/lru.h
@@ -2,7 +2,7 @@
 #ifndef _BCACHEFS_LRU_H
 #define _BCACHEFS_LRU_H
 
-const char *bch2_lru_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_lru_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_lru_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_lru (struct bkey_ops) {	\
diff --git a/libbcachefs/movinggc.c b/libbcachefs/movinggc.c
index cb6b8167..dd1bf665 100644
--- a/libbcachefs/movinggc.c
+++ b/libbcachefs/movinggc.c
@@ -290,10 +290,10 @@ static int bch2_copygc(struct bch_fs *c)
 			     writepoint_ptr(&c->copygc_write_point),
 			     copygc_pred, NULL,
 			     &move_stats);
-	if (ret) {
+	if (ret < 0)
 		bch_err(c, "error %i from bch2_move_data() in copygc", ret);
+	if (ret)
 		return ret;
-	}
 
 	ret = check_copygc_was_done(c, &sectors_not_moved, &buckets_not_moved);
 	if (ret) {
diff --git a/libbcachefs/quota.c b/libbcachefs/quota.c
index ca029a00..364ef631 100644
--- a/libbcachefs/quota.c
+++ b/libbcachefs/quota.c
@@ -57,15 +57,22 @@ const struct bch_sb_field_ops bch_sb_field_ops_quota = {
 	.to_text	= bch2_sb_quota_to_text,
 };
 
-const char *bch2_quota_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_quota_invalid(const struct bch_fs *c, struct bkey_s_c k,
+		       int rw, struct printbuf *err)
 {
-	if (k.k->p.inode >= QTYP_NR)
-		return "invalid quota type";
+	if (k.k->p.inode >= QTYP_NR) {
+		pr_buf(err, "invalid quota type (%llu >= %u)",
+		       k.k->p.inode, QTYP_NR);
+		return -EINVAL;
+	}
 
-	if (bkey_val_bytes(k.k) != sizeof(struct bch_quota))
-		return "incorrect value size";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_quota)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_quota));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_quota_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/quota.h b/libbcachefs/quota.h
index 51e4f971..8c67ae1d 100644
--- a/libbcachefs/quota.h
+++ b/libbcachefs/quota.h
@@ -7,7 +7,7 @@
 
 extern const struct bch_sb_field_ops bch_sb_field_ops_quota;
 
-const char *bch2_quota_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_quota_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_quota_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_quota (struct bkey_ops) {		\
diff --git a/libbcachefs/recovery.c b/libbcachefs/recovery.c
index 6a92c1a0..88ed8030 100644
--- a/libbcachefs/recovery.c
+++ b/libbcachefs/recovery.c
@@ -471,7 +471,7 @@ void bch2_journal_keys_free(struct journal_keys *keys)
 
 	kvfree(keys->d);
 	keys->d = NULL;
-	keys->nr = 0;
+	keys->nr = keys->gap = keys->size = 0;
 }
 
 static struct journal_keys journal_keys_sort(struct list_head *journal_entries)
@@ -1237,7 +1237,7 @@ use_clean:
 	if (c->opts.fsck) {
 		bch_info(c, "checking need_discard and freespace btrees");
 		err = "error checking need_discard and freespace btrees";
-		ret = bch2_check_alloc_info(c, true);
+		ret = bch2_check_alloc_info(c);
 		if (ret)
 			goto err;
 
@@ -1276,6 +1276,19 @@ use_clean:
 	if (ret)
 		goto err;
 
+	if (c->opts.fsck) {
+		bch_info(c, "checking alloc to lru refs");
+		err = "error checking alloc to lru refs";
+		ret = bch2_check_alloc_to_lru_refs(c);
+		if (ret)
+			goto err;
+
+		ret = bch2_check_lrus(c, true);
+		if (ret)
+			goto err;
+		bch_verbose(c, "done checking alloc to lru refs");
+	}
+
 	if (c->sb.version < bcachefs_metadata_version_snapshot_2) {
 		bch2_fs_lazy_rw(c);
 
diff --git a/libbcachefs/reflink.c b/libbcachefs/reflink.c
index 68247309..6a81eb9b 100644
--- a/libbcachefs/reflink.c
+++ b/libbcachefs/reflink.c
@@ -25,18 +25,25 @@ static inline unsigned bkey_type_to_indirect(const struct bkey *k)
 
 /* reflink pointers */
 
-const char *bch2_reflink_p_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_reflink_p_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			   int rw, struct printbuf *err)
 {
 	struct bkey_s_c_reflink_p p = bkey_s_c_to_reflink_p(k);
 
-	if (bkey_val_bytes(p.k) != sizeof(*p.v))
-		return "incorrect value size";
+	if (bkey_val_bytes(p.k) != sizeof(*p.v)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(p.k), sizeof(*p.v));
+		return -EINVAL;
+	}
 
 	if (c->sb.version >= bcachefs_metadata_version_reflink_p_fix &&
-	    le64_to_cpu(p.v->idx) < le32_to_cpu(p.v->front_pad))
-		return "idx < front_pad";
+	    le64_to_cpu(p.v->idx) < le32_to_cpu(p.v->front_pad)) {
+		pr_buf(err, "idx < front_pad (%llu < %u)",
+		       le64_to_cpu(p.v->idx), le32_to_cpu(p.v->front_pad));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_reflink_p_to_text(struct printbuf *out, struct bch_fs *c,
@@ -70,14 +77,18 @@ bool bch2_reflink_p_merge(struct bch_fs *c, struct bkey_s _l, struct bkey_s_c _r
 
 /* indirect extents */
 
-const char *bch2_reflink_v_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_reflink_v_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			   int rw, struct printbuf *err)
 {
 	struct bkey_s_c_reflink_v r = bkey_s_c_to_reflink_v(k);
 
-	if (bkey_val_bytes(r.k) < sizeof(*r.v))
-		return "incorrect value size";
+	if (bkey_val_bytes(r.k) < sizeof(*r.v)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(r.k), sizeof(*r.v));
+		return -EINVAL;
+	}
 
-	return bch2_bkey_ptrs_invalid(c, k);
+	return bch2_bkey_ptrs_invalid(c, k, rw, err);
 }
 
 void bch2_reflink_v_to_text(struct printbuf *out, struct bch_fs *c,
@@ -118,12 +129,16 @@ int bch2_trans_mark_reflink_v(struct btree_trans *trans,
 
 /* indirect inline data */
 
-const char *bch2_indirect_inline_data_invalid(const struct bch_fs *c,
-					      struct bkey_s_c k)
+int bch2_indirect_inline_data_invalid(const struct bch_fs *c, struct bkey_s_c k,
+				      int rw, struct printbuf *err)
 {
-	if (bkey_val_bytes(k.k) < sizeof(struct bch_indirect_inline_data))
-		return "incorrect value size";
-	return NULL;
+	if (bkey_val_bytes(k.k) < sizeof(struct bch_indirect_inline_data)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_indirect_inline_data));
+		return -EINVAL;
+	}
+
+	return 0;
 }
 
 void bch2_indirect_inline_data_to_text(struct printbuf *out,
diff --git a/libbcachefs/reflink.h b/libbcachefs/reflink.h
index 8eb41c02..e0a9d8e4 100644
--- a/libbcachefs/reflink.h
+++ b/libbcachefs/reflink.h
@@ -2,7 +2,8 @@
 #ifndef _BCACHEFS_REFLINK_H
 #define _BCACHEFS_REFLINK_H
 
-const char *bch2_reflink_p_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_reflink_p_invalid(const struct bch_fs *, struct bkey_s_c,
+			   int, struct printbuf *);
 void bch2_reflink_p_to_text(struct printbuf *, struct bch_fs *,
 			    struct bkey_s_c);
 bool bch2_reflink_p_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
@@ -15,7 +16,8 @@ bool bch2_reflink_p_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 	.atomic_trigger	= bch2_mark_reflink_p,			\
 }
 
-const char *bch2_reflink_v_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_reflink_v_invalid(const struct bch_fs *, struct bkey_s_c,
+			   int, struct printbuf *);
 void bch2_reflink_v_to_text(struct printbuf *, struct bch_fs *,
 			    struct bkey_s_c);
 int bch2_trans_mark_reflink_v(struct btree_trans *, struct bkey_s_c,
@@ -29,8 +31,8 @@ int bch2_trans_mark_reflink_v(struct btree_trans *, struct bkey_s_c,
 	.atomic_trigger	= bch2_mark_extent,			\
 }
 
-const char *bch2_indirect_inline_data_invalid(const struct bch_fs *,
-					      struct bkey_s_c);
+int bch2_indirect_inline_data_invalid(const struct bch_fs *, struct bkey_s_c,
+				      int, struct printbuf *);
 void bch2_indirect_inline_data_to_text(struct printbuf *,
 				struct bch_fs *, struct bkey_s_c);
 int bch2_trans_mark_indirect_inline_data(struct btree_trans *,
diff --git a/libbcachefs/subvolume.c b/libbcachefs/subvolume.c
index cdb89ba2..63a57399 100644
--- a/libbcachefs/subvolume.c
+++ b/libbcachefs/subvolume.c
@@ -26,39 +26,55 @@ void bch2_snapshot_to_text(struct printbuf *out, struct bch_fs *c,
 	       le32_to_cpu(s.v->subvol));
 }
 
-const char *bch2_snapshot_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_snapshot_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			  int rw, struct printbuf *err)
 {
 	struct bkey_s_c_snapshot s;
 	u32 i, id;
 
 	if (bkey_cmp(k.k->p, POS(0, U32_MAX)) > 0 ||
-	    bkey_cmp(k.k->p, POS(0, 1)) < 0)
-		return "bad pos";
+	    bkey_cmp(k.k->p, POS(0, 1)) < 0) {
+		pr_buf(err, "bad pos");
+		return -EINVAL;
+	}
 
-	if (bkey_val_bytes(k.k) != sizeof(struct bch_snapshot))
-		return "bad val size";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_snapshot)) {
+		pr_buf(err, "bad val size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_snapshot));
+		return -EINVAL;
+	}
 
 	s = bkey_s_c_to_snapshot(k);
 
 	id = le32_to_cpu(s.v->parent);
-	if (id && id <= k.k->p.offset)
-		return "bad parent node";
+	if (id && id <= k.k->p.offset) {
+		pr_buf(err, "bad parent node (%u <= %llu)",
+		       id, k.k->p.offset);
+		return -EINVAL;
+	}
 
-	if (le32_to_cpu(s.v->children[0]) < le32_to_cpu(s.v->children[1]))
-		return "children not normalized";
+	if (le32_to_cpu(s.v->children[0]) < le32_to_cpu(s.v->children[1])) {
+		pr_buf(err, "children not normalized");
+		return -EINVAL;
+	}
 
 	if (s.v->children[0] &&
-	    s.v->children[0] == s.v->children[1])
-		return "duplicate child nodes";
+	    s.v->children[0] == s.v->children[1]) {
+		pr_buf(err, "duplicate child nodes");
+		return -EINVAL;
+	}
 
 	for (i = 0; i < 2; i++) {
 		id = le32_to_cpu(s.v->children[i]);
 
-		if (id >= k.k->p.offset)
-			return "bad child node";
+		if (id >= k.k->p.offset) {
+			pr_buf(err, "bad child node (%u >= %llu)",
+			       id, k.k->p.offset);
+			return -EINVAL;
+		}
 	}
 
-	return NULL;
+	return 0;
 }
 
 int bch2_mark_snapshot(struct btree_trans *trans,
@@ -729,18 +745,22 @@ static int bch2_delete_dead_snapshots_hook(struct btree_trans *trans,
 
 /* Subvolumes: */
 
-const char *bch2_subvolume_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_subvolume_invalid(const struct bch_fs *c, struct bkey_s_c k,
+			   int rw, struct printbuf *err)
 {
-	if (bkey_cmp(k.k->p, SUBVOL_POS_MIN) < 0)
-		return "invalid pos";
-
-	if (bkey_cmp(k.k->p, SUBVOL_POS_MAX) > 0)
-		return "invalid pos";
+	if (bkey_cmp(k.k->p, SUBVOL_POS_MIN) < 0 ||
+	    bkey_cmp(k.k->p, SUBVOL_POS_MAX) > 0) {
+		pr_buf(err, "invalid pos");
+		return -EINVAL;
+	}
 
-	if (bkey_val_bytes(k.k) != sizeof(struct bch_subvolume))
-		return "bad val size";
+	if (bkey_val_bytes(k.k) != sizeof(struct bch_subvolume)) {
+		pr_buf(err, "incorrect value size (%zu != %zu)",
+		       bkey_val_bytes(k.k), sizeof(struct bch_subvolume));
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_subvolume_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/subvolume.h b/libbcachefs/subvolume.h
index f609291a..a4425389 100644
--- a/libbcachefs/subvolume.h
+++ b/libbcachefs/subvolume.h
@@ -6,7 +6,8 @@
 #include "subvolume_types.h"
 
 void bch2_snapshot_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
-const char *bch2_snapshot_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_snapshot_invalid(const struct bch_fs *, struct bkey_s_c,
+			  int rw, struct printbuf *);
 
 #define bch2_bkey_ops_snapshot (struct bkey_ops) {		\
 	.key_invalid	= bch2_snapshot_invalid,		\
@@ -96,7 +97,8 @@ int bch2_fs_snapshots_check(struct bch_fs *);
 void bch2_fs_snapshots_exit(struct bch_fs *);
 int bch2_fs_snapshots_start(struct bch_fs *);
 
-const char *bch2_subvolume_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_subvolume_invalid(const struct bch_fs *, struct bkey_s_c,
+			   int rw, struct printbuf *);
 void bch2_subvolume_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_subvolume (struct bkey_ops) {		\
diff --git a/libbcachefs/super.c b/libbcachefs/super.c
index 4a071711..1af9bcc0 100644
--- a/libbcachefs/super.c
+++ b/libbcachefs/super.c
@@ -1471,15 +1471,6 @@ int bch2_dev_remove(struct bch_fs *c, struct bch_dev *ca, int flags)
 		goto err;
 	}
 
-	/*
-	 * must flush all existing journal entries, they might have
-	 * (overwritten) keys that point to the device we're removing:
-	 */
-	bch2_journal_flush_all_pins(&c->journal);
-	/*
-	 * hack to ensure bch2_replicas_gc2() clears out entries to this device
-	 */
-	bch2_journal_meta(&c->journal);
 	ret = bch2_journal_error(&c->journal);
 	if (ret) {
 		bch_err(ca, "Remove failed, journal error");
diff --git a/libbcachefs/xattr.c b/libbcachefs/xattr.c
index 8d23b4c2..f119847e 100644
--- a/libbcachefs/xattr.c
+++ b/libbcachefs/xattr.c
@@ -69,32 +69,51 @@ const struct bch_hash_desc bch2_xattr_hash_desc = {
 	.cmp_bkey	= xattr_cmp_bkey,
 };
 
-const char *bch2_xattr_invalid(const struct bch_fs *c, struct bkey_s_c k)
+int bch2_xattr_invalid(const struct bch_fs *c, struct bkey_s_c k,
+		       int rw, struct printbuf *err)
 {
 	const struct xattr_handler *handler;
 	struct bkey_s_c_xattr xattr = bkey_s_c_to_xattr(k);
 
-	if (bkey_val_bytes(k.k) < sizeof(struct bch_xattr))
-		return "value too small";
+	if (bkey_val_bytes(k.k) < sizeof(struct bch_xattr)) {
+		pr_buf(err, "incorrect value size (%zu < %zu)",
+		       bkey_val_bytes(k.k), sizeof(*xattr.v));
+		return -EINVAL;
+	}
 
 	if (bkey_val_u64s(k.k) <
 	    xattr_val_u64s(xattr.v->x_name_len,
-			   le16_to_cpu(xattr.v->x_val_len)))
-		return "value too small";
+			   le16_to_cpu(xattr.v->x_val_len))) {
+		pr_buf(err, "value too small (%zu < %u)",
+		       bkey_val_u64s(k.k),
+		       xattr_val_u64s(xattr.v->x_name_len,
+				      le16_to_cpu(xattr.v->x_val_len)));
+		return -EINVAL;
+	}
 
+	/* XXX why +4 ? */
 	if (bkey_val_u64s(k.k) >
 	    xattr_val_u64s(xattr.v->x_name_len,
-			   le16_to_cpu(xattr.v->x_val_len) + 4))
-		return "value too big";
+			   le16_to_cpu(xattr.v->x_val_len) + 4)) {
+		pr_buf(err, "value too big (%zu > %u)",
+		       bkey_val_u64s(k.k),
+		       xattr_val_u64s(xattr.v->x_name_len,
+				      le16_to_cpu(xattr.v->x_val_len) + 4));
+		return -EINVAL;
+	}
 
 	handler = bch2_xattr_type_to_handler(xattr.v->x_type);
-	if (!handler)
-		return "invalid type";
+	if (!handler) {
+		pr_buf(err, "invalid type (%u)", xattr.v->x_type);
+		return -EINVAL;
+	}
 
-	if (memchr(xattr.v->x_name, '\0', xattr.v->x_name_len))
-		return "xattr name has invalid characters";
+	if (memchr(xattr.v->x_name, '\0', xattr.v->x_name_len)) {
+		pr_buf(err, "xattr name has invalid characters");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_xattr_to_text(struct printbuf *out, struct bch_fs *c,
diff --git a/libbcachefs/xattr.h b/libbcachefs/xattr.h
index f4f89654..66d7a1e3 100644
--- a/libbcachefs/xattr.h
+++ b/libbcachefs/xattr.h
@@ -6,7 +6,7 @@
 
 extern const struct bch_hash_desc bch2_xattr_hash_desc;
 
-const char *bch2_xattr_invalid(const struct bch_fs *, struct bkey_s_c);
+int bch2_xattr_invalid(const struct bch_fs *, struct bkey_s_c, int, struct printbuf *);
 void bch2_xattr_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_xattr (struct bkey_ops) {		\