From dbb9936b0dc905657db6e5289be18e425f1b60d3 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 25 Sep 2022 18:18:48 -0400 Subject: [PATCH] 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 --- fs/bcachefs/bcachefs.h | 4 +- fs/bcachefs/error.c | 152 +++++++++++++++++++++++++---------------- fs/bcachefs/error.h | 3 +- 3 files changed, 97 insertions(+), 62 deletions(-) diff --git a/fs/bcachefs/bcachefs.h b/fs/bcachefs/bcachefs.h index 74da688d994b..08fd899d8837 100644 --- a/fs/bcachefs/bcachefs.h +++ b/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 diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c index f6a895b2ceb7..762abdf2f283 100644 --- a/fs/bcachefs/error.c +++ b/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; + struct fsck_err_state *s; - if (test_bit(BCH_FS_FSCK_DONE, &c->flags)) { - va_start(args, fmt); - vprintk(fmt, args); - va_end(args); - - 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); } diff --git a/fs/bcachefs/error.h b/fs/bcachefs/error.h index b603d738c549..bbf9b6d85b4d 100644 --- a/fs/bcachefs/error.h +++ b/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; \ } \