btrfs: handle errors from btrfs_read_node_slot in split

While investigating a problem with error injection I tripped over
curious behavior in the node/leaf splitting code.  If we get an EIO when
trying to read either the left or right leaf/node for splitting we'll
simply treat the node as if it were full and continue on.  The end
result of this isn't too bad, we simply end up allocating a block when
we may have pushed items into the adjacent blocks.

However this does essentially allow us to continue to modify a file
system that we've gotten errors on, either from a bad disk or csum
mismatch or other corruption.  This isn't particularly safe, so instead
handle these btrfs_read_node_slot() usages differently.  We allow you to
pass in any slot, the idea being that we save some code if the slot
number is outside of the range of the parent.  This means we treat all
errors the same, when in reality we only want to ignore -ENOENT.

Fix this by changing how we call btrfs_read_node_slot(), which is to
only call it for slots we know are valid.  This way if we get an error
back from reading the block we can properly pass the error up the chain.
This was validated with the error injection testing I was doing.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index a1c109d7956aef7c8c700405bd8e5bd9d56ef30a..e1045e6d5e84b761171ce9f99cbdb11608ac2d63 100644 (file)
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1064,11 +1064,14 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
            BTRFS_NODEPTRS_PER_BLOCK(fs_info) / 4)
                return 0;
 
-       left = btrfs_read_node_slot(parent, pslot - 1);
-       if (IS_ERR(left))
-               left = NULL;
+       if (pslot) {
+               left = btrfs_read_node_slot(parent, pslot - 1);
+               if (IS_ERR(left)) {
+                       ret = PTR_ERR(left);
+                       left = NULL;
+                       goto enospc;
+               }
 
-       if (left) {
                __btrfs_tree_lock(left, BTRFS_NESTING_LEFT);
                wret = btrfs_cow_block(trans, root, left,
                                       parent, pslot - 1, &left,
@@ -1079,11 +1082,14 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
                }
        }
 
-       right = btrfs_read_node_slot(parent, pslot + 1);
-       if (IS_ERR(right))
-               right = NULL;
+       if (pslot + 1 < btrfs_header_nritems(parent)) {
+               right = btrfs_read_node_slot(parent, pslot + 1);
+               if (IS_ERR(right)) {
+                       ret = PTR_ERR(right);
+                       right = NULL;
+                       goto enospc;
+               }
 
-       if (right) {
                __btrfs_tree_lock(right, BTRFS_NESTING_RIGHT);
                wret = btrfs_cow_block(trans, root, right,
                                       parent, pslot + 1, &right,
@@ -1240,14 +1246,14 @@ static noinline int push_nodes_for_insert(struct btrfs_trans_handle *trans,
        if (!parent)
                return 1;
 
-       left = btrfs_read_node_slot(parent, pslot - 1);
-       if (IS_ERR(left))
-               left = NULL;
-
        /* first, try to make some room in the middle buffer */
-       if (left) {
+       if (pslot) {
                u32 left_nr;
 
+               left = btrfs_read_node_slot(parent, pslot - 1);
+               if (IS_ERR(left))
+                       return PTR_ERR(left);
+
                __btrfs_tree_lock(left, BTRFS_NESTING_LEFT);
 
                left_nr = btrfs_header_nritems(left);
@@ -1292,16 +1298,17 @@ static noinline int push_nodes_for_insert(struct btrfs_trans_handle *trans,
                btrfs_tree_unlock(left);
                free_extent_buffer(left);
        }
-       right = btrfs_read_node_slot(parent, pslot + 1);
-       if (IS_ERR(right))
-               right = NULL;
 
        /*
         * then try to empty the right most buffer into the middle
         */
-       if (right) {
+       if (pslot + 1 < btrfs_header_nritems(parent)) {
                u32 right_nr;
 
+               right = btrfs_read_node_slot(parent, pslot + 1);
+               if (IS_ERR(right))
+                       return PTR_ERR(right);
+
                __btrfs_tree_lock(right, BTRFS_NESTING_RIGHT);
 
                right_nr = btrfs_header_nritems(right);
@@ -3198,12 +3205,8 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
        btrfs_assert_tree_write_locked(path->nodes[1]);
 
        right = btrfs_read_node_slot(upper, slot + 1);
-       /*
-        * slot + 1 is not valid or we fail to read the right node,
-        * no big deal, just return.
-        */
        if (IS_ERR(right))
-               return 1;
+               return PTR_ERR(right);
 
        __btrfs_tree_lock(right, BTRFS_NESTING_RIGHT);
 
@@ -3417,12 +3420,8 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
        btrfs_assert_tree_write_locked(path->nodes[1]);
 
        left = btrfs_read_node_slot(path->nodes[1], slot - 1);
-       /*
-        * slot - 1 is not valid or we fail to read the left node,
-        * no big deal, just return.
-        */
        if (IS_ERR(left))
-               return 1;
+               return PTR_ERR(left);
 
        __btrfs_tree_lock(left, BTRFS_NESTING_LEFT);