bcachefs: Fix fsck path for refink pointersreflink_fix

The way __bch2_mark_reflink_p returns errors was clashing with returning
the number of sectors processed - we weren't returning FSCK_ERR_EXIT
correctly.

Fix this by only using the return code for errors, which actually ends
up simplifying the overall logic.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>

1 files changed, 47 insertions, 68 deletions

diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index 97151ec80c52..bdbdbd6914de 100644
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -1108,7 +1108,7 @@ static int bch2_mark_reservation(struct bch_fs *c,
 }
 
 static s64 __bch2_mark_reflink_p(struct bch_fs *c, struct bkey_s_c_reflink_p p,
-				 u64 idx, unsigned flags, size_t *r_idx)
+				 u64 *idx, unsigned flags, size_t *r_idx)
 {
 	struct reflink_gc *r;
 	int add = !(flags & BTREE_TRIGGER_OVERWRITE) ? 1 : -1;
@@ -1118,53 +1118,45 @@ static s64 __bch2_mark_reflink_p(struct bch_fs *c, struct bkey_s_c_reflink_p p,
 		r = genradix_ptr(&c->reflink_gc_table, *r_idx);
 		BUG_ON(!r);
 
-		if (idx < r->offset)
+		if (*idx < r->offset)
 			break;
 		(*r_idx)++;
 	}
 
 	if (*r_idx >= c->reflink_gc_nr ||
-	    idx < r->offset - r->size) {
-		ret = p.k->size;
+	    *idx < r->offset - r->size)
 		goto not_found;
-	}
 
 	BUG_ON((s64) r->refcount + add < 0);
 
 	r->refcount += add;
-	return r->offset - idx;
+	*idx = r->offset;
+	return 0;
 not_found:
-	if ((flags & BTREE_TRIGGER_GC) &&
-	    (flags & BTREE_TRIGGER_NOATOMIC)) {
-		/*
-		 * XXX: we're replacing the entire reflink pointer with an error
-		 * key, we should just be replacing the part that was missing:
-		 */
-		if (fsck_err(c, "%llu:%llu len %u points to nonexistent indirect extent %llu",
-			     p.k->p.inode, p.k->p.offset, p.k->size, idx)) {
-			struct bkey_i_error *new;
-
-			new = kmalloc(sizeof(*new), GFP_KERNEL);
-			if (!new) {
-				bch_err(c, "%s: error allocating new key", __func__);
-				return -ENOMEM;
-			}
+	ret = -EIO;
 
-			bkey_init(&new->k);
-			new->k.type	= KEY_TYPE_error;
-			new->k.p	= p.k->p;
-			new->k.size	= p.k->size;
-			ret = bch2_journal_key_insert(c, BTREE_ID_extents, 0, &new->k_i);
+	/*
+	 * XXX: we're replacing the entire reflink pointer with an error
+	 * key, we should just be replacing the part that was missing:
+	 */
+	if (fsck_err(c, "%llu:%llu len %u points to nonexistent indirect extent %llu",
+		     p.k->p.inode, p.k->p.offset, p.k->size, *idx)) {
+		struct bkey_i_error *new;
 
+		new = kmalloc(sizeof(*new), GFP_KERNEL);
+		if (!new) {
+			bch_err(c, "%s: error allocating new key", __func__);
+			return -ENOMEM;
 		}
-	} else {
-		bch2_fs_inconsistent(c,
-				     "%llu:%llu len %u points to nonexistent indirect extent %llu",
-				     p.k->p.inode, p.k->p.offset, p.k->size, idx);
-		bch2_inconsistent_error(c);
-		ret = -EIO;
+
+		bkey_init(&new->k);
+		new->k.type	= KEY_TYPE_error;
+		new->k.p	= p.k->p;
+		new->k.size	= p.k->size;
+		ret = bch2_journal_key_insert(c, BTREE_ID_extents, 0, &new->k_i);
 	}
 fsck_err:
+	*idx = U64_MAX;
 	return ret;
 }
 
@@ -1177,10 +1169,9 @@ static int bch2_mark_reflink_p(struct bch_fs *c,
 	struct reflink_gc *ref;
 	size_t l, r, m;
 	u64 idx = le64_to_cpu(p.v->idx) - le32_to_cpu(p.v->front_pad);
-	u64 sectors = (u64) le32_to_cpu(p.v->front_pad) +
-			    le32_to_cpu(p.v->back_pad) +
-			    p.k->size;
-	s64 ret = 0;
+	u64 end_idx = le64_to_cpu(p.v->idx) + p.k->size +
+		le32_to_cpu(p.v->back_pad);
+	int ret = 0;
 
 	BUG_ON((flags & (BTREE_TRIGGER_INSERT|BTREE_TRIGGER_OVERWRITE)) ==
 	       (BTREE_TRIGGER_INSERT|BTREE_TRIGGER_OVERWRITE));
@@ -1197,17 +1188,10 @@ static int bch2_mark_reflink_p(struct bch_fs *c,
 			r = m;
 	}
 
-	while (sectors) {
-		ret = __bch2_mark_reflink_p(c, p, idx, flags, &l);
-		if (ret <= 0)
-			return ret;
-
-		ret = min_t(s64, ret, sectors);
-		idx	+= ret;
-		sectors	-= ret;
-	}
+	while (idx < end_idx && !ret)
+		ret = __bch2_mark_reflink_p(c, p, &idx, flags, &l);
 
-	return 0;
+	return ret;
 }
 
 static int bch2_mark_key_locked(struct bch_fs *c,
@@ -1725,7 +1709,7 @@ static int bch2_trans_mark_reservation(struct btree_trans *trans,
 
 static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
 			struct bkey_s_c_reflink_p p,
-			u64 idx, unsigned flags)
+			u64 *idx, unsigned flags)
 {
 	struct bch_fs *c = trans->c;
 	struct btree_iter iter;
@@ -1733,9 +1717,9 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
 	struct bkey_i *n;
 	__le64 *refcount;
 	int add = !(flags & BTREE_TRIGGER_OVERWRITE) ? 1 : -1;
-	s64 ret;
+	int ret;
 
-	bch2_trans_iter_init(trans, &iter, BTREE_ID_reflink, POS(0, idx),
+	bch2_trans_iter_init(trans, &iter, BTREE_ID_reflink, POS(0, *idx),
 			     BTREE_ITER_INTENT|
 			     BTREE_ITER_WITH_UPDATES);
 	k = bch2_btree_iter_peek_slot(&iter);
@@ -1754,7 +1738,7 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
 	if (!refcount) {
 		bch2_fs_inconsistent(c,
 			"%llu:%llu len %u points to nonexistent indirect extent %llu",
-			p.k->p.inode, p.k->p.offset, p.k->size, idx);
+			p.k->p.inode, p.k->p.offset, p.k->size, *idx);
 		ret = -EIO;
 		goto err;
 	}
@@ -1762,7 +1746,7 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
 	if (!*refcount && (flags & BTREE_TRIGGER_OVERWRITE)) {
 		bch2_fs_inconsistent(c,
 			"%llu:%llu len %u idx %llu indirect extent refcount underflow",
-			p.k->p.inode, p.k->p.offset, p.k->size, idx);
+			p.k->p.inode, p.k->p.offset, p.k->size, *idx);
 		ret = -EIO;
 		goto err;
 	}
@@ -1794,8 +1778,11 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
 	if (ret)
 		goto err;
 
-	ret = k.k->p.offset - idx;
+	*idx = k.k->p.offset;
+	bch2_trans_iter_exit(trans, &iter);
+	return 0;
 err:
+	*idx = U64_MAX;
 	bch2_trans_iter_exit(trans, &iter);
 	return ret;
 }
@@ -1804,8 +1791,8 @@ static int bch2_trans_mark_reflink_p(struct btree_trans *trans,
 				     struct bkey_s_c k, unsigned flags)
 {
 	struct bkey_s_c_reflink_p p = bkey_s_c_to_reflink_p(k);
-	u64 idx, sectors;
-	s64 ret = 0;
+	u64 idx, end_idx;
+	int ret = 0;
 
 	if (flags & BTREE_TRIGGER_INSERT) {
 		struct bch_reflink_p *v = (struct bch_reflink_p *) p.v;
@@ -1813,22 +1800,14 @@ static int bch2_trans_mark_reflink_p(struct btree_trans *trans,
 		v->front_pad = v->back_pad = 0;
 	}
 
-	idx = le64_to_cpu(p.v->idx) - le32_to_cpu(p.v->front_pad);
-	sectors = (u64) le32_to_cpu(p.v->front_pad) +
-			le32_to_cpu(p.v->back_pad) +
-			p.k->size;
-
-	while (sectors) {
-		ret = __bch2_trans_mark_reflink_p(trans, p, idx, flags);
-		if (ret < 0)
-			return ret;
+	idx	= le64_to_cpu(p.v->idx) - le32_to_cpu(p.v->front_pad);
+	end_idx = le64_to_cpu(p.v->idx) + p.k->size +
+		le32_to_cpu(p.v->back_pad);
 
-		ret = min_t(s64, ret, sectors);
-		idx	+= ret;
-		sectors	-= ret;
-	}
+	while (idx < end_idx && !ret)
+		ret = __bch2_trans_mark_reflink_p(trans, p, &idx, flags);
 
-	return 0;
+	return ret;
 }
 
 int bch2_trans_mark_key(struct btree_trans *trans, struct bkey_s_c old,