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>

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);
 			}
 		}

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);

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)
 {