bcachefs: Improve pointer marking checks and error messages

Importantly, we don't want to use bch2_fs_inconsistent_on() for errors
that fsck can repair, becuase that will just put us in RO mode and
prevent fsck from actually fixing stuff. Probably want to get rid of it
in the future.

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

diff --git a/fs/bcachefs/btree_gc.c b/fs/bcachefs/btree_gc.c
index 68ae08f86f3392a1d132ef1e6d11ee359184d9fb..c4a7ff5f8a089d925da0d3b4c874bf55f06bb12d 100644 (file)
--- a/fs/bcachefs/btree_gc.c
+++ b/fs/bcachefs/btree_gc.c
@@ -144,18 +144,24 @@ static int bch2_gc_mark_key(struct bch_fs *c, struct bkey_s_c k,
                        struct bucket *g2 = PTR_BUCKET(ca, ptr, false);
 
                        if (mustfix_fsck_err_on(!g->gen_valid, c,
-                                       "found ptr with missing gen in alloc btree,\n"
-                                       "type %u gen %u",
-                                       k.k->type, ptr->gen)) {
+                                       "bucket %u:%zu data type %s ptr gen %u missing in alloc btree",
+                                       ptr->dev, PTR_BUCKET_NR(ca, ptr),
+                                       bch2_data_types[ptr_data_type(k.k, ptr)],
+                                       ptr->gen)) {
                                g2->_mark.gen   = g->_mark.gen          = ptr->gen;
                                g2->gen_valid   = g->gen_valid          = true;
                        }
 
                        if (mustfix_fsck_err_on(gen_cmp(ptr->gen, g->mark.gen) > 0, c,
-                                       "%u ptr gen in the future: %u > %u",
-                                       k.k->type, ptr->gen, g->mark.gen)) {
+                                       "bucket %u:%zu data type %s ptr gen in the future: %u > %u",
+                                       ptr->dev, PTR_BUCKET_NR(ca, ptr),
+                                       bch2_data_types[ptr_data_type(k.k, ptr)],
+                                       ptr->gen, g->mark.gen)) {
                                g2->_mark.gen   = g->_mark.gen          = ptr->gen;
                                g2->gen_valid   = g->gen_valid          = true;
+                               g2->_mark.data_type             = 0;
+                               g2->_mark.dirty_sectors         = 0;
+                               g2->_mark.cached_sectors        = 0;
                                set_bit(BCH_FS_FIXED_GENS, &c->flags);
                        }
                }

diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index 61df32cf9f5bc56d1aec7009914a681e9d043be0..d732ec77e281faf5a7a6cce5bb135055e58270da 100644 (file)
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -447,12 +447,6 @@ static void bch2_dev_usage_update(struct bch_fs *c, struct bch_dev *ca,
 
        percpu_rwsem_assert_held(&c->mark_lock);
 
-       bch2_fs_inconsistent_on(old.data_type && new.data_type &&
-                               old.data_type != new.data_type, c,
-               "different types of data in same bucket: %s, %s",
-               bch2_data_types[old.data_type],
-               bch2_data_types[new.data_type]);
-
        preempt_disable();
        dev_usage = this_cpu_ptr(ca->usage[gc]);
 
@@ -504,14 +498,6 @@ void bch2_dev_usage_from_buckets(struct bch_fs *c)
        }
 }
 
-#define bucket_data_cmpxchg(c, ca, fs_usage, g, new, expr)     \
-({                                                             \
-       struct bucket_mark _old = bucket_cmpxchg(g, new, expr); \
-                                                               \
-       bch2_dev_usage_update(c, ca, fs_usage, _old, new, gc);  \
-       _old;                                                   \
-})
-
 static inline void update_replicas(struct bch_fs *c,
                                   struct bch_fs_usage *fs_usage,
                                   struct bch_replicas_entry *r,
@@ -633,7 +619,7 @@ static int __bch2_invalidate_bucket(struct bch_fs *c, struct bch_dev *ca,
        struct bucket *g = __bucket(ca, b, gc);
        struct bucket_mark old, new;
 
-       old = bucket_data_cmpxchg(c, ca, fs_usage, g, new, ({
+       old = bucket_cmpxchg(g, new, ({
                BUG_ON(!is_available_bucket(new));
 
                new.owned_by_allocator  = true;
@@ -643,6 +629,8 @@ static int __bch2_invalidate_bucket(struct bch_fs *c, struct bch_dev *ca,
                new.gen++;
        }));
 
+       bch2_dev_usage_update(c, ca, fs_usage, old, new, gc);
+
        if (old.cached_sectors)
                update_cached_sectors(c, fs_usage, ca->dev_idx,
                                      -((s64) old.cached_sectors));
@@ -671,10 +659,12 @@ static int __bch2_mark_alloc_bucket(struct bch_fs *c, struct bch_dev *ca,
        struct bucket *g = __bucket(ca, b, gc);
        struct bucket_mark old, new;
 
-       old = bucket_data_cmpxchg(c, ca, fs_usage, g, new, ({
+       old = bucket_cmpxchg(g, new, ({
                new.owned_by_allocator  = owned_by_allocator;
        }));
 
+       bch2_dev_usage_update(c, ca, fs_usage, old, new, gc);
+
        BUG_ON(!gc &&
               !owned_by_allocator && !old.owned_by_allocator);
 
@@ -780,6 +770,12 @@ static int __bch2_mark_metadata_bucket(struct bch_fs *c, struct bch_dev *ca,
                overflow = checked_add(new.dirty_sectors, sectors);
        }));
 
+       bch2_fs_inconsistent_on(old.data_type &&
+                               old.data_type != type, c,
+               "different types of data in same bucket: %s, %s",
+               bch2_data_types[old.data_type],
+               bch2_data_types[type]);
+
        bch2_fs_inconsistent_on(overflow, c,
                "bucket sector count overflow: %u + %u > U16_MAX",
                old.dirty_sectors, sectors);
@@ -849,7 +845,7 @@ static void bucket_set_stripe(struct bch_fs *c,
                struct bucket *g = PTR_BUCKET(ca, ptr, gc);
                struct bucket_mark new, old;
 
-               old = bucket_data_cmpxchg(c, ca, fs_usage, g, new, ({
+               old = bucket_cmpxchg(g, new, ({
                        new.stripe                      = enabled;
                        if (journal_seq) {
                                new.journal_seq_valid   = 1;
@@ -857,6 +853,8 @@ static void bucket_set_stripe(struct bch_fs *c,
                        }
                }));
 
+               bch2_dev_usage_update(c, ca, fs_usage, old, new, gc);
+
                /*
                 * XXX write repair code for these, flag stripe as possibly bad
                 */
@@ -901,7 +899,13 @@ static bool bch2_mark_pointer(struct bch_fs *c,
                 * the allocator invalidating a bucket after we've already
                 * checked the gen
                 */
-               if (gen_after(new.gen, p.ptr.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))
@@ -935,6 +939,14 @@ 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 sector count overflow: %u + %lli > U16_MAX",
                !p.ptr.cached
@@ -1444,9 +1456,9 @@ static int bch2_trans_mark_pointer(struct btree_trans *trans,
                 * Unless we're already updating that key:
                 */
                if (k.k->type != KEY_TYPE_alloc) {
-                       bch_err_ratelimited(c, "pointer to nonexistent bucket %llu:%llu",
-                                           iter->pos.inode,
-                                           iter->pos.offset);
+                       bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
+                                     "pointer to nonexistent bucket %llu:%llu",
+                                     iter->pos.inode, iter->pos.offset);
                        ret = -1;
                        goto out;
                }
@@ -1459,6 +1471,17 @@ static int bch2_trans_mark_pointer(struct btree_trans *trans,
                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;
+       }
+
        if (!p.ptr.cached) {
                old = u.dirty_sectors;
                overflow = checked_add(u.dirty_sectors, sectors);

diff --git a/fs/bcachefs/buckets.h b/fs/bcachefs/buckets.h
index 296d250e58dd8a980201035ad12cf7524e34cae2..e93cda51d70569029838be5d108fcae878df4cf8 100644 (file)
--- a/fs/bcachefs/buckets.h
+++ b/fs/bcachefs/buckets.h
@@ -94,6 +94,15 @@ static inline struct bucket *PTR_BUCKET(struct bch_dev *ca,
        return __bucket(ca, PTR_BUCKET_NR(ca, ptr), gc);
 }
 
+static inline enum bch_data_type ptr_data_type(const struct bkey *k,
+                                              const struct bch_extent_ptr *ptr)
+{
+       if (k->type == KEY_TYPE_btree_ptr)
+               return BCH_DATA_BTREE;
+
+       return ptr->cached ? BCH_DATA_CACHED : BCH_DATA_USER;
+}
+
 static inline struct bucket_mark ptr_bucket_mark(struct bch_dev *ca,
                                                 const struct bch_extent_ptr *ptr)
 {