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>

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index 74978a50023f35b8f1fdd3187300ca0416e5d1ff..1d5d7b639241a3cf6e9be4352f652908367a7c5a 100644 (file)
--- 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 9d4e1a658eef06b15736466eb3e1baff1d3de1f3..90bf5c02f5045aafcbf65870a51a752e8947c1b1 100644 (file)
--- 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 cf02e814c5797756bd5e5bbdbfe54bee7b25f96b..1fbf72df9e2f9668e9edf329033a2d9429227727 100644 (file)
--- 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 541826df50d9d3011ed231639d71b68117417f11..9c84eed32007eec25b90c24b3aeb50ccd375f97e 100644 (file)
--- 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;