bcachefs: Improve iter->should_be_locked

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>
This commit is contained in:
Kent Overstreet 2021-06-14 18:16:10 -04:00 committed by Kent Overstreet
parent b89726ab86
commit 8c3f6da9fc
11 changed files with 50 additions and 32 deletions

View File

@ -176,6 +176,12 @@ static inline void bch2_btree_iter_set_pos(struct btree_iter *iter, struct bpos
iter->should_be_locked = false; 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) static inline struct btree_iter *btree_iter_child(struct btree_iter *iter)
{ {
return iter->child_idx == U8_MAX ? NULL return iter->child_idx == U8_MAX ? NULL

View File

@ -937,6 +937,8 @@ bch2_btree_update_start(struct btree_iter *iter, unsigned level,
int journal_flags = 0; int journal_flags = 0;
int ret = 0; int ret = 0;
BUG_ON(!iter->should_be_locked);
if (flags & BTREE_INSERT_JOURNAL_RESERVED) if (flags & BTREE_INSERT_JOURNAL_RESERVED)
journal_flags |= JOURNAL_RES_GET_RESERVED; journal_flags |= JOURNAL_RES_GET_RESERVED;

View 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, update_iter = bch2_trans_get_iter(trans, i->btree_id, update->k.p,
BTREE_ITER_NOT_EXTENTS| BTREE_ITER_NOT_EXTENTS|
BTREE_ITER_INTENT); 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_update(trans, update_iter, update, i->trigger_flags);
bch2_trans_iter_put(trans, update_iter); 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; int ret = 0;
BUG_ON(trans->nr_updates >= BTREE_ITER_MAX); BUG_ON(trans->nr_updates >= BTREE_ITER_MAX);
BUG_ON(!iter->should_be_locked);
#ifdef CONFIG_BCACHEFS_DEBUG #ifdef CONFIG_BCACHEFS_DEBUG
trans_for_each_update(trans, i) 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), iter = bch2_trans_get_iter(trans, id, bkey_start_pos(&k->k),
BTREE_ITER_INTENT); 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); bch2_trans_iter_put(trans, iter);
return ret; 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); iter = bch2_trans_get_iter(trans, id, start, BTREE_ITER_INTENT);
retry: 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)) && !(ret = bkey_err(k)) &&
bkey_cmp(iter->pos, end) < 0) { bkey_cmp(iter->pos, end) < 0) {
struct bkey_i delete; struct bkey_i delete;
bch2_trans_begin(trans);
bkey_init(&delete.k); bkey_init(&delete.k);
/* /*

View File

@ -1817,7 +1817,7 @@ static int __bch2_trans_mark_reflink_p(struct btree_trans *trans,
set_bkey_val_u64s(&n->k, 0); 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); ret = bch2_trans_update(trans, iter, n, 0);
if (ret) if (ret)
goto err; goto err;

View File

@ -104,6 +104,10 @@ int bch2_extent_atomic_end(struct btree_iter *iter,
unsigned nr_iters = 0; unsigned nr_iters = 0;
int ret; int ret;
ret = bch2_btree_iter_traverse(iter);
if (ret)
return ret;
*end = insert->k.p; *end = insert->k.p;
/* extent_update_to_keys(): */ /* extent_update_to_keys(): */

View File

@ -85,7 +85,8 @@ int bch2_create_trans(struct btree_trans *trans, u64 dir_inum,
inode_iter->snapshot = U32_MAX; inode_iter->snapshot = U32_MAX;
bch2_btree_iter_set_pos(inode_iter, SPOS(0, new_inode->bi_inum, 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: err:
bch2_trans_iter_put(trans, inode_iter); bch2_trans_iter_put(trans, inode_iter);
bch2_trans_iter_put(trans, dir_iter); bch2_trans_iter_put(trans, dir_iter);

View File

@ -2611,7 +2611,8 @@ reassemble:
BUG_ON(ret); 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_update(&trans, dst, copy.k, trigger_flags) ?:
bch2_trans_commit(&trans, &disk_res, bch2_trans_commit(&trans, &disk_res,
&inode->ei_journal_seq, &inode->ei_journal_seq,

View File

@ -78,7 +78,8 @@ static int __write_inode(struct btree_trans *trans,
bch2_trans_get_iter(trans, BTREE_ID_inodes, bch2_trans_get_iter(trans, BTREE_ID_inodes,
SPOS(0, inode->bi_inum, snapshot), SPOS(0, inode->bi_inum, snapshot),
BTREE_ITER_INTENT); 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); bch2_trans_iter_put(trans, inode_iter);
return ret; return ret;
} }
@ -305,7 +306,8 @@ static int hash_redo_key(struct btree_trans *trans,
bkey_init(&delete->k); bkey_init(&delete->k);
delete->k.p = k_iter->pos; 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); 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, ret = __bch2_trans_do(trans, NULL, NULL,
BTREE_INSERT_NOFAIL| BTREE_INSERT_NOFAIL|
BTREE_INSERT_LAZY_RW, BTREE_INSERT_LAZY_RW,
bch2_btree_iter_traverse(iter) ?:
bch2_inode_write(trans, iter, &u)); bch2_inode_write(trans, iter, &u));
if (ret) if (ret)
bch_err(c, "error in fsck: error %i " 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); BTREE_ITER_INTENT|BTREE_ITER_NOT_EXTENTS);
BUG_ON(iter->flags & BTREE_ITER_IS_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, bch2_trans_commit(trans, NULL, NULL,
BTREE_INSERT_NOFAIL| BTREE_INSERT_NOFAIL|
BTREE_INSERT_LAZY_RW); BTREE_INSERT_LAZY_RW);
@ -886,6 +890,7 @@ retry:
ret = __bch2_trans_do(&trans, NULL, NULL, ret = __bch2_trans_do(&trans, NULL, NULL,
BTREE_INSERT_NOFAIL| BTREE_INSERT_NOFAIL|
BTREE_INSERT_LAZY_RW, BTREE_INSERT_LAZY_RW,
bch2_btree_iter_traverse(iter) ?:
bch2_trans_update(&trans, iter, &n->k_i, 0)); bch2_trans_update(&trans, iter, &n->k_i, 0));
kfree(n); kfree(n);
if (ret) if (ret)
@ -1338,6 +1343,7 @@ static int check_nlinks_update_hardlinks(struct bch_fs *c,
ret = __bch2_trans_do(&trans, NULL, NULL, ret = __bch2_trans_do(&trans, NULL, NULL,
BTREE_INSERT_NOFAIL| BTREE_INSERT_NOFAIL|
BTREE_INSERT_LAZY_RW, BTREE_INSERT_LAZY_RW,
bch2_btree_iter_traverse(iter) ?:
bch2_inode_write(&trans, iter, &u)); bch2_inode_write(&trans, iter, &u));
if (ret) if (ret)
bch_err(c, "error in fsck: error %i updating inode", ret); bch_err(c, "error in fsck: error %i updating inode", ret);

View 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, iter = bch2_trans_get_node_iter(trans, id, k->k.p,
BTREE_MAX_DEPTH, level, BTREE_MAX_DEPTH, level,
BTREE_ITER_INTENT); BTREE_ITER_INTENT|
BTREE_ITER_NOT_EXTENTS);
/*
* 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);
ret = bch2_btree_iter_traverse(iter) ?: ret = bch2_btree_iter_traverse(iter) ?:
bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN); bch2_trans_update(trans, iter, k, BTREE_TRIGGER_NORUN);
bch2_trans_iter_put(trans, iter); 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|
BTREE_ITER_CACHED_NOFILL| BTREE_ITER_CACHED_NOFILL|
BTREE_ITER_INTENT); 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); bch2_trans_iter_put(trans, iter);
return ret; return ret;
} }

View File

@ -142,7 +142,7 @@ static int bch2_make_extent_indirect(struct btree_trans *trans,
goto err; goto err;
/* rewind iter to start of hole, if necessary: */ /* 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)); r_v = bch2_trans_kmalloc(trans, sizeof(__le64) + bkey_bytes(&orig->k));
ret = PTR_ERR_OR_ZERO(r_v); 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) { 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); bch2_bkey_buf_reassemble(&new_src, c, src_k);
src_k = bkey_i_to_s_c(new_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, ret = bch2_make_extent_indirect(&trans, src_iter,
new_src.k); new_src.k);
if (ret) if (ret)

View 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, iter = bch2_trans_get_iter(&trans, BTREE_ID_xattrs, k.k.p,
BTREE_ITER_INTENT); 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, ret = __bch2_trans_do(&trans, NULL, NULL, 0,
bch2_btree_iter_traverse(iter) ?:
bch2_trans_update(&trans, iter, &k.k_i, 0)); bch2_trans_update(&trans, iter, &k.k_i, 0));
if (ret) { if (ret) {
bch_err(c, "update error in test_delete: %i", 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"); pr_info("deleting once");
ret = __bch2_trans_do(&trans, NULL, NULL, 0, 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) { if (ret) {
bch_err(c, "delete error (first) in test_delete: %i", ret); bch_err(c, "delete error (first) in test_delete: %i", ret);
goto err; goto err;
@ -63,7 +59,8 @@ static int test_delete(struct bch_fs *c, u64 nr)
pr_info("deleting twice"); pr_info("deleting twice");
ret = __bch2_trans_do(&trans, NULL, NULL, 0, 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) { if (ret) {
bch_err(c, "delete error (second) in test_delete: %i", ret); bch_err(c, "delete error (second) in test_delete: %i", ret);
goto err; goto err;
@ -591,6 +588,7 @@ static int rand_mixed(struct bch_fs *c, u64 nr)
k.k.p = iter->pos; k.k.p = iter->pos;
ret = __bch2_trans_do(&trans, NULL, NULL, 0, ret = __bch2_trans_do(&trans, NULL, NULL, 0,
bch2_btree_iter_traverse(iter) ?:
bch2_trans_update(&trans, iter, &k.k_i, 0)); bch2_trans_update(&trans, iter, &k.k_i, 0));
if (ret) { if (ret) {
bch_err(c, "update error in rand_mixed: %i", 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; insert.k.p = iter->pos;
ret = __bch2_trans_do(&trans, NULL, NULL, 0, ret = __bch2_trans_do(&trans, NULL, NULL, 0,
bch2_btree_iter_traverse(iter) ?:
bch2_trans_update(&trans, iter, &insert.k_i, 0)); bch2_trans_update(&trans, iter, &insert.k_i, 0));
if (ret) { if (ret) {
bch_err(c, "error in seq_insert: %i", 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); bkey_reassemble(&u.k_i, k);
ret = __bch2_trans_do(&trans, NULL, NULL, 0, ret = __bch2_trans_do(&trans, NULL, NULL, 0,
bch2_btree_iter_traverse(iter) ?:
bch2_trans_update(&trans, iter, &u.k_i, 0)); bch2_trans_update(&trans, iter, &u.k_i, 0));
if (ret) { if (ret) {
bch_err(c, "error in seq_overwrite: %i", ret); bch_err(c, "error in seq_overwrite: %i", ret);