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>
This commit is contained in:
Kent Overstreet 2019-10-11 14:45:22 -04:00 committed by Kent Overstreet
parent 9a3df993e1
commit ea3532cbf7
3 changed files with 18 additions and 4 deletions

View file

@ -1042,11 +1042,12 @@ static void bch2_coalesce_nodes(struct bch_fs *c, struct btree_iter *iter,
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);
}

View file

@ -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)

View file

@ -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 @@ void __bch2_foreground_maybe_merge(struct bch_fs *c,
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;