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@gmail.com>

3 files changed, 64 insertions, 26 deletions

diff --git a/fs/bcachefs/btree_gc.c b/fs/bcachefs/btree_gc.c
index 6c2253ef9252..5c77a9552a16 100644
--- a/fs/bcachefs/btree_gc.c
+++ b/fs/bcachefs/btree_gc.c
@@ -142,18 +142,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 bffb247d201a..247d0263f7e2 100644
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -445,12 +445,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]);
 
@@ -505,14 +499,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 799bfb3c96d8..a4bab66d8d17 100644
--- 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)
 {