bcachefs: Improve iter->should_be_locked
authorKent Overstreet <kent.overstreet@gmail.com>
Mon, 14 Jun 2021 22:16:10 +0000 (18:16 -0400)
committerKent Overstreet <kent.overstreet@linux.dev>
Sun, 22 Oct 2023 21:09:06 +0000 (17:09 -0400)
Adding iter->should_be_locked introduced a regression where it ended up
not being set on the iterator passed to bch2_btree_update_start(), which
is definitely not what we want.

This patch requires it to be set when calling bch2_trans_update(), and
adds various fixups to make that happen.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
fs/bcachefs/btree_iter.h
fs/bcachefs/btree_update_interior.c
fs/bcachefs/btree_update_leaf.c
fs/bcachefs/buckets.c
fs/bcachefs/extent_update.c
fs/bcachefs/fs-common.c
fs/bcachefs/fs-io.c
fs/bcachefs/fsck.c
fs/bcachefs/recovery.c
fs/bcachefs/reflink.c
fs/bcachefs/tests.c

index 27c685a482ecbbd06616e6a9469136b88fc2f1bc..6efea281d87f6aa6ed7f83ed1e1ba455d82b3027 100644 (file)
@@ -176,6 +176,12 @@ static inline void bch2_btree_iter_set_pos(struct btree_iter *iter, struct bpos
        iter->should_be_locked = false;
 }
 
+static inline void bch2_btree_iter_set_pos_to_extent_start(struct btree_iter *iter)
+{
+       BUG_ON(!(iter->flags & BTREE_ITER_IS_EXTENTS));
+       iter->pos = bkey_start_pos(&iter->k);
+}
+
 static inline struct btree_iter *btree_iter_child(struct btree_iter *iter)
 {
        return iter->child_idx == U8_MAX ? NULL
index bb01b036c7a2e19892150fa9ee40d9b519b1530e..e35e842efe81c102ef2b30d28e4a19d40d24c3a1 100644 (file)
@@ -937,6 +937,8 @@ bch2_btree_update_start(struct btree_iter *iter, unsigned level,
        int journal_flags = 0;
        int ret = 0;
 
+       BUG_ON(!iter->should_be_locked);
+
        if (flags & BTREE_INSERT_JOURNAL_RESERVED)
                journal_flags |= JOURNAL_RES_GET_RESERVED;
 
index 634e25e94425345d06f4b916b5e16c2dda83fc5c..00706b95263023023acfb5ea0db0dee01aed2595 100644 (file)
@@ -855,6 +855,10 @@ static int extent_handle_overwrites(struct btree_trans *trans,
                        update_iter = bch2_trans_get_iter(trans, i->btree_id, update->k.p,
                                                          BTREE_ITER_NOT_EXTENTS|
                                                          BTREE_ITER_INTENT);
+                       ret = bch2_btree_iter_traverse(update_iter);
+                       if (ret)
+                               goto out;
+
                        bch2_trans_update(trans, update_iter, update, i->trigger_flags);
                        bch2_trans_iter_put(trans, update_iter);
                }
@@ -1039,6 +1043,7 @@ int bch2_trans_update(struct btree_trans *trans, struct btree_iter *iter,
        int ret = 0;
 
        BUG_ON(trans->nr_updates >= BTREE_ITER_MAX);
+       BUG_ON(!iter->should_be_locked);
 
 #ifdef CONFIG_BCACHEFS_DEBUG
        trans_for_each_update(trans, i)
@@ -1102,7 +1107,8 @@ int __bch2_btree_insert(struct btree_trans *trans,
        iter = bch2_trans_get_iter(trans, id, bkey_start_pos(&k->k),
                                   BTREE_ITER_INTENT);
 
-       ret = bch2_trans_update(trans, iter, k, 0);
+       ret   = bch2_btree_iter_traverse(iter) ?:
+               bch2_trans_update(trans, iter, k, 0);
        bch2_trans_iter_put(trans, iter);
        return ret;
 }
@@ -1147,13 +1153,12 @@ int bch2_btree_delete_range_trans(struct btree_trans *trans, enum btree_id id,
 
        iter = bch2_trans_get_iter(trans, id, start, BTREE_ITER_INTENT);
 retry:
-       while ((k = bch2_btree_iter_peek(iter)).k &&
+       while ((bch2_trans_begin(trans),
+              (k = bch2_btree_iter_peek(iter)).k) &&
               !(ret = bkey_err(k)) &&
               bkey_cmp(iter->pos, end) < 0) {
                struct bkey_i delete;
 
-               bch2_trans_begin(trans);
-
                bkey_init(&delete.k);
 
                /*
index c427744a665f2aa76c48c18c2e9194daf6be05f8..3dd206d3b546e34aa11a022cff09f00707ff6b5f 100644 (file)
@@ -1817,7 +1817,7 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
                set_bkey_val_u64s(&n->k, 0);
        }
 
-       bch2_btree_iter_set_pos(iter, bkey_start_pos(k.k));
+       bch2_btree_iter_set_pos_to_extent_start(iter);
        ret = bch2_trans_update(trans, iter, n, 0);
        if (ret)
                goto err;
index ef4aaf1c30ed50cc909147543c082271bc54f130..4a8dd085f7fb80b2650f5bab433cae077343d1e3 100644 (file)
@@ -104,6 +104,10 @@ int bch2_extent_atomic_end(struct btree_iter *iter,
        unsigned nr_iters = 0;
        int ret;
 
+       ret = bch2_btree_iter_traverse(iter);
+       if (ret)
+               return ret;
+
        *end = insert->k.p;
 
        /* extent_update_to_keys(): */
index 00a63fecb976e6da858c1adc64de69d40fcf3710..60c54438074e711ba2f9fcc1a34279dbe79a0ad8 100644 (file)
@@ -85,7 +85,8 @@ int bch2_create_trans(struct btree_trans *trans, u64 dir_inum,
        inode_iter->snapshot = U32_MAX;
        bch2_btree_iter_set_pos(inode_iter, SPOS(0, new_inode->bi_inum, U32_MAX));
 
-       ret = bch2_inode_write(trans, inode_iter, new_inode);
+       ret   = bch2_btree_iter_traverse(inode_iter) ?:
+               bch2_inode_write(trans, inode_iter, new_inode);
 err:
        bch2_trans_iter_put(trans, inode_iter);
        bch2_trans_iter_put(trans, dir_iter);
index e39e22581e4b443376aa92c6c3443a56a7089a59..0ffc3971d1b208c45a4878e2ece92b56897e2c6f 100644 (file)
@@ -2611,7 +2611,8 @@ reassemble:
                        BUG_ON(ret);
                }
 
-               ret =   bch2_trans_update(&trans, del, &delete, trigger_flags) ?:
+               ret =   bch2_btree_iter_traverse(del) ?:
+                       bch2_trans_update(&trans, del, &delete, trigger_flags) ?:
                        bch2_trans_update(&trans, dst, copy.k, trigger_flags) ?:
                        bch2_trans_commit(&trans, &disk_res,
                                          &inode->ei_journal_seq,
index 1bb595f4003aaec2b85b526b3eee2110e62f4de5..7ea1a41ac637fa02a1160b10ee929280cd36d73f 100644 (file)
@@ -78,7 +78,8 @@ static int __write_inode(struct btree_trans *trans,
                bch2_trans_get_iter(trans, BTREE_ID_inodes,
                                    SPOS(0, inode->bi_inum, snapshot),
                                    BTREE_ITER_INTENT);
-       int ret = bch2_inode_write(trans, inode_iter, inode);
+       int ret = bch2_btree_iter_traverse(inode_iter) ?:
+               bch2_inode_write(trans, inode_iter, inode);
        bch2_trans_iter_put(trans, inode_iter);
        return ret;
 }
@@ -305,7 +306,8 @@ static int hash_redo_key(struct btree_trans *trans,
 
        bkey_init(&delete->k);
        delete->k.p = k_iter->pos;
-       return  bch2_trans_update(trans, k_iter, delete, 0) ?:
+       return  bch2_btree_iter_traverse(k_iter) ?:
+               bch2_trans_update(trans, k_iter, delete, 0) ?:
                bch2_hash_set(trans, desc, hash_info, k_iter->pos.inode, tmp, 0);
 }
 
@@ -491,6 +493,7 @@ static int check_inode(struct btree_trans *trans,
                ret = __bch2_trans_do(trans, NULL, NULL,
                                      BTREE_INSERT_NOFAIL|
                                      BTREE_INSERT_LAZY_RW,
+                               bch2_btree_iter_traverse(iter) ?:
                                bch2_inode_write(trans, iter, &u));
                if (ret)
                        bch_err(c, "error in fsck: error %i "
@@ -562,7 +565,8 @@ static int fix_overlapping_extent(struct btree_trans *trans,
                                   BTREE_ITER_INTENT|BTREE_ITER_NOT_EXTENTS);
 
        BUG_ON(iter->flags & BTREE_ITER_IS_EXTENTS);
-       ret   = bch2_trans_update(trans, iter, u, BTREE_TRIGGER_NORUN) ?:
+       ret   = bch2_btree_iter_traverse(iter) ?:
+               bch2_trans_update(trans, iter, u, BTREE_TRIGGER_NORUN) ?:
                bch2_trans_commit(trans, NULL, NULL,
                                  BTREE_INSERT_NOFAIL|
                                  BTREE_INSERT_LAZY_RW);
@@ -886,6 +890,7 @@ retry:
                        ret = __bch2_trans_do(&trans, NULL, NULL,
                                              BTREE_INSERT_NOFAIL|
                                              BTREE_INSERT_LAZY_RW,
+                               bch2_btree_iter_traverse(iter) ?:
                                bch2_trans_update(&trans, iter, &n->k_i, 0));
                        kfree(n);
                        if (ret)
@@ -1338,6 +1343,7 @@ static int check_nlinks_update_hardlinks(struct bch_fs *c,
                        ret = __bch2_trans_do(&trans, NULL, NULL,
                                              BTREE_INSERT_NOFAIL|
                                              BTREE_INSERT_LAZY_RW,
+                                             bch2_btree_iter_traverse(iter) ?:
                                        bch2_inode_write(&trans, iter, &u));
                        if (ret)
                                bch_err(c, "error in fsck: error %i updating inode", ret);
index f32414171aab82e062f6f3c43cd3f9a09e18d1e0..c6fa4ca31ae9163581713af436f68a20da74259f 100644 (file)
@@ -509,16 +509,8 @@ static int __bch2_journal_replay_key(struct btree_trans *trans,
 
        iter = bch2_trans_get_node_iter(trans, id, k->k.p,
                                        BTREE_MAX_DEPTH, level,
-                                       BTREE_ITER_INTENT);
-
-       /*
-        * iter->flags & BTREE_ITER_IS_EXTENTS triggers the update path to run
-        * extent_handle_overwrites() and extent_update_to_keys() - but we don't
-        * want that here, journal replay is supposed to treat extents like
-        * regular keys:
-        */
-       BUG_ON(iter->flags & BTREE_ITER_IS_EXTENTS);
-
+                                       BTREE_ITER_INTENT|
+                                       BTREE_ITER_NOT_EXTENTS);
        ret   = bch2_btree_iter_traverse(iter) ?:
                bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
        bch2_trans_iter_put(trans, iter);
@@ -546,7 +538,8 @@ static int __bch2_alloc_replay_key(struct btree_trans *trans, struct bkey_i *k)
                                   BTREE_ITER_CACHED|
                                   BTREE_ITER_CACHED_NOFILL|
                                   BTREE_ITER_INTENT);
-       ret = bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
+       ret   = bch2_btree_iter_traverse(iter) ?:
+               bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
        bch2_trans_iter_put(trans, iter);
        return ret;
 }
index ba700810a4bee67223264c01a877011ef1a16676..ebf391245470f2b2e49a528ade8c1715791f861d 100644 (file)
@@ -142,7 +142,7 @@ static int bch2_make_extent_indirect(struct btree_trans *trans,
                goto err;
 
        /* rewind iter to start of hole, if necessary: */
-       bch2_btree_iter_set_pos(reflink_iter, bkey_start_pos(k.k));
+       bch2_btree_iter_set_pos_to_extent_start(reflink_iter);
 
        r_v = bch2_trans_kmalloc(trans, sizeof(__le64) + bkey_bytes(&orig->k));
        ret = PTR_ERR_OR_ZERO(r_v);
@@ -257,11 +257,11 @@ s64 bch2_remap_range(struct bch_fs *c,
                }
 
                if (src_k.k->type != KEY_TYPE_reflink_p) {
+                       bch2_btree_iter_set_pos_to_extent_start(src_iter);
+
                        bch2_bkey_buf_reassemble(&new_src, c, src_k);
                        src_k = bkey_i_to_s_c(new_src.k);
 
-                       bch2_btree_iter_set_pos(src_iter, bkey_start_pos(src_k.k));
-
                        ret = bch2_make_extent_indirect(&trans, src_iter,
                                                new_src.k);
                        if (ret)
index fa9f600fc17c29087d96f509388ef97499a08c03..a8b8e3a072ad05abb273542c95b2e7efd97752dc 100644 (file)
@@ -40,13 +40,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
        iter = bch2_trans_get_iter(&trans, BTREE_ID_xattrs, k.k.p,
                                   BTREE_ITER_INTENT);
 
-       ret = bch2_btree_iter_traverse(iter);
-       if (ret) {
-               bch_err(c, "lookup error in test_delete: %i", ret);
-               goto err;
-       }
-
        ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+               bch2_btree_iter_traverse(iter) ?:
                bch2_trans_update(&trans, iter, &k.k_i, 0));
        if (ret) {
                bch_err(c, "update error in test_delete: %i", ret);
@@ -55,7 +50,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
 
        pr_info("deleting once");
        ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-                        bch2_btree_delete_at(&trans, iter, 0));
+               bch2_btree_iter_traverse(iter) ?:
+               bch2_btree_delete_at(&trans, iter, 0));
        if (ret) {
                bch_err(c, "delete error (first) in test_delete: %i", ret);
                goto err;
@@ -63,7 +59,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
 
        pr_info("deleting twice");
        ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-                        bch2_btree_delete_at(&trans, iter, 0));
+               bch2_btree_iter_traverse(iter) ?:
+               bch2_btree_delete_at(&trans, iter, 0));
        if (ret) {
                bch_err(c, "delete error (second) in test_delete: %i", ret);
                goto err;
@@ -591,6 +588,7 @@ static int rand_mixed(struct bch_fs *c, u64 nr)
                        k.k.p = iter->pos;
 
                        ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+                               bch2_btree_iter_traverse(iter) ?:
                                bch2_trans_update(&trans, iter, &k.k_i, 0));
                        if (ret) {
                                bch_err(c, "update error in rand_mixed: %i", ret);
@@ -671,6 +669,7 @@ static int seq_insert(struct bch_fs *c, u64 nr)
                insert.k.p = iter->pos;
 
                ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+                       bch2_btree_iter_traverse(iter) ?:
                        bch2_trans_update(&trans, iter, &insert.k_i, 0));
                if (ret) {
                        bch_err(c, "error in seq_insert: %i", ret);
@@ -719,6 +718,7 @@ static int seq_overwrite(struct bch_fs *c, u64 nr)
                bkey_reassemble(&u.k_i, k);
 
                ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+                       bch2_btree_iter_traverse(iter) ?:
                        bch2_trans_update(&trans, iter, &u.k_i, 0));
                if (ret) {
                        bch_err(c, "error in seq_overwrite: %i", ret);