Skip to content

Commit

Permalink
Btrfs: cleanup error handling in build_backref_tree
Browse files Browse the repository at this point in the history 
When balance panics it tends to panic in the

BUG_ON(!upper->checked);

test, because it means it couldn't build the backref tree properly.  This is
annoying to users and frankly a recoverable error, nothing in this function is
actually fatal since it is just an in-memory building of the backrefs for a
given bytenr.  So go through and change all the BUG_ON()'s to ASSERT()'s, and
fix the BUG_ON(!upper->checked) thing to just return an error.

This patch also fixes the error handling so it tears down the work we've done
properly.  This code was horribly broken since we always just panic'ed instead
of actually erroring out, so it needed to be completely re-worked.  With this
patch my broken image no longer panics when I mount it.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
Signed-off-by: Chris Mason <clm@fb.com>
  • Loading branch information
@masoncl
Josef Bacik authored and masoncl committed Oct 3, 2014
1 parent 1d52c78 commit 75bfb9a
Showing 1 changed file with 59 additions and 29 deletions.
88 changes: 59 additions & 29 deletions fs/btrfs/relocation.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,8 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
err = ret;
goto out;
}
BUG_ON(!ret || !path1->slots[0]);
ASSERT(ret);
ASSERT(path1->slots[0]);

path1->slots[0]--;

Expand All @@ -746,10 +747,10 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
* the backref was added previously when processing
* backref of type BTRFS_TREE_BLOCK_REF_KEY
*/
BUG_ON(!list_is_singular(&cur->upper));
ASSERT(list_is_singular(&cur->upper));
edge = list_entry(cur->upper.next, struct backref_edge,
list[LOWER]);
BUG_ON(!list_empty(&edge->list[UPPER]));
ASSERT(list_empty(&edge->list[UPPER]));
exist = edge->node[UPPER];
/*
* add the upper level block to pending list if we need
Expand Down Expand Up @@ -831,7 +832,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
cur->cowonly = 1;
}
#else
BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
#endif
if (key.objectid == key.offset) {
Expand All @@ -840,7 +841,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
* backref of this type.
*/
root = find_reloc_root(rc, cur->bytenr);
BUG_ON(!root);
ASSERT(root);
cur->root = root;
break;
}
Expand Down Expand Up @@ -868,7 +869,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
} else {
upper = rb_entry(rb_node, struct backref_node,
rb_node);
BUG_ON(!upper->checked);
ASSERT(upper->checked);
INIT_LIST_HEAD(&edge->list[UPPER]);
}
list_add_tail(&edge->list[LOWER], &cur->upper);
Expand All @@ -892,7 +893,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,

if (btrfs_root_level(&root->root_item) == cur->level) {
/* tree root */
BUG_ON(btrfs_root_bytenr(&root->root_item) !=
ASSERT(btrfs_root_bytenr(&root->root_item) ==
cur->bytenr);
if (should_ignore_root(root))
list_add(&cur->list, &useless);
Expand Down Expand Up @@ -927,7 +928,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
need_check = true;
for (; level < BTRFS_MAX_LEVEL; level++) {
if (!path2->nodes[level]) {
BUG_ON(btrfs_root_bytenr(&root->root_item) !=
ASSERT(btrfs_root_bytenr(&root->root_item) ==
lower->bytenr);
if (should_ignore_root(root))
list_add(&lower->list, &useless);
Expand Down Expand Up @@ -982,7 +983,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
} else {
upper = rb_entry(rb_node, struct backref_node,
rb_node);
BUG_ON(!upper->checked);
ASSERT(upper->checked);
INIT_LIST_HEAD(&edge->list[UPPER]);
if (!upper->owner)
upper->owner = btrfs_header_owner(eb);
Expand Down Expand Up @@ -1026,7 +1027,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
* everything goes well, connect backref nodes and insert backref nodes
* into the cache.
*/
BUG_ON(!node->checked);
ASSERT(node->checked);
cowonly = node->cowonly;
if (!cowonly) {
rb_node = tree_insert(&cache->rb_root, node->bytenr,
Expand Down Expand Up @@ -1062,8 +1063,21 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
continue;
}

BUG_ON(!upper->checked);
BUG_ON(cowonly != upper->cowonly);
if (!upper->checked) {
/*
* Still want to blow up for developers since this is a
* logic bug.
*/
ASSERT(0);
err = -EINVAL;
goto out;
}
if (cowonly != upper->cowonly) {
ASSERT(0);
err = -EINVAL;
goto out;
}

if (!cowonly) {
rb_node = tree_insert(&cache->rb_root, upper->bytenr,
&upper->rb_node);
Expand All @@ -1086,7 +1100,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
while (!list_empty(&useless)) {
upper = list_entry(useless.next, struct backref_node, list);
list_del_init(&upper->list);
BUG_ON(!list_empty(&upper->upper));
ASSERT(list_empty(&upper->upper));
if (upper == node)
node = NULL;
if (upper->lowest) {
Expand Down Expand Up @@ -1119,29 +1133,45 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
if (err) {
while (!list_empty(&useless)) {
lower = list_entry(useless.next,
struct backref_node, upper);
list_del_init(&lower->upper);
struct backref_node, list);
list_del_init(&lower->list);
}
upper = node;
INIT_LIST_HEAD(&list);
while (upper) {
if (RB_EMPTY_NODE(&upper->rb_node)) {
list_splice_tail(&upper->upper, &list);
free_backref_node(cache, upper);
}

if (list_empty(&list))
break;

edge = list_entry(list.next, struct backref_edge,
list[LOWER]);
while (!list_empty(&list)) {
edge = list_first_entry(&list, struct backref_edge,
list[UPPER]);
list_del(&edge->list[UPPER]);
list_del(&edge->list[LOWER]);
lower = edge->node[LOWER];
upper = edge->node[UPPER];
free_backref_edge(cache, edge);

/*
* Lower is no longer linked to any upper backref nodes
* and isn't in the cache, we can free it ourselves.
*/
if (list_empty(&lower->upper) &&
RB_EMPTY_NODE(&lower->rb_node))
list_add(&lower->list, &useless);

if (!RB_EMPTY_NODE(&upper->rb_node))
continue;

/* Add this guy's upper edges to the list to proces */
list_for_each_entry(edge, &upper->upper, list[LOWER])
list_add_tail(&edge->list[UPPER], &list);
if (list_empty(&upper->upper))
list_add(&upper->list, &useless);
}

while (!list_empty(&useless)) {
lower = list_entry(useless.next,
struct backref_node, list);
list_del_init(&lower->list);
free_backref_node(cache, lower);
}
return ERR_PTR(err);
}
BUG_ON(node && node->detached);
ASSERT(!node || !node->detached);
return node;
}

Expand Down

0 comments on commit 75bfb9a

Please sign in to comment.