bcachefs: Fix duplicate paths left by bch2_path_put()

bch2_path_put() is supposed to drop paths that aren't needed on
transaction restart, or to hold locks that we're supposed to keep until
transaction commit: but it was failing to free paths in some cases that
it should have, leading to transaction path overflows with lots of
duplicate paths.

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

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index bd6e35fcbdbdebaaf2e08eb6d8adfad20448aa9a..837cdcc5c77cd5359f603945742a3ff26c8513dc 100644 (file)
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -424,13 +424,17 @@ int bch2_btree_path_relock_intent(struct btree_trans *trans,
        return 0;
 }
 
-noinline __flatten
-static int __bch2_btree_path_relock(struct btree_trans *trans,
+__flatten
+static bool bch2_btree_path_relock_norestart(struct btree_trans *trans,
                        struct btree_path *path, unsigned long trace_ip)
 {
-       bool ret = btree_path_get_locks(trans, path, false);
+       return btree_path_get_locks(trans, path, false);
+}
 
-       if (!ret) {
+static int __bch2_btree_path_relock(struct btree_trans *trans,
+                       struct btree_path *path, unsigned long trace_ip)
+{
+       if (!bch2_btree_path_relock_norestart(trans, path, trace_ip)) {
                trace_trans_restart_relock_path(trans, trace_ip, path);
                return btree_trans_restart(trans, BCH_ERR_transaction_restart_relock_path);
        }
@@ -1743,30 +1747,30 @@ out:
 
 static struct btree_path *have_path_at_pos(struct btree_trans *trans, struct btree_path *path)
 {
-       struct btree_path *next;
+       struct btree_path *sib;
 
-       next = prev_btree_path(trans, path);
-       if (next && !btree_path_cmp(next, path))
-               return next;
+       sib = prev_btree_path(trans, path);
+       if (sib && !btree_path_cmp(sib, path))
+               return sib;
 
-       next = next_btree_path(trans, path);
-       if (next && !btree_path_cmp(next, path))
-               return next;
+       sib = next_btree_path(trans, path);
+       if (sib && !btree_path_cmp(sib, path))
+               return sib;
 
        return NULL;
 }
 
 static struct btree_path *have_node_at_pos(struct btree_trans *trans, struct btree_path *path)
 {
-       struct btree_path *next;
+       struct btree_path *sib;
 
-       next = prev_btree_path(trans, path);
-       if (next && next->level == path->level && path_l(next)->b == path_l(path)->b)
-               return next;
+       sib = prev_btree_path(trans, path);
+       if (sib && sib->level == path->level && path_l(sib)->b == path_l(path)->b)
+               return sib;
 
-       next = next_btree_path(trans, path);
-       if (next && next->level == path->level && path_l(next)->b == path_l(path)->b)
-               return next;
+       sib = next_btree_path(trans, path);
+       if (sib && sib->level == path->level && path_l(sib)->b == path_l(path)->b)
+               return sib;
 
        return NULL;
 }
@@ -1788,26 +1792,23 @@ void bch2_path_put(struct btree_trans *trans, struct btree_path *path, bool inte
        if (!__btree_path_put(path, intent))
                return;
 
-       /*
-        * Perhaps instead we should check for duplicate paths in traverse_all:
-        */
-       if (path->preserve &&
-           (dup = have_path_at_pos(trans, path))) {
-               dup->preserve = true;
-               path->preserve = false;
-               goto free;
-       }
+       dup = path->preserve
+               ? have_path_at_pos(trans, path)
+               : have_node_at_pos(trans, path);
+
+       if (!dup && !(!path->preserve && !is_btree_node(path, path->level)))
+               return;
 
-       if (!path->preserve &&
-           (dup = have_node_at_pos(trans, path)))
-               goto free;
-       return;
-free:
        if (path->should_be_locked &&
-           !btree_node_locked(dup, path->level))
+           !trans->restarted &&
+           (!dup || !bch2_btree_path_relock_norestart(trans, dup, _THIS_IP_)))
                return;
 
-       dup->should_be_locked |= path->should_be_locked;
+       if (dup) {
+               dup->preserve           |= path->preserve;
+               dup->should_be_locked   |= path->should_be_locked;
+       }
+
        __bch2_path_free(trans, path);
 }