bcachefs: bch2_btree_path_upgrade_norestart()

btree_path_get_locks() leaves a path unlocked (and with error pointers
for nodes) on failure - this used to be necessary for path_traverse() to
correctly lock all needed levels.

But it's not needed anymore, and we need a new helper that doesn't break
future locking invariants and leave us in a bad state (unlocked)  when
used on a should_be_locked path, because bch2_path_get() has no way of
returning a transaction restart.

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

3 files changed, 42 insertions, 10 deletions

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index 3fc696c622c6..bfd97d08a15a 100644
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -1819,8 +1819,15 @@ btree_path_idx_t bch2_path_get(struct btree_trans *trans,
 	 */
 
 	locks_want = min(locks_want, BTREE_MAX_DEPTH);
-	if (locks_want > path->locks_want)
-		bch2_btree_path_upgrade_noupgrade_sibs(trans, path, locks_want, NULL);
+	if (locks_want > path->locks_want) {
+		if (!path->should_be_locked) {
+			bch2_btree_path_upgrade_norestart(trans, path, locks_want);
+		} else if (!bch2_btree_path_upgrade_nounlock(trans, path, locks_want)) {
+			path_idx = __bch2_btree_path_make_mut(trans, path_idx, intent, _THIS_IP_);
+			path = trans->paths + path_idx;
+			bch2_btree_path_upgrade_norestart(trans, path, locks_want);
+		}
+	}
 
 	return path_idx;
 }
diff --git a/fs/bcachefs/btree_locking.c b/fs/bcachefs/btree_locking.c
index c110015b3741..9cd496fc05a1 100644
--- a/fs/bcachefs/btree_locking.c
+++ b/fs/bcachefs/btree_locking.c
@@ -604,16 +604,36 @@ int __bch2_btree_path_relock(struct btree_trans *trans,
 	return 0;
 }
 
-bool bch2_btree_path_upgrade_noupgrade_sibs(struct btree_trans *trans,
+bool bch2_btree_path_upgrade_nounlock(struct btree_trans *trans,
+				      struct btree_path *path,
+				      unsigned new_locks_want)
+{
+	while (path->locks_want < new_locks_want) {
+		unsigned l = path->locks_want++;
+
+		if (!btree_path_node(path, l))
+			break;
+
+		if (!bch2_btree_node_upgrade(trans, path, l)) {
+			path->locks_want = l;
+			break;
+		}
+	}
+
+	bch2_btree_path_verify_locks(path);
+	return path->locks_want == new_locks_want;
+}
+
+bool bch2_btree_path_upgrade_norestart(struct btree_trans *trans,
 			       struct btree_path *path,
-			       unsigned new_locks_want,
-			       struct get_locks_fail *f)
+			       unsigned new_locks_want)
 {
 	EBUG_ON(path->locks_want >= new_locks_want);
+	EBUG_ON(path->should_be_locked);
 
 	path->locks_want = new_locks_want;
 
-	bool ret = btree_path_get_locks(trans, path, true, f);
+	bool ret = btree_path_get_locks(trans, path, true, NULL);
 	bch2_trans_verify_locks(trans);
 	return ret;
 }
@@ -623,7 +643,11 @@ bool __bch2_btree_path_upgrade(struct btree_trans *trans,
 			       unsigned new_locks_want,
 			       struct get_locks_fail *f)
 {
-	bool ret = bch2_btree_path_upgrade_noupgrade_sibs(trans, path, new_locks_want, f);
+	EBUG_ON(path->locks_want >= new_locks_want);
+
+	path->locks_want = new_locks_want;
+
+	bool ret = btree_path_get_locks(trans, path, true, f);
 	if (ret)
 		goto out;
 
diff --git a/fs/bcachefs/btree_locking.h b/fs/bcachefs/btree_locking.h
index 43fe0499ff84..568d4794e562 100644
--- a/fs/bcachefs/btree_locking.h
+++ b/fs/bcachefs/btree_locking.h
@@ -389,9 +389,10 @@ static inline bool bch2_btree_node_relock_notrace(struct btree_trans *trans,
 
 /* upgrade */
 
-bool bch2_btree_path_upgrade_noupgrade_sibs(struct btree_trans *,
-			       struct btree_path *, unsigned,
-			       struct get_locks_fail *);
+bool bch2_btree_path_upgrade_nounlock(struct btree_trans *,
+				      struct btree_path *, unsigned);
+bool bch2_btree_path_upgrade_norestart(struct btree_trans *,
+				       struct btree_path *, unsigned);
 
 bool __bch2_btree_path_upgrade(struct btree_trans *,
 			       struct btree_path *, unsigned,