diff options
author | Kent Overstreet <kent.overstreet@gmail.com> | 2022-08-11 01:55:53 +0300 |
---|---|---|
committer | Kent Overstreet <kent.overstreet@linux.dev> | 2023-10-23 00:09:37 +0300 |
commit | fd211bc71c9b4093ed39d4fc93294f9ff423febc (patch) | |
tree | 8ce776949db181cf6f2bbd234a22d7d5ff24558f | |
parent | 31301dd46975b2423fd38fc64bc58728d89dbcac (diff) | |
download | linux-fd211bc71c9b4093ed39d4fc93294f9ff423febc.tar.xz |
bcachefs: Don't set should_be_locked on paths that aren't locked
It doesn't make any sense to set should_be_locked on btree_paths that
aren't locked, and is often a bug - this patch adds assertions and fixes
some of those bugs.
Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
-rw-r--r-- | fs/bcachefs/btree_iter.c | 57 | ||||
-rw-r--r-- | fs/bcachefs/btree_locking.h | 8 | ||||
-rw-r--r-- | fs/bcachefs/btree_update_interior.c | 2 | ||||
-rw-r--r-- | fs/bcachefs/btree_update_leaf.c | 4 |
4 files changed, 40 insertions, 31 deletions
diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index 74978a50023f..1d5d7b639241 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -2103,7 +2103,7 @@ bch2_btree_iter_traverse(struct btree_iter *iter) if (ret) return ret; - iter->path->should_be_locked = true; + btree_path_set_should_be_locked(iter->path); return 0; } @@ -2133,8 +2133,7 @@ struct btree *bch2_btree_iter_peek_node(struct btree_iter *iter) iter->path = bch2_btree_path_set_pos(trans, iter->path, b->key.k.p, iter->flags & BTREE_ITER_INTENT); - iter->path->should_be_locked = true; - BUG_ON(iter->path->uptodate); + btree_path_set_should_be_locked(iter->path); out: bch2_btree_iter_verify_entry_exit(iter); bch2_btree_iter_verify(iter); @@ -2206,7 +2205,7 @@ struct btree *bch2_btree_iter_next_node(struct btree_iter *iter) iter->path = bch2_btree_path_set_pos(trans, iter->path, b->key.k.p, iter->flags & BTREE_ITER_INTENT); - iter->path->should_be_locked = true; + btree_path_set_should_be_locked(iter->path); BUG_ON(iter->path->uptodate); out: bch2_btree_iter_verify_entry_exit(iter); @@ -2360,7 +2359,7 @@ struct bkey_s_c btree_trans_peek_key_cache(struct btree_iter *iter, struct bpos if (unlikely(ret)) return bkey_s_c_err(ret); - iter->key_cache_path->should_be_locked = true; + btree_path_set_should_be_locked(iter->key_cache_path); return bch2_btree_path_peek_slot(iter->key_cache_path, &u); } @@ -2387,7 +2386,7 @@ static struct bkey_s_c __bch2_btree_iter_peek(struct btree_iter *iter, struct bp goto out; } - iter->path->should_be_locked = true; + btree_path_set_should_be_locked(iter->path); k = btree_path_level_peek_all(trans->c, &iter->path->l[0], &iter->k); @@ -2474,7 +2473,7 @@ struct bkey_s_c bch2_btree_iter_peek_upto(struct btree_iter *iter, struct bpos e while (1) { k = __bch2_btree_iter_peek(iter, search_key); if (!k.k || bkey_err(k)) - goto out; + goto out_no_locked; /* * iter->pos should be mononotically increasing, and always be @@ -2491,7 +2490,7 @@ struct bkey_s_c bch2_btree_iter_peek_upto(struct btree_iter *iter, struct bpos e if (bkey_cmp(iter_pos, end) > 0) { bch2_btree_iter_set_pos(iter, end); k = bkey_s_c_null; - goto out; + goto out_no_locked; } if (iter->update_path && @@ -2551,18 +2550,16 @@ struct bkey_s_c bch2_btree_iter_peek_upto(struct btree_iter *iter, struct bpos e iter->path = bch2_btree_path_set_pos(trans, iter->path, k.k->p, iter->flags & BTREE_ITER_INTENT); - BUG_ON(!iter->path->nodes_locked); -out: + + btree_path_set_should_be_locked(iter->path); +out_no_locked: if (iter->update_path) { ret = bch2_btree_path_relock(trans, iter->update_path, _THIS_IP_); - if (unlikely(ret)) { + if (unlikely(ret)) k = bkey_s_c_err(ret); - } else { - BUG_ON(!(iter->update_path->nodes_locked & 1)); - iter->update_path->should_be_locked = true; - } + else + btree_path_set_should_be_locked(iter->update_path); } - iter->path->should_be_locked = true; if (!(iter->flags & BTREE_ITER_ALL_SNAPSHOTS)) iter->pos.snapshot = iter->snapshot; @@ -2605,13 +2602,13 @@ struct bkey_s_c bch2_btree_iter_peek_all_levels(struct btree_iter *iter) /* ensure that iter->k is consistent with iter->pos: */ bch2_btree_iter_set_pos(iter, iter->pos); k = bkey_s_c_err(ret); - goto out; + goto out_no_locked; } /* Already at end? */ if (!btree_path_node(iter->path, iter->path->level)) { k = bkey_s_c_null; - goto out; + goto out_no_locked; } k = btree_path_level_peek_all(trans->c, @@ -2664,8 +2661,8 @@ struct bkey_s_c bch2_btree_iter_peek_all_levels(struct btree_iter *iter) } iter->pos = k.k->p; -out: - iter->path->should_be_locked = true; + btree_path_set_should_be_locked(iter->path); +out_no_locked: bch2_btree_iter_verify(iter); return k; @@ -2718,7 +2715,7 @@ struct bkey_s_c bch2_btree_iter_peek_prev(struct btree_iter *iter) /* ensure that iter->k is consistent with iter->pos: */ bch2_btree_iter_set_pos(iter, iter->pos); k = bkey_s_c_err(ret); - goto out; + goto out_no_locked; } k = btree_path_level_peek(trans, iter->path, @@ -2782,7 +2779,7 @@ got_key: /* Start of btree: */ bch2_btree_iter_set_pos(iter, POS_MIN); k = bkey_s_c_null; - goto out; + goto out_no_locked; } } @@ -2794,10 +2791,11 @@ got_key: if (iter->flags & BTREE_ITER_FILTER_SNAPSHOTS) iter->pos.snapshot = iter->snapshot; -out: + + btree_path_set_should_be_locked(iter->path); +out_no_locked: if (saved_path) bch2_path_put(trans, saved_path, iter->flags & BTREE_ITER_INTENT); - iter->path->should_be_locked = true; bch2_btree_iter_verify_entry_exit(iter); bch2_btree_iter_verify(iter); @@ -2863,9 +2861,12 @@ struct bkey_s_c bch2_btree_iter_peek_slot(struct btree_iter *iter) if (unlikely(iter->flags & BTREE_ITER_WITH_KEY_CACHE) && (k = btree_trans_peek_key_cache(iter, iter->pos)).k) { - if (!bkey_err(k)) + if (bkey_err(k)) { + goto out_no_locked; + } else { iter->k = *k.k; - goto out; + goto out; + } } k = bch2_btree_path_peek_slot(iter->path, &iter->k); @@ -2919,8 +2920,8 @@ struct bkey_s_c bch2_btree_iter_peek_slot(struct btree_iter *iter) } } out: - iter->path->should_be_locked = true; - + btree_path_set_should_be_locked(iter->path); +out_no_locked: bch2_btree_iter_verify_entry_exit(iter); bch2_btree_iter_verify(iter); ret = bch2_btree_iter_verify_ret(iter, k); diff --git a/fs/bcachefs/btree_locking.h b/fs/bcachefs/btree_locking.h index 9d4e1a658eef..90bf5c02f504 100644 --- a/fs/bcachefs/btree_locking.h +++ b/fs/bcachefs/btree_locking.h @@ -283,4 +283,12 @@ static inline void bch2_btree_node_lock_write(struct btree_trans *trans, __bch2_btree_node_lock_write(trans, b); } +static inline void btree_path_set_should_be_locked(struct btree_path *path) +{ + EBUG_ON(!btree_node_locked(path, path->level)); + EBUG_ON(path->uptodate); + + path->should_be_locked = true; +} + #endif /* _BCACHEFS_BTREE_LOCKING_H */ diff --git a/fs/bcachefs/btree_update_interior.c b/fs/bcachefs/btree_update_interior.c index cf02e814c579..1fbf72df9e2f 100644 --- a/fs/bcachefs/btree_update_interior.c +++ b/fs/bcachefs/btree_update_interior.c @@ -1664,7 +1664,7 @@ int __bch2_foreground_maybe_merge(struct btree_trans *trans, if (ret) goto err; - sib_path->should_be_locked = true; + btree_path_set_should_be_locked(sib_path); m = sib_path->l[level].b; diff --git a/fs/bcachefs/btree_update_leaf.c b/fs/bcachefs/btree_update_leaf.c index 541826df50d9..9c84eed32007 100644 --- a/fs/bcachefs/btree_update_leaf.c +++ b/fs/bcachefs/btree_update_leaf.c @@ -1575,7 +1575,7 @@ bch2_trans_update_by_path_trace(struct btree_trans *trans, struct btree_path *pa if (ret) goto err; - btree_path->should_be_locked = true; + btree_path_set_should_be_locked(btree_path); ret = bch2_trans_update_by_path_trace(trans, btree_path, k, flags, ip); err: bch2_path_put(trans, btree_path, true); @@ -1643,7 +1643,7 @@ int __must_check bch2_trans_update(struct btree_trans *trans, struct btree_iter return btree_trans_restart(trans, BCH_ERR_transaction_restart_key_cache_raced); } - iter->key_cache_path->should_be_locked = true; + btree_path_set_should_be_locked(iter->key_cache_path); } path = iter->key_cache_path; |