Commit 56a40fbc authored by Kent Overstreet's avatar Kent Overstreet Committed by Kent Overstreet

bcachefs: Various fixes for interior update path

The locking was wrong, and we could get a use after free in the error
path where we weren't taking the entrie being freed off the unwritten
list.
Signed-off-by: default avatarKent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: default avatarKent Overstreet <kent.overstreet@linux.dev>
parent 4e4758c6
...@@ -581,7 +581,7 @@ static struct btree_reserve *bch2_btree_reserve_get(struct bch_fs *c, ...@@ -581,7 +581,7 @@ static struct btree_reserve *bch2_btree_reserve_get(struct bch_fs *c,
/* Asynchronous interior node update machinery */ /* Asynchronous interior node update machinery */
static void bch2_btree_update_free(struct btree_update *as) static void __bch2_btree_update_free(struct btree_update *as)
{ {
struct bch_fs *c = as->c; struct bch_fs *c = as->c;
...@@ -596,28 +596,32 @@ static void bch2_btree_update_free(struct btree_update *as) ...@@ -596,28 +596,32 @@ static void bch2_btree_update_free(struct btree_update *as)
if (as->reserve) if (as->reserve)
bch2_btree_reserve_put(c, as->reserve); bch2_btree_reserve_put(c, as->reserve);
mutex_lock(&c->btree_interior_update_lock);
list_del(&as->list); list_del(&as->list);
closure_debug_destroy(&as->cl); closure_debug_destroy(&as->cl);
mempool_free(as, &c->btree_interior_update_pool); mempool_free(as, &c->btree_interior_update_pool);
closure_wake_up(&c->btree_interior_update_wait); closure_wake_up(&c->btree_interior_update_wait);
mutex_unlock(&c->btree_interior_update_lock);
} }
static void btree_update_nodes_reachable(struct btree_update *as, u64 seq) static void bch2_btree_update_free(struct btree_update *as)
{ {
struct bch_fs *c = as->c; struct bch_fs *c = as->c;
mutex_lock(&c->btree_interior_update_lock); mutex_lock(&c->btree_interior_update_lock);
__bch2_btree_update_free(as);
mutex_unlock(&c->btree_interior_update_lock);
}
static void btree_update_nodes_reachable(struct btree_update *as, u64 seq)
{
struct bch_fs *c = as->c;
while (as->nr_new_nodes) { while (as->nr_new_nodes) {
struct btree *b = as->new_nodes[--as->nr_new_nodes]; struct btree *b = as->new_nodes[--as->nr_new_nodes];
BUG_ON(b->will_make_reachable != (unsigned long) as); BUG_ON(b->will_make_reachable != (unsigned long) as);
b->will_make_reachable = 0; b->will_make_reachable = 0;
mutex_unlock(&c->btree_interior_update_lock);
/* /*
* b->will_make_reachable prevented it from being written, so * b->will_make_reachable prevented it from being written, so
...@@ -626,14 +630,11 @@ static void btree_update_nodes_reachable(struct btree_update *as, u64 seq) ...@@ -626,14 +630,11 @@ static void btree_update_nodes_reachable(struct btree_update *as, u64 seq)
btree_node_lock_type(c, b, SIX_LOCK_read); btree_node_lock_type(c, b, SIX_LOCK_read);
bch2_btree_node_write_cond(c, b, btree_node_need_write(b)); bch2_btree_node_write_cond(c, b, btree_node_need_write(b));
six_unlock_read(&b->c.lock); six_unlock_read(&b->c.lock);
mutex_lock(&c->btree_interior_update_lock);
} }
while (as->nr_pending) while (as->nr_pending)
bch2_btree_node_free_ondisk(c, &as->pending[--as->nr_pending], bch2_btree_node_free_ondisk(c, &as->pending[--as->nr_pending],
seq); seq);
mutex_unlock(&c->btree_interior_update_lock);
} }
static void btree_update_nodes_written(struct closure *cl) static void btree_update_nodes_written(struct closure *cl)
...@@ -667,9 +668,12 @@ static void btree_update_nodes_written(struct closure *cl) ...@@ -667,9 +668,12 @@ static void btree_update_nodes_written(struct closure *cl)
mutex_unlock(&c->btree_interior_update_lock); mutex_unlock(&c->btree_interior_update_lock);
btree_node_lock_type(c, b, SIX_LOCK_intent); btree_node_lock_type(c, b, SIX_LOCK_intent);
six_unlock_intent(&b->c.lock); six_unlock_intent(&b->c.lock);
goto out; mutex_lock(&c->btree_interior_update_lock);
goto again;
} }
list_del(&as->unwritten_list);
journal_u64s = 0; journal_u64s = 0;
if (as->mode != BTREE_INTERIOR_UPDATING_ROOT) if (as->mode != BTREE_INTERIOR_UPDATING_ROOT)
...@@ -710,9 +714,6 @@ static void btree_update_nodes_written(struct closure *cl) ...@@ -710,9 +714,6 @@ static void btree_update_nodes_written(struct closure *cl)
bch2_btree_add_journal_pin(c, b, res.seq); bch2_btree_add_journal_pin(c, b, res.seq);
six_unlock_write(&b->c.lock); six_unlock_write(&b->c.lock);
list_del(&as->unwritten_list);
mutex_unlock(&c->btree_interior_update_lock);
/* /*
* b->write_blocked prevented it from being written, so * b->write_blocked prevented it from being written, so
* write it now if it needs to be written: * write it now if it needs to be written:
...@@ -723,9 +724,6 @@ static void btree_update_nodes_written(struct closure *cl) ...@@ -723,9 +724,6 @@ static void btree_update_nodes_written(struct closure *cl)
case BTREE_INTERIOR_UPDATING_AS: case BTREE_INTERIOR_UPDATING_AS:
BUG_ON(b); BUG_ON(b);
list_del(&as->unwritten_list);
mutex_unlock(&c->btree_interior_update_lock);
break; break;
case BTREE_INTERIOR_UPDATING_ROOT: { case BTREE_INTERIOR_UPDATING_ROOT: {
...@@ -739,9 +737,6 @@ static void btree_update_nodes_written(struct closure *cl) ...@@ -739,9 +737,6 @@ static void btree_update_nodes_written(struct closure *cl)
r->alive = true; r->alive = true;
c->btree_roots_dirty = true; c->btree_roots_dirty = true;
mutex_unlock(&c->btree_root_lock); mutex_unlock(&c->btree_root_lock);
list_del(&as->unwritten_list);
mutex_unlock(&c->btree_interior_update_lock);
break; break;
} }
} }
...@@ -753,14 +748,12 @@ static void btree_update_nodes_written(struct closure *cl) ...@@ -753,14 +748,12 @@ static void btree_update_nodes_written(struct closure *cl)
btree_update_nodes_reachable(as, res.seq); btree_update_nodes_reachable(as, res.seq);
free_update: free_update:
bch2_btree_update_free(as); __bch2_btree_update_free(as);
/* /*
* for flush_held_btree_writes() waiting on updates to flush or * for flush_held_btree_writes() waiting on updates to flush or
* nodes to be writeable: * nodes to be writeable:
*/ */
closure_wake_up(&c->btree_interior_update_wait); closure_wake_up(&c->btree_interior_update_wait);
out:
mutex_lock(&c->btree_interior_update_lock);
goto again; goto again;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment