Skip to content

Commit 75bfb9a

Browse files
Josef Bacikmasoncl
authored andcommitted
Btrfs: cleanup error handling in build_backref_tree
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>
1 parent 1d52c78 commit 75bfb9a

File tree

1 file changed

+59
-29
lines changed

1 file changed

+59
-29
lines changed

fs/btrfs/relocation.c

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,8 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
736736
err = ret;
737737
goto out;
738738
}
739-
BUG_ON(!ret || !path1->slots[0]);
739+
ASSERT(ret);
740+
ASSERT(path1->slots[0]);
740741

741742
path1->slots[0]--;
742743

@@ -746,10 +747,10 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
746747
* the backref was added previously when processing
747748
* backref of type BTRFS_TREE_BLOCK_REF_KEY
748749
*/
749-
BUG_ON(!list_is_singular(&cur->upper));
750+
ASSERT(list_is_singular(&cur->upper));
750751
edge = list_entry(cur->upper.next, struct backref_edge,
751752
list[LOWER]);
752-
BUG_ON(!list_empty(&edge->list[UPPER]));
753+
ASSERT(list_empty(&edge->list[UPPER]));
753754
exist = edge->node[UPPER];
754755
/*
755756
* add the upper level block to pending list if we need
@@ -831,7 +832,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
831832
cur->cowonly = 1;
832833
}
833834
#else
834-
BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
835+
ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
835836
if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
836837
#endif
837838
if (key.objectid == key.offset) {
@@ -840,7 +841,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
840841
* backref of this type.
841842
*/
842843
root = find_reloc_root(rc, cur->bytenr);
843-
BUG_ON(!root);
844+
ASSERT(root);
844845
cur->root = root;
845846
break;
846847
}
@@ -868,7 +869,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
868869
} else {
869870
upper = rb_entry(rb_node, struct backref_node,
870871
rb_node);
871-
BUG_ON(!upper->checked);
872+
ASSERT(upper->checked);
872873
INIT_LIST_HEAD(&edge->list[UPPER]);
873874
}
874875
list_add_tail(&edge->list[LOWER], &cur->upper);
@@ -892,7 +893,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
892893

893894
if (btrfs_root_level(&root->root_item) == cur->level) {
894895
/* tree root */
895-
BUG_ON(btrfs_root_bytenr(&root->root_item) !=
896+
ASSERT(btrfs_root_bytenr(&root->root_item) ==
896897
cur->bytenr);
897898
if (should_ignore_root(root))
898899
list_add(&cur->list, &useless);
@@ -927,7 +928,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
927928
need_check = true;
928929
for (; level < BTRFS_MAX_LEVEL; level++) {
929930
if (!path2->nodes[level]) {
930-
BUG_ON(btrfs_root_bytenr(&root->root_item) !=
931+
ASSERT(btrfs_root_bytenr(&root->root_item) ==
931932
lower->bytenr);
932933
if (should_ignore_root(root))
933934
list_add(&lower->list, &useless);
@@ -982,7 +983,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
982983
} else {
983984
upper = rb_entry(rb_node, struct backref_node,
984985
rb_node);
985-
BUG_ON(!upper->checked);
986+
ASSERT(upper->checked);
986987
INIT_LIST_HEAD(&edge->list[UPPER]);
987988
if (!upper->owner)
988989
upper->owner = btrfs_header_owner(eb);
@@ -1026,7 +1027,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
10261027
* everything goes well, connect backref nodes and insert backref nodes
10271028
* into the cache.
10281029
*/
1029-
BUG_ON(!node->checked);
1030+
ASSERT(node->checked);
10301031
cowonly = node->cowonly;
10311032
if (!cowonly) {
10321033
rb_node = tree_insert(&cache->rb_root, node->bytenr,
@@ -1062,8 +1063,21 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
10621063
continue;
10631064
}
10641065

1065-
BUG_ON(!upper->checked);
1066-
BUG_ON(cowonly != upper->cowonly);
1066+
if (!upper->checked) {
1067+
/*
1068+
* Still want to blow up for developers since this is a
1069+
* logic bug.
1070+
*/
1071+
ASSERT(0);
1072+
err = -EINVAL;
1073+
goto out;
1074+
}
1075+
if (cowonly != upper->cowonly) {
1076+
ASSERT(0);
1077+
err = -EINVAL;
1078+
goto out;
1079+
}
1080+
10671081
if (!cowonly) {
10681082
rb_node = tree_insert(&cache->rb_root, upper->bytenr,
10691083
&upper->rb_node);
@@ -1086,7 +1100,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
10861100
while (!list_empty(&useless)) {
10871101
upper = list_entry(useless.next, struct backref_node, list);
10881102
list_del_init(&upper->list);
1089-
BUG_ON(!list_empty(&upper->upper));
1103+
ASSERT(list_empty(&upper->upper));
10901104
if (upper == node)
10911105
node = NULL;
10921106
if (upper->lowest) {
@@ -1119,29 +1133,45 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
11191133
if (err) {
11201134
while (!list_empty(&useless)) {
11211135
lower = list_entry(useless.next,
1122-
struct backref_node, upper);
1123-
list_del_init(&lower->upper);
1136+
struct backref_node, list);
1137+
list_del_init(&lower->list);
11241138
}
1125-
upper = node;
1126-
INIT_LIST_HEAD(&list);
1127-
while (upper) {
1128-
if (RB_EMPTY_NODE(&upper->rb_node)) {
1129-
list_splice_tail(&upper->upper, &list);
1130-
free_backref_node(cache, upper);
1131-
}
1132-
1133-
if (list_empty(&list))
1134-
break;
1135-
1136-
edge = list_entry(list.next, struct backref_edge,
1137-
list[LOWER]);
1139+
while (!list_empty(&list)) {
1140+
edge = list_first_entry(&list, struct backref_edge,
1141+
list[UPPER]);
1142+
list_del(&edge->list[UPPER]);
11381143
list_del(&edge->list[LOWER]);
1144+
lower = edge->node[LOWER];
11391145
upper = edge->node[UPPER];
11401146
free_backref_edge(cache, edge);
1147+
1148+
/*
1149+
* Lower is no longer linked to any upper backref nodes
1150+
* and isn't in the cache, we can free it ourselves.
1151+
*/
1152+
if (list_empty(&lower->upper) &&
1153+
RB_EMPTY_NODE(&lower->rb_node))
1154+
list_add(&lower->list, &useless);
1155+
1156+
if (!RB_EMPTY_NODE(&upper->rb_node))
1157+
continue;
1158+
1159+
/* Add this guy's upper edges to the list to proces */
1160+
list_for_each_entry(edge, &upper->upper, list[LOWER])
1161+
list_add_tail(&edge->list[UPPER], &list);
1162+
if (list_empty(&upper->upper))
1163+
list_add(&upper->list, &useless);
1164+
}
1165+
1166+
while (!list_empty(&useless)) {
1167+
lower = list_entry(useless.next,
1168+
struct backref_node, list);
1169+
list_del_init(&lower->list);
1170+
free_backref_node(cache, lower);
11411171
}
11421172
return ERR_PTR(err);
11431173
}
1144-
BUG_ON(node && node->detached);
1174+
ASSERT(!node || !node->detached);
11451175
return node;
11461176
}
11471177

0 commit comments

Comments
 (0)