lkml.org 
[lkml]   [2014]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.14 005/100] Btrfs: cleanup error handling in build_backref_tree
    Date
    3.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Josef Bacik <jbacik@fb.com>

    commit 75bfb9aff45e44625260f52a5fd581b92ace3e62 upstream.

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/btrfs/relocation.c | 88 +++++++++++++++++++++++++++++++++-----------------
    1 file changed, 59 insertions(+), 29 deletions(-)

    --- a/fs/btrfs/relocation.c
    +++ b/fs/btrfs/relocation.c
    @@ -736,7 +736,8 @@ again:
    err = ret;
    goto out;
    }
    - BUG_ON(!ret || !path1->slots[0]);
    + ASSERT(ret);
    + ASSERT(path1->slots[0]);

    path1->slots[0]--;

    @@ -746,10 +747,10 @@ again:
    * 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
    @@ -831,7 +832,7 @@ again:
    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) {
    @@ -840,7 +841,7 @@ again:
    * backref of this type.
    */
    root = find_reloc_root(rc, cur->bytenr);
    - BUG_ON(!root);
    + ASSERT(root);
    cur->root = root;
    break;
    }
    @@ -868,7 +869,7 @@ again:
    } 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);
    @@ -892,7 +893,7 @@ again:

    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);
    @@ -927,7 +928,7 @@ again:
    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);
    @@ -981,7 +982,7 @@ again:
    } 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);
    @@ -1025,7 +1026,7 @@ next:
    * 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,
    @@ -1061,8 +1062,21 @@ next:
    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);
    @@ -1085,7 +1099,7 @@ next:
    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) {
    @@ -1118,29 +1132,45 @@ out:
    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;
    }




    \
     
     \ /
      Last update: 2014-10-28 08:01    [W:4.191 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site