bcachefs: Convert .key_invalid methods to printbufs

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>

fs/bcachefs/alloc_background.c

@@ -302,71 +302,86 @@ 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, 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";
+	if (!bch2_dev_exists2(c, k.k->p.inode)) {
+		pr_buf(err, "invalid device (%llu)", k.k->p.inode);
+		return -EINVAL;
+	}
 
 	/* 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, 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_dev_exists2(c, k.k->p.inode)) {
+		pr_buf(err, "invalid device (%llu)", k.k->p.inode);
+		return -EINVAL;
+	}
 
-	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, 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";
+	if (!bch2_dev_exists2(c, k.k->p.inode)) {
+		pr_buf(err, "invalid device (%llu)", k.k->p.inode);
+		return -EINVAL;
+	}
 
 	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";
+	    k.k->p.offset >= ca->mi.nbuckets) {
+		pr_buf(err, "invalid bucket");
+		return -EINVAL;
+	}
 
-	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, 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";
+	if (!bch2_dev_exists2(c, k.k->p.inode)) {
+		pr_buf(err, "invalid device (%llu)", k.k->p.inode);
+		return -EINVAL;
+	}
 
 	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";
+	    k.k->p.offset >= ca->mi.nbuckets) {
+		pr_buf(err, "invalid bucket");
+		return -EINVAL;
+	}
 
-	return NULL;
+	return 0;
 }
 
 void bch2_alloc_v4_swab(struct bkey_s k)

fs/bcachefs/alloc_background.h

@@ -66,10 +66,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, struct printbuf *);
+int bch2_alloc_v2_invalid(const struct bch_fs *, struct bkey_s_c, struct printbuf *);
+int bch2_alloc_v3_invalid(const struct bch_fs *, struct bkey_s_c, struct printbuf *);
+int bch2_alloc_v4_invalid(const struct bch_fs *, struct bkey_s_c k, struct printbuf *);
 void bch2_alloc_v4_swab(struct bkey_s);
 void bch2_alloc_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 

fs/bcachefs/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,
+			       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,
+				 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,
+				   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,
+					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,
+				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,14 @@ 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, 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, err);
 }
 
 static unsigned bch2_key_types_allowed[] = {
@@ -182,63 +196,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,
+			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,
+		      struct printbuf *err)
 {
-	return __bch2_bkey_invalid(c, k, type) ?:
-		bch2_bkey_val_invalid(c, k);
+	return __bch2_bkey_invalid(c, k, type, err) ?:
+		bch2_bkey_val_invalid(c, k, 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)

fs/bcachefs/bkey_methods.h

@@ -14,8 +14,8 @@ extern const char * const bch2_bkey_types[];
 
 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 *, struct bkey_s_c,
+				       struct printbuf *);
 	void		(*val_to_text)(struct printbuf *, struct bch_fs *,
 				       struct bkey_s_c);
 	void		(*swab)(struct bkey_s);
@@ -32,12 +32,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, struct printbuf *);
+int __bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
+				enum btree_node_type, struct printbuf *);
+int bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
+			      enum btree_node_type, 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 *);

fs/bcachefs/btree_io.c

@@ -762,14 +762,23 @@ static int validate_bset(struct bch_fs *c, struct bch_dev *ca,
 	return ret;
 }
 
+static int bset_key_invalid(struct bch_fs *c, struct btree *b,
+			    struct bkey_s_c k,
+			    bool updated_range, int write,
+			    struct printbuf *err)
+{
+	return __bch2_bkey_invalid(c, k, btree_node_type(b), err) ?:
+		(!updated_range ? bch2_bkey_in_btree_node(b, k, err) : 0) ?:
+		(write ? bch2_bkey_val_invalid(c, k, 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;
@@ -778,7 +787,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,
@@ -804,14 +812,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),
@@ -827,16 +836,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);
@@ -848,8 +856,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;
 }
 
@@ -868,6 +875,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;
@@ -1060,17 +1068,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, &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, &buf);
+
+			btree_err(BTREE_ERR_FIXABLE, c, NULL, b, i, "%s", buf.buf);
 
 			btree_keys_account_key_drop(&b->nr, 0, k);
 
@@ -1107,6 +1118,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) {
@@ -1715,10 +1727,16 @@ 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, &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);

fs/bcachefs/btree_update_interior.c

@@ -1176,7 +1176,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));
@@ -1184,14 +1184,16 @@ 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), &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), &buf);
+		bch2_bkey_in_btree_node(b, bkey_i_to_s_c(insert), &buf);
+
+		bch2_fs_inconsistent(c, "%s", buf.buf);
 		dump_stack();
 	}
 
@@ -1211,6 +1213,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

fs/bcachefs/btree_update_leaf.c

@@ -862,23 +862,31 @@ 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, &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, &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;

fs/bcachefs/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;

fs/bcachefs/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,
+			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,

fs/bcachefs/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, struct printbuf *);
 void bch2_dirent_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_dirent (struct bkey_ops) {	\

fs/bcachefs/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,
+			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, err);
 }
 
 void bch2_stripe_to_text(struct printbuf *out, struct bch_fs *c,

fs/bcachefs/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,
+			struct printbuf *);
 void bch2_stripe_to_text(struct printbuf *, struct bch_fs *,
 			 struct bkey_s_c);
 

fs/bcachefs/extents.c

@@ -155,12 +155,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,
+			   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, err);
 }
 
 void bch2_btree_ptr_to_text(struct printbuf *out, struct bch_fs *c,
@@ -169,21 +173,31 @@ 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,
+				      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, err);
 }
 
 void bch2_btree_ptr_v2_to_text(struct printbuf *out, struct bch_fs *c,
@@ -219,17 +233,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);
@@ -362,17 +365,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,
+			     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,
@@ -1000,69 +1010,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,
+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)
+			      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;
+		}
 
-	if (ptr->offset + size_ondisk > bucket_to_sector(ca, ca->mi.nbuckets))
-		return "offset past end of device";
+	bucket = sector_to_bucket_and_offset(ca, ptr->offset, &bucket_offset);
 
-	if (ptr->offset < bucket_to_sector(ca, ca->mi.first_bucket))
-		return "offset before first bucket";
+	if (bucket >= ca->mi.nbuckets) {
+		pr_buf(err, "pointer past last bucket (%llu > %llu)",
+		       bucket, ca->mi.nbuckets);
+		return -EINVAL;
+	}
 
-	if (bucket_remainder(ca, ptr->offset) +
-	    size_ondisk > ca->mi.bucket_size)
-		return "spans multiple buckets";
+	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;
+	}
 
-	return NULL;
+	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 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,
+			   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:
@@ -1070,22 +1097,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:
@@ -1093,13 +1128,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)

fs/bcachefs/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, 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, 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,7 @@ 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, 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 +615,7 @@ 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, struct printbuf *);
 
 void bch2_ptr_swab(struct bkey_s);
 

fs/bcachefs/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,
+		       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,
+			  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,
+				  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,

fs/bcachefs/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, struct printbuf *);
+int bch2_inode_v2_invalid(const struct bch_fs *, struct bkey_s_c, 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,8 @@ 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, 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,	\

fs/bcachefs/journal_io.c

@@ -209,7 +209,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,
@@ -249,22 +249,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;
-
-		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",
+	if (bch2_bkey_invalid(c, bkey_i_to_s_c(k),
+			      __btree_node_type(level, btree_id), &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),
-				 invalid, buf.buf);
-		printbuf_exit(&buf);
+		       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));
+		pr_newline(&buf);
+		bch2_bkey_invalid(c, bkey_i_to_s_c(k),
+				  __btree_node_type(level, btree_id), &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;
 	}
 
@@ -272,6 +278,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;
 }
 

fs/bcachefs/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,
+		     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,

fs/bcachefs/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, struct printbuf *);
 void bch2_lru_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_lru (struct bkey_ops) {	\

fs/bcachefs/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,
+		       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,

fs/bcachefs/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, struct printbuf *);
 void bch2_quota_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_quota (struct bkey_ops) {		\

fs/bcachefs/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,
+			   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,
+			   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, 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,
+				      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,

fs/bcachefs/reflink.h

@@ -2,7 +2,7 @@
 #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, 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 +15,7 @@ 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, 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 +29,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,
+				      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 *,

fs/bcachefs/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,
+			  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,
+			   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,

fs/bcachefs/subvolume.h

@@ -6,7 +6,7 @@
 #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, struct printbuf *);
 
 #define bch2_bkey_ops_snapshot (struct bkey_ops) {		\
 	.key_invalid	= bch2_snapshot_invalid,		\
@@ -96,7 +96,7 @@ 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, struct printbuf *);
 void bch2_subvolume_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_subvolume (struct bkey_ops) {		\

fs/bcachefs/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,
+		       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,

fs/bcachefs/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, struct printbuf *);
 void bch2_xattr_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
 
 #define bch2_bkey_ops_xattr (struct bkey_ops) {		\