bcachefs: Improve iter->should_be_locked

Adding iter->should_be_locked introduced a regression where it ended up
not being set on the iterator passed to bch2_btree_update_start(), which
is definitely not what we want.

This patch requires it to be set when calling bch2_trans_update(), and
adds various fixups to make that happen.

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

11 files changed, 50 insertions, 32 deletions

diff --git a/fs/bcachefs/btree_iter.h b/fs/bcachefs/btree_iter.h
index 27c685a482ec..6efea281d87f 100644
--- a/fs/bcachefs/btree_iter.h
+++ b/fs/bcachefs/btree_iter.h
@@ -176,6 +176,12 @@ static inline void bch2_btree_iter_set_pos(struct btree_iter *iter, struct bpos
 	iter->should_be_locked = false;
 }
 
+static inline void bch2_btree_iter_set_pos_to_extent_start(struct btree_iter *iter)
+{
+	BUG_ON(!(iter->flags & BTREE_ITER_IS_EXTENTS));
+	iter->pos = bkey_start_pos(&iter->k);
+}
+
 static inline struct btree_iter *btree_iter_child(struct btree_iter *iter)
 {
 	return iter->child_idx == U8_MAX ? NULL
diff --git a/fs/bcachefs/btree_update_interior.c b/fs/bcachefs/btree_update_interior.c
index bb01b036c7a2..e35e842efe81 100644
--- a/fs/bcachefs/btree_update_interior.c
+++ b/fs/bcachefs/btree_update_interior.c
@@ -937,6 +937,8 @@ bch2_btree_update_start(struct btree_iter *iter, unsigned level,
 	int journal_flags = 0;
 	int ret = 0;
 
+	BUG_ON(!iter->should_be_locked);
+
 	if (flags & BTREE_INSERT_JOURNAL_RESERVED)
 		journal_flags |= JOURNAL_RES_GET_RESERVED;
 
diff --git a/fs/bcachefs/btree_update_leaf.c b/fs/bcachefs/btree_update_leaf.c
index 634e25e94425..00706b952630 100644
--- a/fs/bcachefs/btree_update_leaf.c
+++ b/fs/bcachefs/btree_update_leaf.c
@@ -855,6 +855,10 @@ static int extent_handle_overwrites(struct btree_trans *trans,
 			update_iter = bch2_trans_get_iter(trans, i->btree_id, update->k.p,
 							  BTREE_ITER_NOT_EXTENTS|
 							  BTREE_ITER_INTENT);
+			ret = bch2_btree_iter_traverse(update_iter);
+			if (ret)
+				goto out;
+
 			bch2_trans_update(trans, update_iter, update, i->trigger_flags);
 			bch2_trans_iter_put(trans, update_iter);
 		}
@@ -1039,6 +1043,7 @@ int bch2_trans_update(struct btree_trans *trans, struct btree_iter *iter,
 	int ret = 0;
 
 	BUG_ON(trans->nr_updates >= BTREE_ITER_MAX);
+	BUG_ON(!iter->should_be_locked);
 
 #ifdef CONFIG_BCACHEFS_DEBUG
 	trans_for_each_update(trans, i)
@@ -1102,7 +1107,8 @@ int __bch2_btree_insert(struct btree_trans *trans,
 	iter = bch2_trans_get_iter(trans, id, bkey_start_pos(&k->k),
 				   BTREE_ITER_INTENT);
 
-	ret = bch2_trans_update(trans, iter, k, 0);
+	ret   = bch2_btree_iter_traverse(iter) ?:
+		bch2_trans_update(trans, iter, k, 0);
 	bch2_trans_iter_put(trans, iter);
 	return ret;
 }
@@ -1147,13 +1153,12 @@ int bch2_btree_delete_range_trans(struct btree_trans *trans, enum btree_id id,
 
 	iter = bch2_trans_get_iter(trans, id, start, BTREE_ITER_INTENT);
 retry:
-	while ((k = bch2_btree_iter_peek(iter)).k &&
+	while ((bch2_trans_begin(trans),
+	       (k = bch2_btree_iter_peek(iter)).k) &&
 	       !(ret = bkey_err(k)) &&
 	       bkey_cmp(iter->pos, end) < 0) {
 		struct bkey_i delete;
 
-		bch2_trans_begin(trans);
-
 		bkey_init(&delete.k);
 
 		/*
diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index c427744a665f..3dd206d3b546 100644
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -1817,7 +1817,7 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
 		set_bkey_val_u64s(&n->k, 0);
 	}
 
-	bch2_btree_iter_set_pos(iter, bkey_start_pos(k.k));
+	bch2_btree_iter_set_pos_to_extent_start(iter);
 	ret = bch2_trans_update(trans, iter, n, 0);
 	if (ret)
 		goto err;
diff --git a/fs/bcachefs/extent_update.c b/fs/bcachefs/extent_update.c
index ef4aaf1c30ed..4a8dd085f7fb 100644
--- a/fs/bcachefs/extent_update.c
+++ b/fs/bcachefs/extent_update.c
@@ -104,6 +104,10 @@ int bch2_extent_atomic_end(struct btree_iter *iter,
 	unsigned nr_iters = 0;
 	int ret;
 
+	ret = bch2_btree_iter_traverse(iter);
+	if (ret)
+		return ret;
+
 	*end = insert->k.p;
 
 	/* extent_update_to_keys(): */
diff --git a/fs/bcachefs/fs-common.c b/fs/bcachefs/fs-common.c
index 00a63fecb976..60c54438074e 100644
--- a/fs/bcachefs/fs-common.c
+++ b/fs/bcachefs/fs-common.c
@@ -85,7 +85,8 @@ int bch2_create_trans(struct btree_trans *trans, u64 dir_inum,
 	inode_iter->snapshot = U32_MAX;
 	bch2_btree_iter_set_pos(inode_iter, SPOS(0, new_inode->bi_inum, U32_MAX));
 
-	ret = bch2_inode_write(trans, inode_iter, new_inode);
+	ret   = bch2_btree_iter_traverse(inode_iter) ?:
+		bch2_inode_write(trans, inode_iter, new_inode);
 err:
 	bch2_trans_iter_put(trans, inode_iter);
 	bch2_trans_iter_put(trans, dir_iter);
diff --git a/fs/bcachefs/fs-io.c b/fs/bcachefs/fs-io.c
index e39e22581e4b..0ffc3971d1b2 100644
--- a/fs/bcachefs/fs-io.c
+++ b/fs/bcachefs/fs-io.c
@@ -2611,7 +2611,8 @@ reassemble:
 			BUG_ON(ret);
 		}
 
-		ret =   bch2_trans_update(&trans, del, &delete, trigger_flags) ?:
+		ret =   bch2_btree_iter_traverse(del) ?:
+			bch2_trans_update(&trans, del, &delete, trigger_flags) ?:
 			bch2_trans_update(&trans, dst, copy.k, trigger_flags) ?:
 			bch2_trans_commit(&trans, &disk_res,
 					  &inode->ei_journal_seq,
diff --git a/fs/bcachefs/fsck.c b/fs/bcachefs/fsck.c
index 1bb595f4003a..7ea1a41ac637 100644
--- a/fs/bcachefs/fsck.c
+++ b/fs/bcachefs/fsck.c
@@ -78,7 +78,8 @@ static int __write_inode(struct btree_trans *trans,
 		bch2_trans_get_iter(trans, BTREE_ID_inodes,
 				    SPOS(0, inode->bi_inum, snapshot),
 				    BTREE_ITER_INTENT);
-	int ret = bch2_inode_write(trans, inode_iter, inode);
+	int ret = bch2_btree_iter_traverse(inode_iter) ?:
+		bch2_inode_write(trans, inode_iter, inode);
 	bch2_trans_iter_put(trans, inode_iter);
 	return ret;
 }
@@ -305,7 +306,8 @@ static int hash_redo_key(struct btree_trans *trans,
 
 	bkey_init(&delete->k);
 	delete->k.p = k_iter->pos;
-	return  bch2_trans_update(trans, k_iter, delete, 0) ?:
+	return  bch2_btree_iter_traverse(k_iter) ?:
+		bch2_trans_update(trans, k_iter, delete, 0) ?:
 		bch2_hash_set(trans, desc, hash_info, k_iter->pos.inode, tmp, 0);
 }
 
@@ -491,6 +493,7 @@ static int check_inode(struct btree_trans *trans,
 		ret = __bch2_trans_do(trans, NULL, NULL,
 				      BTREE_INSERT_NOFAIL|
 				      BTREE_INSERT_LAZY_RW,
+				bch2_btree_iter_traverse(iter) ?:
 				bch2_inode_write(trans, iter, &u));
 		if (ret)
 			bch_err(c, "error in fsck: error %i "
@@ -562,7 +565,8 @@ static int fix_overlapping_extent(struct btree_trans *trans,
 				   BTREE_ITER_INTENT|BTREE_ITER_NOT_EXTENTS);
 
 	BUG_ON(iter->flags & BTREE_ITER_IS_EXTENTS);
-	ret   = bch2_trans_update(trans, iter, u, BTREE_TRIGGER_NORUN) ?:
+	ret   = bch2_btree_iter_traverse(iter) ?:
+		bch2_trans_update(trans, iter, u, BTREE_TRIGGER_NORUN) ?:
 		bch2_trans_commit(trans, NULL, NULL,
 				  BTREE_INSERT_NOFAIL|
 				  BTREE_INSERT_LAZY_RW);
@@ -886,6 +890,7 @@ retry:
 			ret = __bch2_trans_do(&trans, NULL, NULL,
 					      BTREE_INSERT_NOFAIL|
 					      BTREE_INSERT_LAZY_RW,
+				bch2_btree_iter_traverse(iter) ?:
 				bch2_trans_update(&trans, iter, &n->k_i, 0));
 			kfree(n);
 			if (ret)
@@ -1338,6 +1343,7 @@ static int check_nlinks_update_hardlinks(struct bch_fs *c,
 			ret = __bch2_trans_do(&trans, NULL, NULL,
 					      BTREE_INSERT_NOFAIL|
 					      BTREE_INSERT_LAZY_RW,
+					      bch2_btree_iter_traverse(iter) ?:
 					bch2_inode_write(&trans, iter, &u));
 			if (ret)
 				bch_err(c, "error in fsck: error %i updating inode", ret);
diff --git a/fs/bcachefs/recovery.c b/fs/bcachefs/recovery.c
index f32414171aab..c6fa4ca31ae9 100644
--- a/fs/bcachefs/recovery.c
+++ b/fs/bcachefs/recovery.c
@@ -509,16 +509,8 @@ static int __bch2_journal_replay_key(struct btree_trans *trans,
 
 	iter = bch2_trans_get_node_iter(trans, id, k->k.p,
 					BTREE_MAX_DEPTH, level,
-					BTREE_ITER_INTENT);
-
-	/*
-	 * iter->flags & BTREE_ITER_IS_EXTENTS triggers the update path to run
-	 * extent_handle_overwrites() and extent_update_to_keys() - but we don't
-	 * want that here, journal replay is supposed to treat extents like
-	 * regular keys:
-	 */
-	BUG_ON(iter->flags & BTREE_ITER_IS_EXTENTS);
-
+					BTREE_ITER_INTENT|
+					BTREE_ITER_NOT_EXTENTS);
 	ret   = bch2_btree_iter_traverse(iter) ?:
 		bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
 	bch2_trans_iter_put(trans, iter);
@@ -546,7 +538,8 @@ static int __bch2_alloc_replay_key(struct btree_trans *trans, struct bkey_i *k)
 				   BTREE_ITER_CACHED|
 				   BTREE_ITER_CACHED_NOFILL|
 				   BTREE_ITER_INTENT);
-	ret = bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
+	ret   = bch2_btree_iter_traverse(iter) ?:
+		bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
 	bch2_trans_iter_put(trans, iter);
 	return ret;
 }
diff --git a/fs/bcachefs/reflink.c b/fs/bcachefs/reflink.c
index ba700810a4be..ebf391245470 100644
--- a/fs/bcachefs/reflink.c
+++ b/fs/bcachefs/reflink.c
@@ -142,7 +142,7 @@ static int bch2_make_extent_indirect(struct btree_trans *trans,
 		goto err;
 
 	/* rewind iter to start of hole, if necessary: */
-	bch2_btree_iter_set_pos(reflink_iter, bkey_start_pos(k.k));
+	bch2_btree_iter_set_pos_to_extent_start(reflink_iter);
 
 	r_v = bch2_trans_kmalloc(trans, sizeof(__le64) + bkey_bytes(&orig->k));
 	ret = PTR_ERR_OR_ZERO(r_v);
@@ -257,11 +257,11 @@ s64 bch2_remap_range(struct bch_fs *c,
 		}
 
 		if (src_k.k->type != KEY_TYPE_reflink_p) {
+			bch2_btree_iter_set_pos_to_extent_start(src_iter);
+
 			bch2_bkey_buf_reassemble(&new_src, c, src_k);
 			src_k = bkey_i_to_s_c(new_src.k);
 
-			bch2_btree_iter_set_pos(src_iter, bkey_start_pos(src_k.k));
-
 			ret = bch2_make_extent_indirect(&trans, src_iter,
 						new_src.k);
 			if (ret)
diff --git a/fs/bcachefs/tests.c b/fs/bcachefs/tests.c
index fa9f600fc17c..a8b8e3a072ad 100644
--- a/fs/bcachefs/tests.c
+++ b/fs/bcachefs/tests.c
@@ -40,13 +40,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
 	iter = bch2_trans_get_iter(&trans, BTREE_ID_xattrs, k.k.p,
 				   BTREE_ITER_INTENT);
 
-	ret = bch2_btree_iter_traverse(iter);
-	if (ret) {
-		bch_err(c, "lookup error in test_delete: %i", ret);
-		goto err;
-	}
-
 	ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+		bch2_btree_iter_traverse(iter) ?:
 		bch2_trans_update(&trans, iter, &k.k_i, 0));
 	if (ret) {
 		bch_err(c, "update error in test_delete: %i", ret);
@@ -55,7 +50,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
 
 	pr_info("deleting once");
 	ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-			 bch2_btree_delete_at(&trans, iter, 0));
+		bch2_btree_iter_traverse(iter) ?:
+		bch2_btree_delete_at(&trans, iter, 0));
 	if (ret) {
 		bch_err(c, "delete error (first) in test_delete: %i", ret);
 		goto err;
@@ -63,7 +59,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
 
 	pr_info("deleting twice");
 	ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-			 bch2_btree_delete_at(&trans, iter, 0));
+		bch2_btree_iter_traverse(iter) ?:
+		bch2_btree_delete_at(&trans, iter, 0));
 	if (ret) {
 		bch_err(c, "delete error (second) in test_delete: %i", ret);
 		goto err;
@@ -591,6 +588,7 @@ static int rand_mixed(struct bch_fs *c, u64 nr)
 			k.k.p = iter->pos;
 
 			ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+				bch2_btree_iter_traverse(iter) ?:
 				bch2_trans_update(&trans, iter, &k.k_i, 0));
 			if (ret) {
 				bch_err(c, "update error in rand_mixed: %i", ret);
@@ -671,6 +669,7 @@ static int seq_insert(struct bch_fs *c, u64 nr)
 		insert.k.p = iter->pos;
 
 		ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+			bch2_btree_iter_traverse(iter) ?:
 			bch2_trans_update(&trans, iter, &insert.k_i, 0));
 		if (ret) {
 			bch_err(c, "error in seq_insert: %i", ret);
@@ -719,6 +718,7 @@ static int seq_overwrite(struct bch_fs *c, u64 nr)
 		bkey_reassemble(&u.k_i, k);
 
 		ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+			bch2_btree_iter_traverse(iter) ?:
 			bch2_trans_update(&trans, iter, &u.k_i, 0));
 		if (ret) {
 			bch_err(c, "error in seq_overwrite: %i", ret);