Commit 7abda8c1 authored by Kent Overstreet's avatar Kent Overstreet Committed by Kent Overstreet

bcachefs: Fix __bch2_btree_node_lock

__bch2_btree_node_lock() was implementing the wrong lock ordering for
cached vs. non cached paths - this fixes it to match the btree path sort
order as defined by __btree_path_cmp(), and also simplifies the code
some.
Signed-off-by: default avatarKent Overstreet <kent.overstreet@gmail.com>
parent c7ce2732
...@@ -46,6 +46,9 @@ static inline int __btree_path_cmp(const struct btree_path *l, ...@@ -46,6 +46,9 @@ static inline int __btree_path_cmp(const struct btree_path *l,
struct bpos r_pos, struct bpos r_pos,
unsigned r_level) unsigned r_level)
{ {
/*
* Must match lock ordering as defined by __bch2_btree_node_lock:
*/
return cmp_int(l->btree_id, r_btree_id) ?: return cmp_int(l->btree_id, r_btree_id) ?:
cmp_int((int) l->cached, (int) r_cached) ?: cmp_int((int) l->cached, (int) r_cached) ?:
bpos_cmp(l->pos, r_pos) ?: bpos_cmp(l->pos, r_pos) ?:
...@@ -288,8 +291,8 @@ bool __bch2_btree_node_lock(struct btree_trans *trans, ...@@ -288,8 +291,8 @@ bool __bch2_btree_node_lock(struct btree_trans *trans,
six_lock_should_sleep_fn should_sleep_fn, void *p, six_lock_should_sleep_fn should_sleep_fn, void *p,
unsigned long ip) unsigned long ip)
{ {
struct btree_path *linked, *deadlock_path = NULL; struct btree_path *linked;
unsigned reason = 9; unsigned reason;
/* Check if it's safe to block: */ /* Check if it's safe to block: */
trans_for_each_path(trans, linked) { trans_for_each_path(trans, linked) {
...@@ -310,28 +313,28 @@ bool __bch2_btree_node_lock(struct btree_trans *trans, ...@@ -310,28 +313,28 @@ bool __bch2_btree_node_lock(struct btree_trans *trans,
*/ */
if (type == SIX_LOCK_intent && if (type == SIX_LOCK_intent &&
linked->nodes_locked != linked->nodes_intent_locked) { linked->nodes_locked != linked->nodes_intent_locked) {
deadlock_path = linked;
reason = 1; reason = 1;
goto deadlock;
} }
if (linked->btree_id != path->btree_id) { if (linked->btree_id != path->btree_id) {
if (linked->btree_id > path->btree_id) { if (linked->btree_id < path->btree_id)
deadlock_path = linked; continue;
reason = 3;
} reason = 3;
continue; goto deadlock;
} }
/* /*
* Within the same btree, cached paths come before non * Within the same btree, non-cached paths come before cached
* cached paths: * paths:
*/ */
if (linked->cached != path->cached) { if (linked->cached != path->cached) {
if (path->cached) { if (!linked->cached)
deadlock_path = linked; continue;
reason = 4;
} reason = 4;
continue; goto deadlock;
} }
/* /*
...@@ -340,34 +343,32 @@ bool __bch2_btree_node_lock(struct btree_trans *trans, ...@@ -340,34 +343,32 @@ bool __bch2_btree_node_lock(struct btree_trans *trans,
* we're about to lock, it must have the ancestors locked too: * we're about to lock, it must have the ancestors locked too:
*/ */
if (level > __fls(linked->nodes_locked)) { if (level > __fls(linked->nodes_locked)) {
deadlock_path = linked;
reason = 5; reason = 5;
goto deadlock;
} }
/* Must lock btree nodes in key order: */ /* Must lock btree nodes in key order: */
if (btree_node_locked(linked, level) && if (btree_node_locked(linked, level) &&
bpos_cmp(pos, btree_node_pos((void *) linked->l[level].b, bpos_cmp(pos, btree_node_pos((void *) linked->l[level].b,
linked->cached)) <= 0) { linked->cached)) <= 0) {
deadlock_path = linked;
reason = 7; reason = 7;
goto deadlock;
} }
} }
if (unlikely(deadlock_path)) {
trace_trans_restart_would_deadlock(trans->fn, ip,
trans->in_traverse_all, reason,
deadlock_path->btree_id,
deadlock_path->cached,
&deadlock_path->pos,
path->btree_id,
path->cached,
&pos);
btree_trans_restart(trans);
return false;
}
return btree_node_lock_type(trans, path, b, pos, level, return btree_node_lock_type(trans, path, b, pos, level,
type, should_sleep_fn, p); type, should_sleep_fn, p);
deadlock:
trace_trans_restart_would_deadlock(trans->fn, ip,
trans->in_traverse_all, reason,
linked->btree_id,
linked->cached,
&linked->pos,
path->btree_id,
path->cached,
&pos);
btree_trans_restart(trans);
return false;
} }
/* Btree iterator locking: */ /* Btree iterator locking: */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment