diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c index a0e9e14e3fa5..aeb058c800cd 100644 --- a/fs/bcachefs/btree_cache.c +++ b/fs/bcachefs/btree_cache.c @@ -151,8 +151,6 @@ void bch2_btree_node_hash_remove(struct btree_cache *bc, struct btree *b) /* Cause future lookups for this node to fail: */ b->hash_val = 0; - - six_lock_wakeup_all(&b->c.lock); } int __bch2_btree_node_hash_insert(struct btree_cache *bc, struct btree *b) @@ -755,16 +753,6 @@ static noinline struct btree *bch2_btree_node_fill(struct bch_fs *c, return b; } -static int lock_node_check_fn(struct six_lock *lock, void *p) -{ - struct btree *b = container_of(lock, struct btree, c.lock); - const struct bkey_i *k = p; - - if (b->hash_val != btree_ptr_hash_val(k)) - return BCH_ERR_lock_fail_node_reused; - return 0; -} - static noinline void btree_bad_header(struct bch_fs *c, struct btree *b) { struct printbuf buf = PRINTBUF; @@ -886,15 +874,11 @@ struct btree *bch2_btree_node_get(struct btree_trans *trans, struct btree_path * if (btree_node_read_locked(path, level + 1)) btree_node_unlock(trans, path, level + 1); - ret = btree_node_lock(trans, path, &b->c, k->k.p, level, lock_type, - lock_node_check_fn, (void *) k, trace_ip); - if (unlikely(ret)) { - if (bch2_err_matches(ret, BCH_ERR_lock_fail_node_reused)) - goto retry; - if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) - return ERR_PTR(ret); - BUG(); - } + ret = btree_node_lock(trans, path, &b->c, level, lock_type, trace_ip); + if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) + return ERR_PTR(ret); + + BUG_ON(ret); if (unlikely(b->hash_val != btree_ptr_hash_val(k) || b->c.level != level || @@ -1000,13 +984,10 @@ struct btree *bch2_btree_node_get_noiter(struct btree_trans *trans, } else { lock_node: ret = btree_node_lock_nopath(trans, &b->c, SIX_LOCK_read); - if (unlikely(ret)) { - if (bch2_err_matches(ret, BCH_ERR_lock_fail_node_reused)) - goto retry; - if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) - return ERR_PTR(ret); - BUG(); - } + if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) + return ERR_PTR(ret); + + BUG_ON(ret); if (unlikely(b->hash_val != btree_ptr_hash_val(k) || b->c.btree_id != btree_id || diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index ece80d7914b2..6f4af13cf9e4 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -689,16 +689,6 @@ void bch2_trans_node_reinit_iter(struct btree_trans *trans, struct btree *b) /* Btree path: traverse, set_pos: */ -static int lock_root_check_fn(struct six_lock *lock, void *p) -{ - struct btree *b = container_of(lock, struct btree, c.lock); - struct btree **rootp = p; - - if (b != *rootp) - return BCH_ERR_lock_fail_root_changed; - return 0; -} - static inline int btree_path_lock_root(struct btree_trans *trans, struct btree_path *path, unsigned depth_want, @@ -730,10 +720,8 @@ static inline int btree_path_lock_root(struct btree_trans *trans, } lock_type = __btree_lock_want(path, path->level); - ret = btree_node_lock(trans, path, &b->c, SPOS_MAX, - path->level, lock_type, - lock_root_check_fn, rootp, - trace_ip); + ret = btree_node_lock(trans, path, &b->c, + path->level, lock_type, trace_ip); if (unlikely(ret)) { if (bch2_err_matches(ret, BCH_ERR_lock_fail_root_changed)) continue; @@ -939,7 +927,7 @@ static int btree_path_traverse_one(struct btree_trans *, struct btree_path *, static int bch2_btree_path_traverse_all(struct btree_trans *trans) { struct bch_fs *c = trans->c; - struct btree_path *path, *prev; + struct btree_path *path; unsigned long trace_ip = _RET_IP_; int i, ret = 0; @@ -948,7 +936,6 @@ static int bch2_btree_path_traverse_all(struct btree_trans *trans) trans->in_traverse_all = true; retry_all: - prev = NULL; trans->restarted = 0; trans_for_each_path(trans, path) @@ -956,18 +943,6 @@ static int bch2_btree_path_traverse_all(struct btree_trans *trans) btree_trans_sort_paths(trans); - trans_for_each_path_inorder_reverse(trans, path, i) { - if (prev) { - if (path->btree_id == prev->btree_id && - path->locks_want < prev->locks_want) - __bch2_btree_path_upgrade(trans, path, prev->locks_want); - else if (!path->locks_want && prev->locks_want) - __bch2_btree_path_upgrade(trans, path, 1); - } - - prev = path; - } - bch2_trans_unlock(trans); cond_resched(); @@ -3026,16 +3001,7 @@ void bch2_btree_trans_to_text(struct printbuf *out, struct btree_trans *trans) b = READ_ONCE(trans->locking); if (b) { - path = &trans->paths[trans->locking_path_idx]; - prt_printf(out, " locking path %u %c l=%u %c %s:", - trans->locking_path_idx, - path->cached ? 'c' : 'b', - trans->locking_level, - lock_types[trans->locking_wait.lock_want], - bch2_btree_ids[trans->locking_btree_id]); - bch2_bpos_to_text(out, trans->locking_pos); - - prt_printf(out, " node "); + prt_printf(out, " locking node "); bch2_btree_path_node_to_text(out, b); prt_printf(out, "\n"); } diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c index 977c523359a5..1a88d1d79699 100644 --- a/fs/bcachefs/btree_key_cache.c +++ b/fs/bcachefs/btree_key_cache.c @@ -398,17 +398,6 @@ static int btree_key_cache_fill(struct btree_trans *trans, return ret; } -static int bkey_cached_check_fn(struct six_lock *lock, void *p) -{ - struct bkey_cached *ck = container_of(lock, struct bkey_cached, c.lock); - const struct btree_path *path = p; - - if (ck->key.btree_id != path->btree_id && - bpos_cmp(ck->key.pos, path->pos)) - return BCH_ERR_lock_fail_node_reused; - return 0; -} - __flatten int bch2_btree_path_traverse_cached(struct btree_trans *trans, struct btree_path *path, unsigned flags) @@ -440,16 +429,12 @@ int bch2_btree_path_traverse_cached(struct btree_trans *trans, struct btree_path } else { enum six_lock_type lock_want = __btree_lock_want(path, 0); - ret = btree_node_lock(trans, path, (void *) ck, path->pos, 0, - lock_want, - bkey_cached_check_fn, path, _THIS_IP_); - if (ret) { - if (bch2_err_matches(ret, BCH_ERR_lock_fail_node_reused)) - goto retry; - if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) - goto err; - BUG(); - } + ret = btree_node_lock(trans, path, (void *) ck, 0, + lock_want, _THIS_IP_); + if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) + goto err; + + BUG_ON(ret); if (ck->key.btree_id != path->btree_id || bpos_cmp(ck->key.pos, path->pos)) { diff --git a/fs/bcachefs/btree_locking.c b/fs/bcachefs/btree_locking.c index e270579d3622..11f83a936dc7 100644 --- a/fs/bcachefs/btree_locking.c +++ b/fs/bcachefs/btree_locking.c @@ -92,6 +92,7 @@ static int abort_lock(struct lock_graph *g, struct trans_waiting_for_lock *i) int ret; if (i == g->g) { + trace_and_count(i->trans->c, trans_restart_would_deadlock, i->trans, _RET_IP_); ret = btree_trans_restart(i->trans, BCH_ERR_transaction_restart_would_deadlock); } else { i->trans->lock_must_abort = true; @@ -216,8 +217,10 @@ int bch2_check_for_deadlock(struct btree_trans *trans, struct printbuf *cycle) struct btree_path *path; int ret; - if (trans->lock_must_abort) + if (trans->lock_must_abort) { + trace_and_count(trans->c, trans_restart_would_deadlock, trans, _RET_IP_); return btree_trans_restart(trans, BCH_ERR_transaction_restart_would_deadlock); + } g.nr = 0; ret = lock_graph_descend(&g, trans, cycle); @@ -294,7 +297,7 @@ int bch2_six_check_for_deadlock(struct six_lock *lock, void *p) return bch2_check_for_deadlock(trans, NULL); } -int __bch2_btree_node_lock_write(struct btree_trans *trans, +int __bch2_btree_node_lock_write(struct btree_trans *trans, struct btree_path *path, struct btree_bkey_cached_common *b, bool lock_may_not_fail) { @@ -311,99 +314,12 @@ int __bch2_btree_node_lock_write(struct btree_trans *trans, ret = __btree_node_lock_nopath(trans, b, SIX_LOCK_write, lock_may_not_fail); six_lock_readers_add(&b->lock, readers); + if (ret) + mark_btree_node_locked_noreset(path, b->level, SIX_LOCK_intent); + return ret; } -static inline bool path_has_read_locks(struct btree_path *path) -{ - unsigned l; - - for (l = 0; l < BTREE_MAX_DEPTH; l++) - if (btree_node_read_locked(path, l)) - return true; - return false; -} - -/* Slowpath: */ -int __bch2_btree_node_lock(struct btree_trans *trans, - struct btree_path *path, - struct btree_bkey_cached_common *b, - struct bpos pos, unsigned level, - enum six_lock_type type, - six_lock_should_sleep_fn should_sleep_fn, void *p, - unsigned long ip) -{ - struct btree_path *linked; - unsigned reason; - - /* Check if it's safe to block: */ - trans_for_each_path(trans, linked) { - if (!linked->nodes_locked) - continue; - - /* - * Can't block taking an intent lock if we have _any_ nodes read - * locked: - * - * - Our read lock blocks another thread with an intent lock on - * the same node from getting a write lock, and thus from - * dropping its intent lock - * - * - And the other thread may have multiple nodes intent locked: - * both the node we want to intent lock, and the node we - * already have read locked - deadlock: - */ - if (type == SIX_LOCK_intent && - path_has_read_locks(linked)) { - reason = 1; - goto deadlock; - } - - if (linked->btree_id != path->btree_id) { - if (linked->btree_id < path->btree_id) - continue; - - reason = 3; - goto deadlock; - } - - /* - * Within the same btree, non-cached paths come before cached - * paths: - */ - if (linked->cached != path->cached) { - if (!linked->cached) - continue; - - reason = 4; - goto deadlock; - } - - /* - * Interior nodes must be locked before their descendants: if - * another path has possible descendants locked of the node - * we're about to lock, it must have the ancestors locked too: - */ - if (level > btree_path_highest_level_locked(linked)) { - reason = 5; - goto deadlock; - } - - /* Must lock btree nodes in key order: */ - if (btree_node_locked(linked, level) && - bpos_cmp(pos, btree_node_pos(&linked->l[level].b->c)) <= 0) { - reason = 7; - goto deadlock; - } - } - - return btree_node_lock_type(trans, path, b, pos, level, - type, should_sleep_fn, p); -deadlock: - trace_and_count(trans->c, trans_restart_would_deadlock, trans, ip, reason, linked, path, &pos); - return btree_trans_restart(trans, BCH_ERR_transaction_restart_would_deadlock); -} - /* relock */ static inline bool btree_path_get_locks(struct btree_trans *trans, diff --git a/fs/bcachefs/btree_locking.h b/fs/bcachefs/btree_locking.h index 86f68b26cc94..6d8df25bf076 100644 --- a/fs/bcachefs/btree_locking.h +++ b/fs/bcachefs/btree_locking.h @@ -195,8 +195,8 @@ static inline int __btree_node_lock_nopath(struct btree_trans *trans, int ret; trans->lock_may_not_fail = lock_may_not_fail; - trans->locking = b; trans->lock_must_abort = false; + trans->locking = b; ret = six_lock_type_waiter(&b->lock, type, &trans->locking_wait, bch2_six_check_for_deadlock, trans); @@ -222,26 +222,6 @@ static inline void btree_node_lock_nopath_nofail(struct btree_trans *trans, BUG_ON(ret); } -static inline int btree_node_lock_type(struct btree_trans *trans, - struct btree_path *path, - struct btree_bkey_cached_common *b, - struct bpos pos, unsigned level, - enum six_lock_type type, - six_lock_should_sleep_fn should_sleep_fn, void *p) -{ - if (six_trylock_type(&b->lock, type)) - return 0; - - trans->locking_path_idx = path->idx; - trans->locking_pos = pos; - trans->locking_btree_id = path->btree_id; - trans->locking_level = level; - trans->lock_may_not_fail = false; - trans->locking = b; - return six_lock_type_waiter(&b->lock, type, &trans->locking_wait, - bch2_six_check_for_deadlock, trans); -} - /* * Lock a btree node if we already have it locked on one of our linked * iterators: @@ -263,19 +243,11 @@ static inline bool btree_node_lock_increment(struct btree_trans *trans, return false; } -int __bch2_btree_node_lock(struct btree_trans *, struct btree_path *, - struct btree_bkey_cached_common *, - struct bpos, unsigned, - enum six_lock_type, - six_lock_should_sleep_fn, void *, - unsigned long); - static inline int btree_node_lock(struct btree_trans *trans, struct btree_path *path, struct btree_bkey_cached_common *b, - struct bpos pos, unsigned level, + unsigned level, enum six_lock_type type, - six_lock_should_sleep_fn should_sleep_fn, void *p, unsigned long ip) { int ret = 0; @@ -285,8 +257,7 @@ static inline int btree_node_lock(struct btree_trans *trans, if (likely(six_trylock_type(&b->lock, type)) || btree_node_lock_increment(trans, b, level, type) || - !(ret = __bch2_btree_node_lock(trans, path, b, pos, level, type, - should_sleep_fn, p, ip))) { + !(ret = btree_node_lock_nopath(trans, b, type))) { #ifdef CONFIG_BCACHEFS_LOCK_TIME_STATS path->l[b->level].lock_taken_time = ktime_get_ns(); #endif @@ -295,15 +266,14 @@ static inline int btree_node_lock(struct btree_trans *trans, return ret; } -int __bch2_btree_node_lock_write(struct btree_trans *, struct btree_bkey_cached_common *, bool); +int __bch2_btree_node_lock_write(struct btree_trans *, struct btree_path *, + struct btree_bkey_cached_common *b, bool); static inline int __btree_node_lock_write(struct btree_trans *trans, struct btree_path *path, struct btree_bkey_cached_common *b, bool lock_may_not_fail) { - int ret; - EBUG_ON(&path->l[b->level].b->c != b); EBUG_ON(path->l[b->level].lock_seq != b->lock.state.seq); EBUG_ON(!btree_node_intent_locked(path, b->level)); @@ -315,13 +285,9 @@ static inline int __btree_node_lock_write(struct btree_trans *trans, */ mark_btree_node_locked_noreset(path, b->level, SIX_LOCK_write); - ret = likely(six_trylock_write(&b->lock)) + return likely(six_trylock_write(&b->lock)) ? 0 - : __bch2_btree_node_lock_write(trans, b, lock_may_not_fail); - if (ret) - mark_btree_node_locked_noreset(path, b->level, SIX_LOCK_intent); - - return ret; + : __bch2_btree_node_lock_write(trans, path, b, lock_may_not_fail); } static inline void bch2_btree_node_lock_write_nofail(struct btree_trans *trans, diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h index 578cf8fa3d2f..2b57e6d6ed31 100644 --- a/fs/bcachefs/btree_types.h +++ b/fs/bcachefs/btree_types.h @@ -391,10 +391,6 @@ struct btree_trans { struct list_head list; u64 last_begin_time; - unsigned locking_path_idx; - struct bpos locking_pos; - u8 locking_btree_id; - u8 locking_level; u8 lock_may_not_fail; u8 lock_must_abort; struct btree_bkey_cached_common *locking; diff --git a/fs/bcachefs/btree_update_leaf.c b/fs/bcachefs/btree_update_leaf.c index bf3177a3a420..7b21971fa13d 100644 --- a/fs/bcachefs/btree_update_leaf.c +++ b/fs/bcachefs/btree_update_leaf.c @@ -796,23 +796,6 @@ static inline void normalize_read_intent_locks(struct btree_trans *trans) bch2_trans_verify_locks(trans); } -static inline bool have_conflicting_read_lock(struct btree_trans *trans, struct btree_path *pos) -{ - struct btree_path *path; - unsigned i; - - trans_for_each_path_inorder(trans, path, i) { - //if (path == pos) - // break; - - if (btree_node_read_locked(path, path->level) && - !bch2_btree_path_upgrade_noupgrade_sibs(trans, path, path->level + 1)) - return true; - } - - return false; -} - static inline int trans_lock_write(struct btree_trans *trans) { struct btree_insert_entry *i; @@ -822,31 +805,15 @@ static inline int trans_lock_write(struct btree_trans *trans) if (same_leaf_as_prev(trans, i)) continue; - /* - * six locks are unfair, and read locks block while a thread - * wants a write lock: thus, we need to tell the cycle detector - * we have a write lock _before_ taking the lock: - */ - mark_btree_node_locked_noreset(i->path, i->level, SIX_LOCK_write); - - if (!six_trylock_write(&insert_l(i)->b->c.lock)) { - if (have_conflicting_read_lock(trans, i->path)) - goto fail; - - ret = btree_node_lock_type(trans, i->path, - &insert_l(i)->b->c, - i->path->pos, i->level, - SIX_LOCK_write, NULL, NULL); - BUG_ON(ret); - } + ret = bch2_btree_node_lock_write(trans, i->path, &insert_l(i)->b->c); + if (ret) + goto fail; bch2_btree_node_prep_for_write(trans, i->path, insert_l(i)->b); } return 0; fail: - mark_btree_node_locked_noreset(i->path, i->level, SIX_LOCK_intent); - while (--i >= trans->updates) { if (same_leaf_as_prev(trans, i)) continue; diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h index 1ea004f1adbb..bf7ae99d9cce 100644 --- a/fs/bcachefs/errcode.h +++ b/fs/bcachefs/errcode.h @@ -52,7 +52,6 @@ x(BCH_ERR_no_btree_node, no_btree_node_down) \ x(BCH_ERR_no_btree_node, no_btree_node_init) \ x(BCH_ERR_no_btree_node, no_btree_node_cached) \ - x(0, lock_fail_node_reused) \ x(0, lock_fail_root_changed) \ x(0, journal_reclaim_would_deadlock) \ x(0, fsck) \ diff --git a/fs/bcachefs/trace.h b/fs/bcachefs/trace.h index 35c40678f4b5..69e142d8b651 100644 --- a/fs/bcachefs/trace.h +++ b/fs/bcachefs/trace.h @@ -1012,57 +1012,10 @@ DEFINE_EVENT(transaction_restart_iter, trans_restart_memory_allocation_failure, TP_ARGS(trans, caller_ip, path) ); -TRACE_EVENT(trans_restart_would_deadlock, +DEFINE_EVENT(transaction_event, trans_restart_would_deadlock, TP_PROTO(struct btree_trans *trans, - unsigned long caller_ip, - unsigned reason, - struct btree_path *have, - struct btree_path *want, - struct bpos *want_pos), - TP_ARGS(trans, caller_ip, reason, - have, want, want_pos), - - TP_STRUCT__entry( - __array(char, trans_fn, 32 ) - __field(unsigned long, caller_ip ) - __field(u8, in_traverse_all ) - __field(u8, reason ) - __field(u8, have_btree_id ) - __field(u8, have_type ) - __field(u8, want_btree_id ) - __field(u8, want_type ) - TRACE_BPOS_entries(have_pos) - TRACE_BPOS_entries(want_pos) - ), - - TP_fast_assign( - strlcpy(__entry->trans_fn, trans->fn, sizeof(__entry->trans_fn)); - __entry->caller_ip = caller_ip; - __entry->in_traverse_all = trans->in_traverse_all; - __entry->reason = reason; - __entry->have_btree_id = have->btree_id; - __entry->have_type = have->cached; - __entry->want_btree_id = want->btree_id; - __entry->want_type = want->cached; - TRACE_BPOS_assign(have_pos, have->pos); - TRACE_BPOS_assign(want_pos, *want_pos); - ), - - TP_printk("%s %pS traverse_all %u because %u have %u:%u %llu:%llu:%u want %u:%u %llu:%llu:%u", - __entry->trans_fn, - (void *) __entry->caller_ip, - __entry->in_traverse_all, - __entry->reason, - __entry->have_btree_id, - __entry->have_type, - __entry->have_pos_inode, - __entry->have_pos_offset, - __entry->have_pos_snapshot, - __entry->want_btree_id, - __entry->want_type, - __entry->want_pos_inode, - __entry->want_pos_offset, - __entry->want_pos_snapshot) + unsigned long caller_ip), + TP_ARGS(trans, caller_ip) ); DEFINE_EVENT(transaction_event, trans_restart_would_deadlock_recursion_limit,