diff options
| author | Kent Overstreet <kent.overstreet@gmail.com> | 2019-10-11 14:45:22 -0400 |
|---|---|---|
| committer | Kent Overstreet <kent.overstreet@gmail.com> | 2019-10-11 14:45:35 -0400 |
| commit | bc5a7252e34008d1fee125e58eaf984e89dc3105 (patch) | |
| tree | babe12191a12dd6a904e90e3be357b903dd66264 | |
| parent | 7e03c1ab0ef2e3148ba70656eab67471c85a0419 (diff) | |
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@gmail.com>
| -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 8b114d4f5e6a..c4c2e1a3ee0e 100644 --- a/fs/bcachefs/btree_gc.c +++ b/fs/bcachefs/btree_gc.c @@ -1040,11 +1040,12 @@ next: old_nodes[i] = new_nodes[i]; } else { old_nodes[i] = NULL; - if (new_nodes[i]) - six_unlock_intent(&new_nodes[i]->lock); } } + for (i = 0; i < nr_new_nodes; i++) + six_unlock_intent(&new_nodes[i]->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 e7f7820f9392..d1ea9b132c48 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->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 6813eddd26f5..ec0de33f0a1a 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->lock); + if (n2) + six_unlock_intent(&n2->lock); + six_unlock_intent(&n1->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->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->lock); bch2_btree_update_done(as); return 0; |