bcachefs: Improve bch2_fsck_err()

 - factor out fsck_err_get()
 - if the "bcachefs (%s):" prefix has already been applied, don't
   duplicate it
 - convert to printbufs instead of static char arrays
 - tidy up control flow a bit
 - use bch2_print_string_as_lines(), to avoid messages getting truncated
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

fs/bcachefs/bcachefs.h

@@ -226,9 +226,11 @@ do {									\
 	 dynamic_fault("bcachefs:meta:write:" name)
 
 #ifdef __KERNEL__
-#define bch2_fmt(_c, fmt)		"bcachefs (%s): " fmt "\n", ((_c)->name)
+#define bch2_log_msg(_c, fmt)		"bcachefs (%s): " fmt, ((_c)->name)
+#define bch2_fmt(_c, fmt)		bch2_log_msg(_c, fmt "\n")
 #define bch2_fmt_inum(_c, _inum, fmt)	"bcachefs (%s inum %llu): " fmt "\n", ((_c)->name), (_inum)
 #else
+#define bch2_log_msg(_c, fmt)		fmt
 #define bch2_fmt(_c, fmt)		fmt "\n"
 #define bch2_fmt_inum(_c, _inum, fmt)	"inum %llu: " fmt "\n", (_inum)
 #endif

fs/bcachefs/error.c

@@ -68,102 +68,135 @@ void bch2_io_error(struct bch_dev *ca)
 #include "tools-util.h"
 #endif
 
-int bch2_fsck_err(struct bch_fs *c, unsigned flags, const char *fmt, ...)
+static struct fsck_err_state *fsck_err_get(struct bch_fs *c, const char *fmt)
 {
-	struct fsck_err_state *s = NULL;
-	va_list args;
-	bool fix = false, print = true, suppressing = false;
-	char _buf[sizeof(s->buf)], *buf = _buf;
-
-	if (test_bit(BCH_FS_FSCK_DONE, &c->flags)) {
-		va_start(args, fmt);
-		vprintk(fmt, args);
-		va_end(args);
+	struct fsck_err_state *s;
 
-		if (c->opts.errors == BCH_ON_ERROR_continue) {
-			bch_err(c, "fixing");
-			return -BCH_ERR_fsck_fix;
-		} else {
-			bch2_inconsistent_error(c);
-			return -BCH_ERR_fsck_errors_not_fixed;
-		}
-	}
-
-	mutex_lock(&c->fsck_error_lock);
+	if (test_bit(BCH_FS_FSCK_DONE, &c->flags))
+		return NULL;
 
 	list_for_each_entry(s, &c->fsck_errors, list)
-		if (s->fmt == fmt)
-			goto found;
+		if (s->fmt == fmt) {
+			/*
+			 * move it to the head of the list: repeated fsck errors
+			 * are common
+			 */
+			list_move(&s->list, &c->fsck_errors);
+			return s;
+		}
 
 	s = kzalloc(sizeof(*s), GFP_NOFS);
 	if (!s) {
 		if (!c->fsck_alloc_err)
 			bch_err(c, "kmalloc err, cannot ratelimit fsck errs");
 		c->fsck_alloc_err = true;
-		buf = _buf;
-		goto print;
+		return NULL;
 	}
 
 	INIT_LIST_HEAD(&s->list);
 	s->fmt = fmt;
-found:
-	list_move(&s->list, &c->fsck_errors);
-	s->nr++;
-	if (c->opts.ratelimit_errors &&
-	    !(flags & FSCK_NO_RATELIMIT) &&
-	    s->nr >= FSCK_ERR_RATELIMIT_NR) {
-		if (s->nr == FSCK_ERR_RATELIMIT_NR)
-			suppressing = true;
-		else
-			print = false;
+	s->buf = PRINTBUF;
+	list_add(&s->list, &c->fsck_errors);
+	return s;
+}
+
+int bch2_fsck_err(struct bch_fs *c, unsigned flags, const char *fmt, ...)
+{
+	struct fsck_err_state *s = NULL;
+	va_list args;
+	bool print = true, suppressing = false;
+	struct printbuf buf = PRINTBUF, *out = &buf;
+	int ret = -BCH_ERR_fsck_ignore;
+
+	mutex_lock(&c->fsck_error_lock);
+	s = fsck_err_get(c, fmt);
+	if (s) {
+		if (c->opts.ratelimit_errors &&
+		    !(flags & FSCK_NO_RATELIMIT) &&
+		    s->nr >= FSCK_ERR_RATELIMIT_NR) {
+			if (s->nr == FSCK_ERR_RATELIMIT_NR)
+				suppressing = true;
+			else
+				print = false;
+		}
+
+		printbuf_reset(&s->buf);
+		out = &s->buf;
+		s->nr++;
 	}
-	buf		= s->buf;
-print:
+
+	if (!strncmp(fmt, "bcachefs:", 9))
+		prt_printf(out, bch2_log_msg(c, ""));
+
 	va_start(args, fmt);
-	vscnprintf(buf, sizeof(_buf), fmt, args);
+	prt_vprintf(out, fmt, args);
 	va_end(args);
 
-	if (c->opts.fix_errors == FSCK_OPT_EXIT) {
-		bch_err(c, "%s, exiting", buf);
+	if (test_bit(BCH_FS_FSCK_DONE, &c->flags)) {
+		if (c->opts.errors != BCH_ON_ERROR_continue ||
+		    !(flags & (FSCK_CAN_FIX|FSCK_CAN_IGNORE))) {
+			prt_str(out, ", shutting down");
+			bch2_inconsistent_error(c);
+			ret = -BCH_ERR_fsck_errors_not_fixed;
+		} else if (flags & FSCK_CAN_FIX) {
+			prt_str(out, ", fixing");
+			ret = -BCH_ERR_fsck_fix;
+		} else {
+			prt_str(out, ", continuing");
+			ret = -BCH_ERR_fsck_ignore;
+		}
+	} else if (c->opts.fix_errors == FSCK_OPT_EXIT) {
+		prt_str(out, ", exiting");
+		ret = -BCH_ERR_fsck_errors_not_fixed;
 	} else if (flags & FSCK_CAN_FIX) {
 		if (c->opts.fix_errors == FSCK_OPT_ASK) {
-			printk(KERN_ERR "%s: fix?", buf);
-			fix = ask_yn();
+			prt_str(out, ": fix?");
+			bch2_print_string_as_lines(KERN_ERR, out->buf);
+			print = false;
+			ret = ask_yn()
+				? -BCH_ERR_fsck_fix
+				: -BCH_ERR_fsck_ignore;
 		} else if (c->opts.fix_errors == FSCK_OPT_YES ||
 			   (c->opts.nochanges &&
 			    !(flags & FSCK_CAN_IGNORE))) {
-			if (print)
-				bch_err(c, "%s, fixing", buf);
-			fix = true;
+			prt_str(out, ", fixing");
+			ret = -BCH_ERR_fsck_fix;
 		} else {
-			if (print)
-				bch_err(c, "%s, not fixing", buf);
-			fix = false;
+			prt_str(out, ", not fixing");
 		}
 	} else if (flags & FSCK_NEED_FSCK) {
-		if (print)
-			bch_err(c, "%s (run fsck to correct)", buf);
+		prt_str(out, " (run fsck to correct)");
 	} else {
-		if (print)
-			bch_err(c, "%s (repair unimplemented)", buf);
+		prt_str(out, " (repair unimplemented)");
 	}
 
-	if (suppressing)
+	if (ret == -BCH_ERR_fsck_ignore &&
+	    (c->opts.fix_errors == FSCK_OPT_EXIT ||
+	     !(flags & FSCK_CAN_IGNORE)))
+		ret = -BCH_ERR_fsck_errors_not_fixed;
+
+	if (print)
+		bch2_print_string_as_lines(KERN_ERR, out->buf);
+
+	if (!test_bit(BCH_FS_FSCK_DONE, &c->flags) &&
+	    (ret != -BCH_ERR_fsck_fix &&
+	     ret != -BCH_ERR_fsck_ignore))
+		bch_err(c, "Unable to continue, halting");
+	else if (suppressing)
 		bch_err(c, "Ratelimiting new instances of previous error");
 
 	mutex_unlock(&c->fsck_error_lock);
 
-	if (fix) {
+	printbuf_exit(&buf);
+
+	if (ret == -BCH_ERR_fsck_fix) {
 		set_bit(BCH_FS_ERRORS_FIXED, &c->flags);
-		return -BCH_ERR_fsck_fix;
 	} else {
 		set_bit(BCH_FS_ERRORS_NOT_FIXED, &c->flags);
 		set_bit(BCH_FS_ERROR, &c->flags);
-		return c->opts.fix_errors == FSCK_OPT_EXIT ||
-			!(flags & FSCK_CAN_IGNORE)
-			? -BCH_ERR_fsck_errors_not_fixed
-			: -BCH_ERR_fsck_ignore;
 	}
+
+	return ret;
 }
 
 void bch2_flush_fsck_errs(struct bch_fs *c)
@@ -174,9 +207,10 @@ void bch2_flush_fsck_errs(struct bch_fs *c)
 
 	list_for_each_entry_safe(s, n, &c->fsck_errors, list) {
 		if (s->ratelimited)
-			bch_err(c, "Saw %llu errors like:\n    %s", s->nr, s->buf);
+			bch_err(c, "Saw %llu errors like:\n    %s", s->nr, s->buf.buf);
 
 		list_del(&s->list);
+		printbuf_exit(&s->buf);
 		kfree(s);
 	}
 

fs/bcachefs/error.h

@@ -103,7 +103,7 @@ struct fsck_err_state {
 	const char		*fmt;
 	u64			nr;
 	bool			ratelimited;
-	char			buf[512];
+	struct printbuf		buf;
 };
 
 #define FSCK_CAN_FIX		(1 << 0)
@@ -121,7 +121,6 @@ void bch2_flush_fsck_errs(struct bch_fs *);
 									\
 	if (_ret != -BCH_ERR_fsck_fix &&				\
 	    _ret != -BCH_ERR_fsck_ignore) {				\
-		bch_err(c, "Unable to continue, halting");		\
 		ret = _ret;						\
 		goto fsck_err;						\
 	}								\