From 7e7ae6ca57d210dcedc4268323c9471d97194111 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Thu, 5 Nov 2020 20:49:08 -0500 Subject: [PATCH] bcachefs: Fix spurious transaction restarts The checks for lock ordering violations weren't quite right. Signed-off-by: Kent Overstreet Signed-off-by: Kent Overstreet --- fs/bcachefs/btree_iter.c | 39 +++++++++++++++++++++++++-------------- fs/bcachefs/btree_iter.h | 2 +- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index f62658f1b1dd5..bbb125fb9d431 100644 --- 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 f80e09255f68c..f7a73619c85b2 100644 --- 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); } -- 2.30.2