bcachefs: Fix spurious transaction restarts

The checks for lock ordering violations weren't quite right.

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

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index f62658f1b1dd509e8b0938ac3f36183bbea2e4a8..bbb125fb9d431b54a981270c9f2d76834ea718c7 100644 (file)
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -238,14 +238,32 @@ bool __bch2_btree_node_lock(struct btree *b, struct bpos pos,
                        }
                }
 
+               if (linked->btree_id != iter->btree_id) {
+                       if (linked->btree_id > iter->btree_id) {
+                               deadlock_iter = linked;
+                               reason = 3;
+                       }
+                       continue;
+               }
+
+               /*
+                * Within the same btree, cached iterators come before non
+                * cached iterators:
+                */
+               if (btree_iter_is_cached(linked) != btree_iter_is_cached(iter)) {
+                       if (btree_iter_is_cached(iter)) {
+                               deadlock_iter = linked;
+                               reason = 4;
+                       }
+                       continue;
+               }
+
                /*
                 * Interior nodes must be locked before their descendants: if
                 * another iterator has possible descendants locked of the node
                 * we're about to lock, it must have the ancestors locked too:
                 */
-               if (linked->btree_id == iter->btree_id &&
-                   btree_iter_is_cached(linked) == btree_iter_is_cached(iter) &&
-                   level > __fls(linked->nodes_locked)) {
+               if (level > __fls(linked->nodes_locked)) {
                        if (!(trans->nounlock)) {
                                linked->locks_want =
                                        max(level + 1, max_t(unsigned,
@@ -253,27 +271,20 @@ bool __bch2_btree_node_lock(struct btree *b, struct bpos pos,
                                            iter->locks_want));
                                if (!btree_iter_get_locks(linked, true, false)) {
                                        deadlock_iter = linked;
-                                       reason = 3;
+                                       reason = 5;
                                }
                        } else {
                                deadlock_iter = linked;
-                               reason = 4;
+                               reason = 6;
                        }
                }
 
                /* Must lock btree nodes in key order: */
-               if ((cmp_int(iter->btree_id, linked->btree_id) ?:
-                    -cmp_int(btree_iter_type(iter), btree_iter_type(linked))) < 0) {
-                       deadlock_iter = linked;
-                       reason = 5;
-               }
-
-               if (iter->btree_id == linked->btree_id &&
-                   btree_node_locked(linked, level) &&
+               if (btree_node_locked(linked, level) &&
                    bkey_cmp(pos, btree_node_pos((void *) linked->l[level].b,
                                                 btree_iter_type(linked))) <= 0) {
                        deadlock_iter = linked;
-                       reason = 6;
+                       reason = 7;
                }
 
                /*

diff --git a/fs/bcachefs/btree_iter.h b/fs/bcachefs/btree_iter.h
index f80e09255f68cf4a1dc6e0fd8ee93403154ea4bd..f7a73619c85b2ff00680e10c95a952bcfebb7c7c 100644 (file)
--- a/fs/bcachefs/btree_iter.h
+++ b/fs/bcachefs/btree_iter.h
@@ -182,7 +182,7 @@ static inline int btree_iter_lock_cmp(const struct btree_iter *l,
                                      const struct btree_iter *r)
 {
        return   cmp_int(l->btree_id, r->btree_id) ?:
-               -cmp_int(btree_iter_type(l), btree_iter_type(r)) ?:
+               -cmp_int(btree_iter_is_cached(l), btree_iter_is_cached(r)) ?:
                 bkey_cmp(l->pos, r->pos);
 }