From 9ef846a7a13bc6daa3fc431acab5c13d7fb4aa84 Mon Sep 17 00:00:00 2001
From: Kent Overstreet <kent.overstreet@gmail.com>
Date: Wed, 3 Jun 2020 18:27:07 -0400
Subject: [PATCH] bcachefs: Improve assorted error messages
This also consolidates the various checks in bch2_mark_pointer() and
bch2_trans_mark_pointer().
Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
fs/bcachefs/btree_io.c | 15 +--
fs/bcachefs/buckets.c | 243 ++++++++++++++++++++---------------------
fs/bcachefs/error.h | 1 +
fs/bcachefs/extents.c | 2 +-
fs/bcachefs/fsck.c | 2 +-
5 files changed, 127 insertions(+), 136 deletions(-)
diff --git a/fs/bcachefs/btree_io.c b/fs/bcachefs/btree_io.c
index a5888de327fc4..5325c24548f90 100644
--- a/fs/bcachefs/btree_io.c
+++ b/fs/bcachefs/btree_io.c
@@ -631,14 +631,14 @@ static void btree_err_msg(struct printbuf *out, struct bch_fs *c,
struct btree *b, struct bset *i,
unsigned offset, int write)
{
- pr_buf(out, "error validating btree node %s"
- "at btree %u level %u/%u\n"
- "pos %llu:%llu node offset %u",
+ pr_buf(out, "error validating btree node %sat btree %u level %u/%u\n"
+ "pos ",
write ? "before write " : "",
b->c.btree_id, b->c.level,
- c->btree_roots[b->c.btree_id].level,
- b->key.k.p.inode, b->key.k.p.offset,
- b->written);
+ c->btree_roots[b->c.btree_id].level);
+ bch2_bkey_val_to_text(out, c, bkey_i_to_s_c(&b->key));
+
+ pr_buf(out, " node offset %u", b->written);
if (i)
pr_buf(out, " bset u64s %u", le16_to_cpu(i->u64s));
}
@@ -944,7 +944,8 @@ int bch2_btree_node_read_done(struct bch_fs *c, struct btree *b, bool have_retry
btree_err_on(b->data->keys.seq != bp->seq,
BTREE_ERR_MUST_RETRY, c, b, NULL,
- "got wrong btree node");
+ "got wrong btree node (seq %llx want %llx)",
+ b->data->keys.seq, bp->seq);
}
while (b->written < c->opts.btree_node_size) {
diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index ebdbdd049f50a..4074bc073cfe1 100644
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -918,61 +918,117 @@ static void bucket_set_stripe(struct bch_fs *c,
}
}
-static bool bch2_mark_pointer(struct bch_fs *c,
- struct extent_ptr_decoded p,
- s64 sectors, enum bch_data_type data_type,
- struct bch_fs_usage *fs_usage,
- u64 journal_seq, unsigned flags)
+static int __mark_pointer(struct bch_fs *c, struct bkey_s_c k,
+ struct extent_ptr_decoded p,
+ s64 sectors, enum bch_data_type ptr_data_type,
+ u8 bucket_gen, u8 *bucket_data_type,
+ u16 *dirty_sectors, u16 *cached_sectors)
+{
+ u16 *dst_sectors = !p.ptr.cached
+ ? dirty_sectors
+ : cached_sectors;
+ u16 orig_sectors = *dst_sectors;
+ char buf[200];
+
+ if (gen_after(p.ptr.gen, bucket_gen)) {
+ bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
+ "bucket %u:%zu gen %u data type %s: ptr gen %u newer than bucket gen\n"
+ "while marking %s",
+ p.ptr.dev, PTR_BUCKET_NR(bch_dev_bkey_exists(c, p.ptr.dev), &p.ptr),
+ bucket_gen,
+ bch2_data_types[*bucket_data_type ?: ptr_data_type],
+ p.ptr.gen,
+ (bch2_bkey_val_to_text(&PBUF(buf), c, k), buf));
+ return -EIO;
+ }
+
+ if (gen_cmp(bucket_gen, p.ptr.gen) >= 96U) {
+ bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
+ "bucket %u:%zu gen %u data type %s: ptr gen %u too stale\n"
+ "while marking %s",
+ p.ptr.dev, PTR_BUCKET_NR(bch_dev_bkey_exists(c, p.ptr.dev), &p.ptr),
+ bucket_gen,
+ bch2_data_types[*bucket_data_type ?: ptr_data_type],
+ p.ptr.gen,
+ (bch2_bkey_val_to_text(&PBUF(buf), c, k), buf));
+ return -EIO;
+ }
+
+ if (bucket_gen != p.ptr.gen && !p.ptr.cached) {
+ bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
+ "bucket %u:%zu gen %u data type %s: stale dirty ptr (gen %u)\n"
+ "while marking %s",
+ p.ptr.dev, PTR_BUCKET_NR(bch_dev_bkey_exists(c, p.ptr.dev), &p.ptr),
+ bucket_gen,
+ bch2_data_types[*bucket_data_type ?: ptr_data_type],
+ p.ptr.gen,
+ (bch2_bkey_val_to_text(&PBUF(buf), c, k), buf));
+ return -EIO;
+ }
+
+ if (bucket_gen != p.ptr.gen)
+ return 1;
+
+ if (*bucket_data_type && *bucket_data_type != ptr_data_type) {
+ bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
+ "bucket %u:%zu gen %u different types of data in same bucket: %s, %s\n"
+ "while marking %s",
+ p.ptr.dev, PTR_BUCKET_NR(bch_dev_bkey_exists(c, p.ptr.dev), &p.ptr),
+ bucket_gen,
+ bch2_data_types[*bucket_data_type],
+ bch2_data_types[ptr_data_type],
+ (bch2_bkey_val_to_text(&PBUF(buf), c, k), buf));
+ return -EIO;
+ }
+
+ if (checked_add(*dst_sectors, sectors)) {
+ bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
+ "bucket %u:%zu gen %u data type %s sector count overflow: %u + %lli > U16_MAX\n"
+ "while marking %s",
+ p.ptr.dev, PTR_BUCKET_NR(bch_dev_bkey_exists(c, p.ptr.dev), &p.ptr),
+ bucket_gen,
+ bch2_data_types[*bucket_data_type ?: ptr_data_type],
+ orig_sectors, sectors,
+ (bch2_bkey_val_to_text(&PBUF(buf), c, k), buf));
+ return -EIO;
+ }
+
+ *bucket_data_type = *dirty_sectors || *cached_sectors
+ ? ptr_data_type : 0;
+ return 0;
+}
+
+static int bch2_mark_pointer(struct bch_fs *c, struct bkey_s_c k,
+ struct extent_ptr_decoded p,
+ s64 sectors, enum bch_data_type data_type,
+ struct bch_fs_usage *fs_usage,
+ u64 journal_seq, unsigned flags)
{
bool gc = flags & BTREE_TRIGGER_GC;
struct bucket_mark old, new;
struct bch_dev *ca = bch_dev_bkey_exists(c, p.ptr.dev);
struct bucket *g = PTR_BUCKET(ca, &p.ptr, gc);
- u16 *dst_sectors, orig_sectors;
- bool overflow;
+ u8 bucket_data_type;
u64 v;
+ int ret;
v = atomic64_read(&g->_mark.v);
do {
new.v.counter = old.v.counter = v;
+ bucket_data_type = new.data_type;
- /*
- * Check this after reading bucket mark to guard against
- * the allocator invalidating a bucket after we've already
- * checked the gen
- */
- if (gen_after(p.ptr.gen, new.gen)) {
- bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
- "pointer gen in the future");
- return true;
- }
-
- if (new.gen != p.ptr.gen) {
- /* XXX write repair code for this */
- if (!p.ptr.cached &&
- test_bit(JOURNAL_REPLAY_DONE, &c->journal.flags))
- bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
- "stale dirty pointer");
- return true;
- }
-
- dst_sectors = !p.ptr.cached
- ? &new.dirty_sectors
- : &new.cached_sectors;
- orig_sectors = *dst_sectors;
-
- overflow = checked_add(*dst_sectors, sectors);
+ ret = __mark_pointer(c, k, p, sectors, data_type, new.gen,
+ &bucket_data_type,
+ &new.dirty_sectors,
+ &new.cached_sectors);
+ if (ret)
+ return ret;
- if (!new.dirty_sectors &&
- !new.cached_sectors) {
- new.data_type = 0;
+ new.data_type = bucket_data_type;
- if (journal_seq) {
- new.journal_seq_valid = 1;
- new.journal_seq = journal_seq;
- }
- } else {
- new.data_type = data_type;
+ if (journal_seq) {
+ new.journal_seq_valid = 1;
+ new.journal_seq = journal_seq;
}
if (flags & BTREE_TRIGGER_NOATOMIC) {
@@ -983,25 +1039,11 @@ static bool bch2_mark_pointer(struct bch_fs *c,
old.v.counter,
new.v.counter)) != old.v.counter);
- if (old.data_type && old.data_type != data_type)
- bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
- "bucket %u:%zu gen %u different types of data in same bucket: %s, %s",
- p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
- new.gen,
- bch2_data_types[old.data_type],
- bch2_data_types[data_type]);
-
- bch2_fs_inconsistent_on(overflow, c,
- "bucket %u:%zu gen %u data type %s sector count overflow: %u + %lli > U16_MAX",
- p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), new.gen,
- bch2_data_types[old.data_type ?: data_type],
- orig_sectors, sectors);
-
bch2_dev_usage_update(c, ca, fs_usage, old, new, gc);
BUG_ON(!gc && bucket_became_unavailable(old, new));
- return false;
+ return 0;
}
static int bch2_mark_stripe_ptr(struct bch_fs *c,
@@ -1065,6 +1107,7 @@ static int bch2_mark_extent(struct bch_fs *c, struct bkey_s_c k,
struct extent_ptr_decoded p;
struct bch_replicas_padded r;
s64 dirty_sectors = 0;
+ bool stale;
int ret;
r.e.data_type = data_type;
@@ -1077,8 +1120,13 @@ static int bch2_mark_extent(struct bch_fs *c, struct bkey_s_c k,
s64 disk_sectors = data_type == BCH_DATA_BTREE
? sectors
: ptr_disk_sectors_delta(p, offset, sectors, flags);
- bool stale = bch2_mark_pointer(c, p, disk_sectors, data_type,
- fs_usage, journal_seq, flags);
+
+ ret = bch2_mark_pointer(c, k, p, disk_sectors, data_type,
+ fs_usage, journal_seq, flags);
+ if (ret < 0)
+ return ret;
+
+ stale = ret > 0;
if (p.ptr.cached) {
if (!stale)
@@ -1439,25 +1487,24 @@ static int trans_get_key(struct btree_trans *trans,
}
static int bch2_trans_mark_pointer(struct btree_trans *trans,
- struct extent_ptr_decoded p,
+ struct bkey_s_c k, struct extent_ptr_decoded p,
s64 sectors, enum bch_data_type data_type)
{
struct bch_fs *c = trans->c;
struct bch_dev *ca = bch_dev_bkey_exists(c, p.ptr.dev);
struct btree_iter *iter;
- struct bkey_s_c k;
+ struct bkey_s_c k_a;
struct bkey_alloc_unpacked u;
struct bkey_i_alloc *a;
- u16 *dst_sectors, orig_sectors;
int ret;
ret = trans_get_key(trans, BTREE_ID_ALLOC,
POS(p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr)),
- &iter, &k);
+ &iter, &k_a);
if (ret < 0)
return ret;
- if (k.k->type != KEY_TYPE_alloc ||
+ if (k_a.k->type != KEY_TYPE_alloc ||
(!ret && unlikely(!test_bit(BCH_FS_ALLOC_WRITTEN, &c->flags)))) {
/*
* During journal replay, and if gc repairs alloc info at
@@ -1474,71 +1521,13 @@ static int bch2_trans_mark_pointer(struct btree_trans *trans,
u = alloc_mem_to_key(g, m);
percpu_up_read(&c->mark_lock);
} else {
- u = bch2_alloc_unpack(k);
- }
-
- if (u.gen != p.ptr.gen) {
- ret = 1;
-
- if (gen_after(p.ptr.gen, u.gen)) {
- bch2_fs_inconsistent(c,
- "bucket %llu:%llu gen %u data type %s: ptr gen %u newer than bucket gen",
- iter->pos.inode, iter->pos.offset, u.gen,
- bch2_data_types[u.data_type ?: data_type],
- p.ptr.gen);
- ret = -EIO;
- }
-
- if (gen_cmp(u.gen, p.ptr.gen) >= 96U) {
- bch2_fs_inconsistent(c,
- "bucket %llu:%llu gen %u data type %s: ptr gen %u too stale",
- iter->pos.inode, iter->pos.offset, u.gen,
- bch2_data_types[u.data_type ?: data_type],
- p.ptr.gen);
- ret = -EIO;
- }
-
- if (!p.ptr.cached) {
- bch2_fs_inconsistent(c,
- "bucket %llu:%llu gen %u data type %s: stale dirty ptr (gen %u)",
- iter->pos.inode, iter->pos.offset, u.gen,
- bch2_data_types[u.data_type ?: data_type],
- p.ptr.gen);
- ret = -EIO;
- }
-
- goto out;
- }
-
- if (u.data_type && u.data_type != data_type) {
- bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
- "bucket %llu:%llu gen %u different types of data in same bucket: %s, %s",
- iter->pos.inode, iter->pos.offset,
- u.gen,
- bch2_data_types[u.data_type],
- bch2_data_types[data_type]);
- ret = -1;
- goto out;
+ u = bch2_alloc_unpack(k_a);
}
- dst_sectors = !p.ptr.cached
- ? &u.dirty_sectors
- : &u.cached_sectors;
- orig_sectors = *dst_sectors;
-
- if (checked_add(*dst_sectors, sectors)) {
- bch2_fs_inconsistent(c,
- "bucket %llu:%llu gen %u data type %s sector count overflow: %u + %lli > U16_MAX",
- iter->pos.inode, iter->pos.offset, u.gen,
- bch2_data_types[u.data_type ?: data_type],
- orig_sectors, sectors);
- /* return an error indicating that we need full fsck */
- ret = -EIO;
+ ret = __mark_pointer(c, k, p, sectors, data_type, u.gen, &u.data_type,
+ &u.dirty_sectors, &u.cached_sectors);
+ if (ret)
goto out;
- }
-
- u.data_type = u.dirty_sectors || u.cached_sectors
- ? data_type : 0;
a = bch2_trans_kmalloc(trans, BKEY_ALLOC_U64s_MAX * 8);
ret = PTR_ERR_OR_ZERO(a);
@@ -1623,7 +1612,7 @@ static int bch2_trans_mark_extent(struct btree_trans *trans,
? sectors
: ptr_disk_sectors_delta(p, offset, sectors, flags);
- ret = bch2_trans_mark_pointer(trans, p, disk_sectors,
+ ret = bch2_trans_mark_pointer(trans, k, p, disk_sectors,
data_type);
if (ret < 0)
return ret;
diff --git a/fs/bcachefs/error.h b/fs/bcachefs/error.h
index de319794ccd10..94b53312fbbda 100644
--- a/fs/bcachefs/error.h
+++ b/fs/bcachefs/error.h
@@ -102,6 +102,7 @@ struct fsck_err_state {
#define FSCK_CAN_IGNORE (1 << 1)
#define FSCK_NEED_FSCK (1 << 2)
+__printf(3, 4) __cold
enum fsck_err_ret bch2_fsck_err(struct bch_fs *,
unsigned, const char *, ...);
void bch2_flush_fsck_errs(struct bch_fs *);
diff --git a/fs/bcachefs/extents.c b/fs/bcachefs/extents.c
index 52beaab227ef7..62eb3b1e2cbf6 100644
--- a/fs/bcachefs/extents.c
+++ b/fs/bcachefs/extents.c
@@ -219,7 +219,7 @@ void bch2_btree_ptr_v2_to_text(struct printbuf *out, struct bch_fs *c,
{
struct bkey_s_c_btree_ptr_v2 bp = bkey_s_c_to_btree_ptr_v2(k);
- pr_buf(out, "seq %llu sectors %u written %u min_key ",
+ pr_buf(out, "seq %llx sectors %u written %u min_key ",
le64_to_cpu(bp.v->seq),
le16_to_cpu(bp.v->sectors),
le16_to_cpu(bp.v->sectors_written));
diff --git a/fs/bcachefs/fsck.c b/fs/bcachefs/fsck.c
index 3ab621c62c43d..c6ca5968a2e07 100644
--- a/fs/bcachefs/fsck.c
+++ b/fs/bcachefs/fsck.c
@@ -1169,7 +1169,7 @@ static int check_inode_nlink(struct bch_fs *c,
}
if (!S_ISDIR(u->bi_mode) && link->dir_count) {
- need_fsck_err(c, "non directory with subdirectories",
+ need_fsck_err(c, "non directory with subdirectories (inum %llu)",
u->bi_inum);
return 0;
}
--
2.30.2