diff options
| author | Kent Overstreet <kent.overstreet@gmail.com> | 2019-10-11 20:45:22 +0200 |
|---|---|---|
| committer | Kent Overstreet <kent.overstreet@linux.dev> | 2023-10-22 23:08:29 +0200 |
| commit | ea3532cbf7fdbb9fa4e45114532d55d1fc3ac7c2 (patch) | |
| tree | 304ed5223daf5a0f80a271f53f103eabfeb53406 | |
| parent | bcachefs: Kill bchfs_extent_update() (diff) | |
| download | linux-ea3532cbf7fdbb9fa4e45114532d55d1fc3ac7c2.tar.xz linux-ea3532cbf7fdbb9fa4e45114532d55d1fc3ac7c2.zip | |
bcachefs: Fix a subtle race in the btree split path
We have to free the old (in memory) btree node _before_ unlocking the
new nodes - else, some other thread with a read lock on the old node
could see stale data after another thread has already updated the new
node.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
| -rw-r--r-- | fs/bcachefs/btree_gc.c | 5 | ||||
| -rw-r--r-- | fs/bcachefs/btree_iter.c | 2 | ||||
| -rw-r--r-- | fs/bcachefs/btree_update_interior.c | 15 |
3 files changed, 18 insertions, 4 deletions
diff --git a/fs/bcachefs/btree_gc.c b/fs/bcachefs/btree_gc.c index f7d9abfdb3de..4a66c44764f6 100644 --- a/fs/bcachefs/btree_gc.c +++ b/fs/bcachefs/btree_gc.c @@ -1042,11 +1042,12 @@ next: old_nodes[i] = new_nodes[i]; } else { old_nodes[i] = NULL; - if (new_nodes[i]) - six_unlock_intent(&new_nodes[i]->c.lock); } } + for (i = 0; i < nr_new_nodes; i++) + six_unlock_intent(&new_nodes[i]->c.lock); + bch2_btree_update_done(as); bch2_keylist_free(&keylist, NULL); } diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index 8c6d3193c3fe..a91cee797703 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -833,8 +833,6 @@ void bch2_btree_iter_node_replace(struct btree_iter *iter, struct btree *b) btree_iter_node_set(linked, b); } - - six_unlock_intent(&b->c.lock); } void bch2_btree_iter_node_drop(struct btree_iter *iter, struct btree *b) diff --git a/fs/bcachefs/btree_update_interior.c b/fs/bcachefs/btree_update_interior.c index 946254c51a69..3b134d3a9984 100644 --- a/fs/bcachefs/btree_update_interior.c +++ b/fs/bcachefs/btree_update_interior.c @@ -1446,8 +1446,20 @@ static void btree_split(struct btree_update *as, struct btree *b, bch2_btree_iter_node_replace(iter, n2); bch2_btree_iter_node_replace(iter, n1); + /* + * The old node must be freed (in memory) _before_ unlocking the new + * nodes - else another thread could re-acquire a read lock on the old + * node after another thread has locked and updated the new node, thus + * seeing stale data: + */ bch2_btree_node_free_inmem(c, b, iter); + if (n3) + six_unlock_intent(&n3->c.lock); + if (n2) + six_unlock_intent(&n2->c.lock); + six_unlock_intent(&n1->c.lock); + bch2_btree_trans_verify_locks(iter->trans); bch2_time_stats_update(&c->times[BCH_TIME_btree_node_split], @@ -1761,6 +1773,8 @@ retry: bch2_btree_node_free_inmem(c, b, iter); bch2_btree_node_free_inmem(c, m, iter); + six_unlock_intent(&n->c.lock); + bch2_btree_update_done(as); if (!(flags & BTREE_INSERT_GC_LOCK_HELD)) @@ -1855,6 +1869,7 @@ static int __btree_node_rewrite(struct bch_fs *c, struct btree_iter *iter, bch2_btree_iter_node_drop(iter, b); bch2_btree_iter_node_replace(iter, n); bch2_btree_node_free_inmem(c, b, iter); + six_unlock_intent(&n->c.lock); bch2_btree_update_done(as); return 0; |