From a97f90407d3acc4eb025b69afd65f7f56d54c5be Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Wed, 10 Apr 2024 23:22:46 -0400 Subject: 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 --- fs/bcachefs/btree_iter.c | 11 +++++++++-- fs/bcachefs/btree_locking.c | 34 +++++++++++++++++++++++++++++----- fs/bcachefs/btree_locking.h | 7 ++++--- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index bfe9f0c1e1be..5b6ef29e6354 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -1768,8 +1768,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 efe2a007b482..161ac72df350 100644 --- a/fs/bcachefs/btree_locking.c +++ b/fs/bcachefs/btree_locking.c @@ -612,16 +612,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; } @@ -631,7 +651,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 7c07f9fa9add..7e9ab778efd0 100644 --- a/fs/bcachefs/btree_locking.h +++ b/fs/bcachefs/btree_locking.h @@ -373,9 +373,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, -- cgit v1.2.3