bcachefs: Better printing of inconsistency errors

Build up and emit the error message for an inconsistency error all at
once, instead of spread over multiple printk calls, so they're not
jumbled in the dmesg log.

Also, add better indenting.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

Author: Kent Overstreet

fs/bcachefs/backpointers.c

@@ -96,6 +96,7 @@ static noinline int backpointer_mod_err(struct btree_trans *trans,
 {
 	struct bch_fs *c = trans->c;
 	struct printbuf buf = PRINTBUF;
+	int ret = 0;
 
 	if (insert) {
 		prt_printf(&buf, "existing backpointer found when inserting ");
@@ -125,17 +126,15 @@ static noinline int backpointer_mod_err(struct btree_trans *trans,
 
 		prt_printf(&buf, "for ");
 		bch2_bkey_val_to_text(&buf, c, orig_k);
-
-		bch_err(c, "%s", buf.buf);
 	}
 
-	printbuf_exit(&buf);
+	if (c->curr_recovery_pass > BCH_RECOVERY_PASS_check_extents_to_backpointers &&
+	    __bch2_inconsistent_error(c, &buf))
+		ret = -BCH_ERR_erofs_unfixed_errors;
 
-	if (c->curr_recovery_pass > BCH_RECOVERY_PASS_check_extents_to_backpointers) {
-		return bch2_inconsistent_error(c) ? BCH_ERR_erofs_unfixed_errors : 0;
-	} else {
-		return 0;
-	}
+	bch_err(c, "%s", buf.buf);
+	printbuf_exit(&buf);
+	return ret;
 }
 
 int bch2_bucket_backpointer_mod_nowritebuffer(struct btree_trans *trans,

fs/bcachefs/btree_io.c

@@ -548,32 +548,39 @@ static int __btree_err(int ret,
 		       enum bch_sb_error_id err_type,
 		       const char *fmt, ...)
 {
-	struct printbuf out = PRINTBUF;
 	bool silent = c->curr_recovery_pass == BCH_RECOVERY_PASS_scan_for_btree_nodes;
-	va_list args;
+
+	if (!have_retry && ret == -BCH_ERR_btree_node_read_err_want_retry)
+		ret = -BCH_ERR_btree_node_read_err_fixable;
+	if (!have_retry && ret == -BCH_ERR_btree_node_read_err_must_retry)
+		ret = -BCH_ERR_btree_node_read_err_bad_node;
+
+	if (!silent && ret != -BCH_ERR_btree_node_read_err_fixable)
+		bch2_sb_error_count(c, err_type);
+
+	struct printbuf out = PRINTBUF;
+	if (write != WRITE && ret != -BCH_ERR_btree_node_read_err_fixable) {
+		printbuf_indent_add_nextline(&out, 2);
+#ifdef BCACHEFS_LOG_PREFIX
+		prt_printf(&out, bch2_log_msg(c, ""));
+#endif
+	}
 
 	btree_err_msg(&out, c, ca, b, i, k, b->written, write);
 
+	va_list args;
 	va_start(args, fmt);
 	prt_vprintf(&out, fmt, args);
 	va_end(args);
 
 	if (write == WRITE) {
-		bch2_print_string_as_lines(KERN_ERR, out.buf);
-		ret = c->opts.errors == BCH_ON_ERROR_continue
-			? 0
-			: -BCH_ERR_fsck_errors_not_fixed;
-		goto out;
+		prt_str(&out, ", ");
+		ret = __bch2_inconsistent_error(c, &out)
+			? -BCH_ERR_fsck_errors_not_fixed
+			: 0;
+		silent = false;
 	}
 
-	if (!have_retry && ret == -BCH_ERR_btree_node_read_err_want_retry)
-		ret = -BCH_ERR_btree_node_read_err_fixable;
-	if (!have_retry && ret == -BCH_ERR_btree_node_read_err_must_retry)
-		ret = -BCH_ERR_btree_node_read_err_bad_node;
-
-	if (!silent && ret != -BCH_ERR_btree_node_read_err_fixable)
-		bch2_sb_error_count(c, err_type);
-
 	switch (ret) {
 	case -BCH_ERR_btree_node_read_err_fixable:
 		ret = !silent
@@ -583,25 +590,21 @@ static int __btree_err(int ret,
 		    ret != -BCH_ERR_fsck_ignore)
 			goto fsck_err;
 		ret = -BCH_ERR_fsck_fix;
-		break;
-	case -BCH_ERR_btree_node_read_err_want_retry:
-	case -BCH_ERR_btree_node_read_err_must_retry:
-		if (!silent)
-			bch2_print_string_as_lines(KERN_ERR, out.buf);
-		break;
+		goto out;
 	case -BCH_ERR_btree_node_read_err_bad_node:
-		if (!silent)
-			bch2_print_string_as_lines(KERN_ERR, out.buf);
-		ret = bch2_topology_error(c);
+		prt_str(&out, ", ");
+		ret = __bch2_topology_error(c, &out);
+		if (ret)
+			silent = false;
 		break;
 	case -BCH_ERR_btree_node_read_err_incompatible:
-		if (!silent)
-			bch2_print_string_as_lines(KERN_ERR, out.buf);
 		ret = -BCH_ERR_fsck_errors_not_fixed;
+		silent = false;
 		break;
-	default:
-		BUG();
 	}
+
+	if (!silent)
+		bch2_print_string_as_lines(KERN_ERR, out.buf);
 out:
 fsck_err:
 	printbuf_exit(&out);

fs/bcachefs/btree_iter.c

@@ -1495,16 +1495,6 @@ void bch2_trans_updates_to_text(struct printbuf *buf, struct btree_trans *trans)
 	printbuf_indent_sub(buf, 2);
 }
 
-noinline __cold
-void bch2_dump_trans_updates(struct btree_trans *trans)
-{
-	struct printbuf buf = PRINTBUF;
-
-	bch2_trans_updates_to_text(&buf, trans);
-	bch2_print_str(trans->c, buf.buf);
-	printbuf_exit(&buf);
-}
-
 static void bch2_btree_path_to_text_short(struct printbuf *out, struct btree_trans *trans, btree_path_idx_t path_idx)
 {
 	struct btree_path *path = trans->paths + path_idx;

fs/bcachefs/btree_iter.h

@@ -9,7 +9,6 @@
 void bch2_trans_updates_to_text(struct printbuf *, struct btree_trans *);
 void bch2_btree_path_to_text(struct printbuf *, struct btree_trans *, btree_path_idx_t);
 void bch2_trans_paths_to_text(struct printbuf *, struct btree_trans *);
-void bch2_dump_trans_updates(struct btree_trans *);
 void bch2_dump_trans_paths_updates(struct btree_trans *);
 
 static inline int __bkey_err(const struct bkey *k)

fs/bcachefs/btree_node_scan.c

@@ -579,10 +579,12 @@ int bch2_get_scanned_nodes(struct bch_fs *c, enum btree_id btree,
 
 		found_btree_node_to_key(&tmp.k, &n);
 
-		struct printbuf buf = PRINTBUF;
-		bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(&tmp.k));
-		bch_verbose(c, "%s(): recovering %s", __func__, buf.buf);
-		printbuf_exit(&buf);
+		if (c->opts.verbose) {
+			struct printbuf buf = PRINTBUF;
+			bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(&tmp.k));
+			bch_verbose(c, "%s(): recovering %s", __func__, buf.buf);
+			printbuf_exit(&buf);
+		}
 
 		BUG_ON(bch2_bkey_validate(c, bkey_i_to_s_c(&tmp.k),
 					  (struct bkey_validate_context) {

fs/bcachefs/btree_update_interior.c

@@ -54,6 +54,8 @@ int bch2_btree_node_check_topology(struct btree_trans *trans, struct btree *b)
 	struct bkey_buf prev;
 	int ret = 0;
 
+	printbuf_indent_add_nextline(&buf, 2);
+
 	BUG_ON(b->key.k.type == KEY_TYPE_btree_ptr_v2 &&
 	       !bpos_eq(bkey_i_to_btree_ptr_v2(&b->key)->v.min_key,
 			b->data->min_key));
@@ -64,19 +66,20 @@ int bch2_btree_node_check_topology(struct btree_trans *trans, struct btree *b)
 
 	if (b == btree_node_root(c, b)) {
 		if (!bpos_eq(b->data->min_key, POS_MIN)) {
-			printbuf_reset(&buf);
+			ret = __bch2_topology_error(c, &buf);
+
 			bch2_bpos_to_text(&buf, b->data->min_key);
 			log_fsck_err(trans, btree_root_bad_min_key,
 				      "btree root with incorrect min_key: %s", buf.buf);
-			goto topology_repair;
+			goto out;
 		}
 
 		if (!bpos_eq(b->data->max_key, SPOS_MAX)) {
-			printbuf_reset(&buf);
+			ret = __bch2_topology_error(c, &buf);
 			bch2_bpos_to_text(&buf, b->data->max_key);
 			log_fsck_err(trans, btree_root_bad_max_key,
 				      "btree root with incorrect max_key: %s", buf.buf);
-			goto topology_repair;
+			goto out;
 		}
 	}
 
@@ -94,9 +97,8 @@ int bch2_btree_node_check_topology(struct btree_trans *trans, struct btree *b)
 			: bpos_successor(prev.k->k.p);
 
 		if (!bpos_eq(expected_min, bp.v->min_key)) {
-			bch2_topology_error(c);
+			ret = __bch2_topology_error(c, &buf);
 
-			printbuf_reset(&buf);
 			prt_str(&buf, "end of prev node doesn't match start of next node\nin ");
 			bch2_btree_id_level_to_text(&buf, b->c.btree_id, b->c.level);
 			prt_str(&buf, " node ");
@@ -107,28 +109,25 @@ int bch2_btree_node_check_topology(struct btree_trans *trans, struct btree *b)
 			bch2_bkey_val_to_text(&buf, c, k);
 
 			log_fsck_err(trans, btree_node_topology_bad_min_key, "%s", buf.buf);
-			goto topology_repair;
+			goto out;
 		}
 
 		bch2_bkey_buf_reassemble(&prev, c, k);
 		bch2_btree_and_journal_iter_advance(&iter);
 	}
 
 	if (bkey_deleted(&prev.k->k)) {
-		bch2_topology_error(c);
+		ret = __bch2_topology_error(c, &buf);
 
-		printbuf_reset(&buf);
 		prt_str(&buf, "empty interior node\nin ");
 		bch2_btree_id_level_to_text(&buf, b->c.btree_id, b->c.level);
 		prt_str(&buf, " node ");
 		bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(&b->key));
 
 		log_fsck_err(trans, btree_node_topology_empty_interior_node, "%s", buf.buf);
-		goto topology_repair;
 	} else if (!bpos_eq(prev.k->k.p, b->key.k.p)) {
-		bch2_topology_error(c);
+		ret = __bch2_topology_error(c, &buf);
 
-		printbuf_reset(&buf);
 		prt_str(&buf, "last child node doesn't end at end of parent node\nin ");
 		bch2_btree_id_level_to_text(&buf, b->c.btree_id, b->c.level);
 		prt_str(&buf, " node ");
@@ -137,17 +136,13 @@ int bch2_btree_node_check_topology(struct btree_trans *trans, struct btree *b)
 		bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(prev.k));
 
 		log_fsck_err(trans, btree_node_topology_bad_max_key, "%s", buf.buf);
-		goto topology_repair;
 	}
 out:
 fsck_err:
 	bch2_btree_and_journal_iter_exit(&iter);
 	bch2_bkey_buf_exit(&prev, c);
 	printbuf_exit(&buf);
 	return ret;
-topology_repair:
-	ret = bch2_topology_error(c);
-	goto out;
 }
 
 /* Calculate ideal packed bkey format for new btree nodes: */
@@ -2007,18 +2002,22 @@ int __bch2_foreground_maybe_merge(struct btree_trans *trans,
 	}
 
 	if (!bpos_eq(bpos_successor(prev->data->max_key), next->data->min_key)) {
-		struct printbuf buf1 = PRINTBUF, buf2 = PRINTBUF;
-
-		bch2_bpos_to_text(&buf1, prev->data->max_key);
-		bch2_bpos_to_text(&buf2, next->data->min_key);
-		bch_err(c,
-			"%s(): btree topology error:\n"
-			"  prev ends at   %s\n"
-			"  next starts at %s",
-			__func__, buf1.buf, buf2.buf);
-		printbuf_exit(&buf1);
-		printbuf_exit(&buf2);
-		ret = bch2_topology_error(c);
+		struct printbuf buf = PRINTBUF;
+
+		printbuf_indent_add_nextline(&buf, 2);
+		prt_printf(&buf, "%s(): ", __func__);
+		ret = __bch2_topology_error(c, &buf);
+		prt_newline(&buf);
+
+		prt_printf(&buf, "prev ends at   ");
+		bch2_bpos_to_text(&buf, prev->data->max_key);
+		prt_newline(&buf);
+
+		prt_printf(&buf, "next starts at ");
+		bch2_bpos_to_text(&buf, next->data->min_key);
+
+		bch_err(c, "%s", buf.buf);
+		printbuf_exit(&buf);
 		goto err;
 	}