btrfs: simplify loop in select_reloc_root()

We have this setup as a loop, but in reality we will never walk back up
the backref tree, if we do then it's a bug.  Get rid of the loop and
handle the case where we have node->new_bytenr set at all.  Previous
check was only if node->new_bytenr != root->node->start, but if it did
then we would hit the WARN_ON() and walk back up the tree.

Instead we want to just return error if ->new_bytenr is set, and then do
the normal updating of the node for the reloc root and carry on.

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 5af1907e230b3c7d922c2cb894f9baddc1dcc172..4cad2540e3ae38b8245c4a3aa867afc1e44cdfda 100644 (file)
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2058,97 +2058,72 @@ struct btrfs_root *select_reloc_root(struct btrfs_trans_handle *trans,
        int index = 0;
        int ret;
 
-       next = node;
-       while (1) {
-               cond_resched();
-               next = walk_up_backref(next, edges, &index);
-               root = next->root;
-
-               /*
-                * If there is no root, then our references for this block are
-                * incomplete, as we should be able to walk all the way up to a
-                * block that is owned by a root.
-                *
-                * This path is only for SHAREABLE roots, so if we come upon a
-                * non-SHAREABLE root then we have backrefs that resolve
-                * improperly.
-                *
-                * Both of these cases indicate file system corruption, or a bug
-                * in the backref walking code.
-                */
-               if (!root) {
-                       ASSERT(0);
-                       btrfs_err(trans->fs_info,
-               "bytenr %llu doesn't have a backref path ending in a root",
-                                 node->bytenr);
-                       return ERR_PTR(-EUCLEAN);
-               }
-               if (!test_bit(BTRFS_ROOT_SHAREABLE, &root->state)) {
-                       ASSERT(0);
-                       btrfs_err(trans->fs_info,
-       "bytenr %llu has multiple refs with one ending in a non-shareable root",
-                                 node->bytenr);
-                       return ERR_PTR(-EUCLEAN);
-               }
+       next = walk_up_backref(node, edges, &index);
+       root = next->root;
 
-               if (btrfs_root_id(root) == BTRFS_TREE_RELOC_OBJECTID) {
-                       ret = record_reloc_root_in_trans(trans, root);
-                       if (ret)
-                               return ERR_PTR(ret);
-                       break;
-               }
+       /*
+        * If there is no root, then our references for this block are
+        * incomplete, as we should be able to walk all the way up to a block
+        * that is owned by a root.
+        *
+        * This path is only for SHAREABLE roots, so if we come upon a
+        * non-SHAREABLE root then we have backrefs that resolve improperly.
+        *
+        * Both of these cases indicate file system corruption, or a bug in the
+        * backref walking code.
+        */
+       if (unlikely(!root)) {
+               btrfs_err(trans->fs_info,
+                         "bytenr %llu doesn't have a backref path ending in a root",
+                         node->bytenr);
+               return ERR_PTR(-EUCLEAN);
+       }
+       if (unlikely(!test_bit(BTRFS_ROOT_SHAREABLE, &root->state))) {
+               btrfs_err(trans->fs_info,
+                         "bytenr %llu has multiple refs with one ending in a non-shareable root",
+                         node->bytenr);
+               return ERR_PTR(-EUCLEAN);
+       }
 
-               ret = btrfs_record_root_in_trans(trans, root);
+       if (btrfs_root_id(root) == BTRFS_TREE_RELOC_OBJECTID) {
+               ret = record_reloc_root_in_trans(trans, root);
                if (ret)
                        return ERR_PTR(ret);
-               root = root->reloc_root;
-
-               /*
-                * We could have raced with another thread which failed, so
-                * root->reloc_root may not be set, return ENOENT in this case.
-                */
-               if (!root)
-                       return ERR_PTR(-ENOENT);
+               goto found;
+       }
 
-               if (next->new_bytenr != root->node->start) {
-                       /*
-                        * We just created the reloc root, so we shouldn't have
-                        * ->new_bytenr set yet. If it is then we have multiple
-                        *  roots pointing at the same bytenr which indicates
-                        *  corruption, or we've made a mistake in the backref
-                        *  walking code.
-                        */
-                       ASSERT(next->new_bytenr == 0);
-                       if (next->new_bytenr) {
-                               btrfs_err(trans->fs_info,
-       "bytenr %llu possibly has multiple roots pointing at the same bytenr %llu",
-                                         node->bytenr, next->bytenr);
-                               return ERR_PTR(-EUCLEAN);
-                       }
+       ret = btrfs_record_root_in_trans(trans, root);
+       if (ret)
+               return ERR_PTR(ret);
+       root = root->reloc_root;
 
-                       next->new_bytenr = root->node->start;
-                       btrfs_put_root(next->root);
-                       next->root = btrfs_grab_root(root);
-                       ASSERT(next->root);
-                       mark_block_processed(rc, next);
-                       break;
-               }
+       /*
+        * We could have raced with another thread which failed, so
+        * root->reloc_root may not be set, return ENOENT in this case.
+        */
+       if (!root)
+               return ERR_PTR(-ENOENT);
 
-               WARN_ON(1);
-               root = NULL;
-               next = walk_down_backref(edges, &index);
-               if (!next || next->level <= node->level)
-                       break;
-       }
-       if (!root) {
+       if (next->new_bytenr) {
                /*
-                * This can happen if there's fs corruption or if there's a bug
-                * in the backref lookup code.
+                * We just created the reloc root, so we shouldn't have
+                * ->new_bytenr set yet. If it is then we have multiple roots
+                *  pointing at the same bytenr which indicates corruption, or
+                *  we've made a mistake in the backref walking code.
                 */
-               ASSERT(0);
-               return ERR_PTR(-ENOENT);
+               ASSERT(next->new_bytenr == 0);
+               btrfs_err(trans->fs_info,
+                         "bytenr %llu possibly has multiple roots pointing at the same bytenr %llu",
+                         node->bytenr, next->bytenr);
+               return ERR_PTR(-EUCLEAN);
        }
 
+       next->new_bytenr = root->node->start;
+       btrfs_put_root(next->root);
+       next->root = btrfs_grab_root(root);
+       ASSERT(next->root);
+       mark_block_processed(rc, next);
+found:
        next = node;
        /* setup backref node path for btrfs_reloc_cow_block */
        while (1) {