Commit a300261a authored by Kent Overstreet's avatar Kent Overstreet Committed by Kent Overstreet

bcachefs: Fix duplicate paths left by bch2_path_put()

bch2_path_put() is supposed to drop paths that aren't needed on
transaction restart, or to hold locks that we're supposed to keep until
transaction commit: but it was failing to free paths in some cases that
it should have, leading to transaction path overflows with lots of
duplicate paths.
Signed-off-by: default avatarKent Overstreet <kent.overstreet@gmail.com>
parent 6fae65c1
...@@ -424,13 +424,17 @@ int bch2_btree_path_relock_intent(struct btree_trans *trans, ...@@ -424,13 +424,17 @@ int bch2_btree_path_relock_intent(struct btree_trans *trans,
return 0; return 0;
} }
noinline __flatten __flatten
static int __bch2_btree_path_relock(struct btree_trans *trans, static bool bch2_btree_path_relock_norestart(struct btree_trans *trans,
struct btree_path *path, unsigned long trace_ip) struct btree_path *path, unsigned long trace_ip)
{ {
bool ret = btree_path_get_locks(trans, path, false); return btree_path_get_locks(trans, path, false);
}
if (!ret) { static int __bch2_btree_path_relock(struct btree_trans *trans,
struct btree_path *path, unsigned long trace_ip)
{
if (!bch2_btree_path_relock_norestart(trans, path, trace_ip)) {
trace_trans_restart_relock_path(trans, trace_ip, path); trace_trans_restart_relock_path(trans, trace_ip, path);
return btree_trans_restart(trans, BCH_ERR_transaction_restart_relock_path); return btree_trans_restart(trans, BCH_ERR_transaction_restart_relock_path);
} }
...@@ -1743,30 +1747,30 @@ __bch2_btree_path_set_pos(struct btree_trans *trans, ...@@ -1743,30 +1747,30 @@ __bch2_btree_path_set_pos(struct btree_trans *trans,
static struct btree_path *have_path_at_pos(struct btree_trans *trans, struct btree_path *path) static struct btree_path *have_path_at_pos(struct btree_trans *trans, struct btree_path *path)
{ {
struct btree_path *next; struct btree_path *sib;
next = prev_btree_path(trans, path); sib = prev_btree_path(trans, path);
if (next && !btree_path_cmp(next, path)) if (sib && !btree_path_cmp(sib, path))
return next; return sib;
next = next_btree_path(trans, path); sib = next_btree_path(trans, path);
if (next && !btree_path_cmp(next, path)) if (sib && !btree_path_cmp(sib, path))
return next; return sib;
return NULL; return NULL;
} }
static struct btree_path *have_node_at_pos(struct btree_trans *trans, struct btree_path *path) static struct btree_path *have_node_at_pos(struct btree_trans *trans, struct btree_path *path)
{ {
struct btree_path *next; struct btree_path *sib;
next = prev_btree_path(trans, path); sib = prev_btree_path(trans, path);
if (next && next->level == path->level && path_l(next)->b == path_l(path)->b) if (sib && sib->level == path->level && path_l(sib)->b == path_l(path)->b)
return next; return sib;
next = next_btree_path(trans, path); sib = next_btree_path(trans, path);
if (next && next->level == path->level && path_l(next)->b == path_l(path)->b) if (sib && sib->level == path->level && path_l(sib)->b == path_l(path)->b)
return next; return sib;
return NULL; return NULL;
} }
...@@ -1788,26 +1792,23 @@ void bch2_path_put(struct btree_trans *trans, struct btree_path *path, bool inte ...@@ -1788,26 +1792,23 @@ void bch2_path_put(struct btree_trans *trans, struct btree_path *path, bool inte
if (!__btree_path_put(path, intent)) if (!__btree_path_put(path, intent))
return; return;
/* dup = path->preserve
* Perhaps instead we should check for duplicate paths in traverse_all: ? have_path_at_pos(trans, path)
*/ : have_node_at_pos(trans, path);
if (path->preserve &&
(dup = have_path_at_pos(trans, path))) { if (!dup && !(!path->preserve && !is_btree_node(path, path->level)))
dup->preserve = true; return;
path->preserve = false;
goto free;
}
if (!path->preserve &&
(dup = have_node_at_pos(trans, path)))
goto free;
return;
free:
if (path->should_be_locked && if (path->should_be_locked &&
!btree_node_locked(dup, path->level)) !trans->restarted &&
(!dup || !bch2_btree_path_relock_norestart(trans, dup, _THIS_IP_)))
return; return;
dup->should_be_locked |= path->should_be_locked; if (dup) {
dup->preserve |= path->preserve;
dup->should_be_locked |= path->should_be_locked;
}
__bch2_path_free(trans, path); __bch2_path_free(trans, path);
} }
......
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