bcachefs: Better helpers for inconsistency errors

An inconsistency error often happens as part of an event with multiple
error messages, and we want to build up one single error message with
proper indenting to produce more readable log messages that don't get
garbled.

Add new helpers that emit messages to a printbuf instead of printing
them directly, next patch will convert to use them.

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

diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c
index 5a6f54a539bbdcd6f9e7195e0ebaf45656348bf6..d5f00aa252f687ad987a917127cff72c4d7d65a2 100644 (file)
--- a/fs/bcachefs/error.c
+++ b/fs/bcachefs/error.c
@@ -11,7 +11,16 @@
 
 #define FSCK_ERR_RATELIMIT_NR  10
 
-bool bch2_inconsistent_error(struct bch_fs *c)
+void bch2_log_msg_start(struct bch_fs *c, struct printbuf *out)
+{
+       printbuf_indent_add_nextline(out, 2);
+
+#ifdef BCACHEFS_LOG_PREFIX
+       prt_printf(out, bch2_log_msg(c, ""));
+#endif
+}
+
+bool __bch2_inconsistent_error(struct bch_fs *c, struct printbuf *out)
 {
        set_bit(BCH_FS_error, &c->flags);
 
@@ -21,8 +30,8 @@ bool bch2_inconsistent_error(struct bch_fs *c)
        case BCH_ON_ERROR_fix_safe:
        case BCH_ON_ERROR_ro:
                if (bch2_fs_emergency_read_only(c))
-                       bch_err(c, "inconsistency detected - emergency read only at journal seq %llu",
-                               journal_cur_seq(&c->journal));
+                       prt_printf(out, "inconsistency detected - emergency read only at journal seq %llu\n",
+                                  journal_cur_seq(&c->journal));
                return true;
        case BCH_ON_ERROR_panic:
                panic(bch2_fmt(c, "panic after error"));
@@ -32,6 +41,56 @@ bool bch2_inconsistent_error(struct bch_fs *c)
        }
 }
 
+bool bch2_inconsistent_error(struct bch_fs *c)
+{
+       struct printbuf buf = PRINTBUF;
+       printbuf_indent_add_nextline(&buf, 2);
+
+       bool ret = __bch2_inconsistent_error(c, &buf);
+       if (ret)
+               bch_err(c, "%s", buf.buf);
+       printbuf_exit(&buf);
+       return ret;
+}
+
+__printf(3, 0)
+static bool bch2_fs_trans_inconsistent(struct bch_fs *c, struct btree_trans *trans,
+                                      const char *fmt, va_list args)
+{
+       struct printbuf buf = PRINTBUF;
+
+       bch2_log_msg_start(c, &buf);
+
+       prt_vprintf(&buf, fmt, args);
+       prt_newline(&buf);
+
+       if (trans)
+               bch2_trans_updates_to_text(&buf, trans);
+       bool ret = __bch2_inconsistent_error(c, &buf);
+       bch2_print_string_as_lines(KERN_ERR, buf.buf);
+
+       printbuf_exit(&buf);
+       return ret;
+}
+
+bool bch2_fs_inconsistent(struct bch_fs *c, const char *fmt, ...)
+{
+       va_list args;
+       va_start(args, fmt);
+       bool ret = bch2_fs_trans_inconsistent(c, NULL, fmt, args);
+       va_end(args);
+       return ret;
+}
+
+bool bch2_trans_inconsistent(struct btree_trans *trans, const char *fmt, ...)
+{
+       va_list args;
+       va_start(args, fmt);
+       bool ret = bch2_fs_trans_inconsistent(trans->c, trans, fmt, args);
+       va_end(args);
+       return ret;
+}
+
 int bch2_topology_error(struct bch_fs *c)
 {
        set_bit(BCH_FS_topology_error, &c->flags);
@@ -44,6 +103,31 @@ int bch2_topology_error(struct bch_fs *c)
        }
 }
 
+int __bch2_topology_error(struct bch_fs *c, struct printbuf *out)
+{
+       prt_printf(out, "btree topology error: ");
+
+       return bch2_topology_error(c);
+}
+
+int bch2_fs_topology_error(struct bch_fs *c, const char *fmt, ...)
+{
+       struct printbuf buf = PRINTBUF;
+
+       bch2_log_msg_start(c, &buf);
+
+       va_list args;
+       va_start(args, fmt);
+       prt_vprintf(&buf, fmt, args);
+       va_end(args);
+
+       int ret = __bch2_topology_error(c, &buf);
+       bch2_print_string_as_lines(KERN_ERR, buf.buf);
+
+       printbuf_exit(&buf);
+       return ret;
+}
+
 void bch2_fatal_error(struct bch_fs *c)
 {
        if (bch2_fs_emergency_read_only(c))
@@ -379,6 +463,7 @@ int __bch2_fsck_err(struct bch_fs *c,
                    !(flags & (FSCK_CAN_FIX|FSCK_CAN_IGNORE))) {
                        prt_str(out, ", shutting down");
                        inconsistent = true;
+                       print = true;
                        ret = -BCH_ERR_fsck_errors_not_fixed;
                } else if (flags & FSCK_CAN_FIX) {
                        prt_str(out, ", ");
@@ -440,15 +525,20 @@ print:
        prt_newline(out);
 
        if (inconsistent)
-               bch2_inconsistent_error(c);
+               __bch2_inconsistent_error(c, out);
        else if (exiting)
                prt_printf(out, "Unable to continue, halting\n");
        else if (suppressing)
                prt_printf(out, "Ratelimiting new instances of previous error\n");
 
        if (print) {
+               /* possibly strip an empty line, from printbuf_indent_add */
+               while (out->pos && out->buf[out->pos - 1] == ' ')
+                       --out->pos;
+               printbuf_nul_terminate(out);
+
                if (bch2_fs_stdio_redirect(c))
-                       bch2_print(c, "%s\n", out->buf);
+                       bch2_print(c, "%s", out->buf);
                else
                        bch2_print_string_as_lines(KERN_ERR, out->buf);
        }
@@ -456,9 +546,6 @@ print:
        if (s)
                s->ret = ret;
 
-       if (inconsistent)
-               bch2_inconsistent_error(c);
-
        /*
         * We don't yet track whether the filesystem currently has errors, for
         * log_fsck_err()s: that would require us to track for every error type
@@ -527,9 +614,7 @@ int __bch2_bkey_fsck_err(struct bch_fs *c,
        prt_vprintf(&buf, fmt, args);
        va_end(args);
 
-       prt_str(&buf, ": delete?");
-
-       int ret = __bch2_fsck_err(c, NULL, fsck_flags, err, "%s", buf.buf);
+       int ret = __bch2_fsck_err(c, NULL, fsck_flags, err, "%s, delete?", buf.buf);
        printbuf_exit(&buf);
        return ret;
 }

diff --git a/fs/bcachefs/error.h b/fs/bcachefs/error.h
index 7d3f0e2a5fd6f1bc40f21e360bbc8579dae034f9..e3f72e26abdd6e182148541a040acf73a2479508 100644 (file)
--- a/fs/bcachefs/error.h
+++ b/fs/bcachefs/error.h
@@ -18,6 +18,8 @@ struct work_struct;
 
 /* Error messages: */
 
+void bch2_log_msg_start(struct bch_fs *, struct printbuf *);
+
 /*
  * Inconsistency errors: The on disk data is inconsistent. If these occur during
  * initial recovery, they don't indicate a bug in the running code - we walk all
@@ -29,21 +31,10 @@ struct work_struct;
  * BCH_ON_ERROR_CONTINUE mode
  */
 
+bool __bch2_inconsistent_error(struct bch_fs *, struct printbuf *);
 bool bch2_inconsistent_error(struct bch_fs *);
-
-int bch2_topology_error(struct bch_fs *);
-
-#define bch2_fs_topology_error(c, ...)                                 \
-({                                                                     \
-       bch_err(c, "btree topology error: " __VA_ARGS__);               \
-       bch2_topology_error(c);                                         \
-})
-
-#define bch2_fs_inconsistent(c, ...)                                   \
-({                                                                     \
-       bch_err(c, __VA_ARGS__);                                        \
-       bch2_inconsistent_error(c);                                     \
-})
+__printf(2, 3)
+bool bch2_fs_inconsistent(struct bch_fs *, const char *, ...);
 
 #define bch2_fs_inconsistent_on(cond, ...)                             \
 ({                                                                     \
@@ -53,26 +44,22 @@ int bch2_topology_error(struct bch_fs *);
        _ret;                                                           \
 })
 
-/*
- * When a transaction update discovers or is causing a fs inconsistency, it's
- * helpful to also dump the pending updates:
- */
-#define bch2_trans_inconsistent(trans, ...)                            \
-({                                                                     \
-       bch_err(trans->c, __VA_ARGS__);                                 \
-       bch2_dump_trans_updates(trans);                                 \
-       bch2_inconsistent_error(trans->c);                              \
-})
+__printf(2, 3)
+bool bch2_trans_inconsistent(struct btree_trans *, const char *, ...);
 
-#define bch2_trans_inconsistent_on(cond, trans, ...)                   \
+#define bch2_trans_inconsistent_on(cond, ...)                          \
 ({                                                                     \
        bool _ret = unlikely(!!(cond));                                 \
-                                                                       \
        if (_ret)                                                       \
-               bch2_trans_inconsistent(trans, __VA_ARGS__);            \
+               bch2_trans_inconsistent(__VA_ARGS__);                   \
        _ret;                                                           \
 })
 
+int bch2_topology_error(struct bch_fs *);
+int __bch2_topology_error(struct bch_fs *, struct printbuf *);
+__printf(2, 3)
+int bch2_fs_topology_error(struct bch_fs *, const char *, ...);
+
 /*
  * Fsck errors: inconsistency errors we detect at mount time, and should ideally
  * be able to repair: