btrfs: fix return value check of btrfs_join_transaction()

The error check of btrfs_join_transaction()/btrfs_join_transaction_nolock()
is added, and the mistake of the error check in several places is
corrected.

For more stable Btrfs, I think that we should reduce BUG_ON().
But, I think that long time is necessary for this.
So, I propose this patch as a short-term solution.

With this patch:
 - To more stable Btrfs, the part that should be corrected is clarified.
 - The panic isn't done by the NULL pointer reference etc. (even if
   BUG_ON() is increased temporarily)
 - The error code is returned in the place where the error can be easily
   returned.

As a long-term plan:
 - BUG_ON() is reduced by using the forced-readonly framework, etc.

Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Signed-off-by: Chris Mason <chris.mason@oracle.com>

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 2887b8be6fdd26c30794c72c8718db868dd9781f..b36eeef19194ecf384330a2d1e7eda5e552b2a09 100644 (file)
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1550,6 +1550,7 @@ static int transaction_kthread(void *arg)
                spin_unlock(&root->fs_info->new_trans_lock);
 
                trans = btrfs_join_transaction(root, 1);
+               BUG_ON(IS_ERR(trans));
                if (transid == trans->transid) {
                        ret = btrfs_commit_transaction(trans, root);
                        BUG_ON(ret);
@@ -2464,10 +2465,14 @@ int btrfs_commit_super(struct btrfs_root *root)
        up_write(&root->fs_info->cleanup_work_sem);
 
        trans = btrfs_join_transaction(root, 1);
+       if (IS_ERR(trans))
+               return PTR_ERR(trans);
        ret = btrfs_commit_transaction(trans, root);
        BUG_ON(ret);
        /* run commit again to drop the original snapshot */
        trans = btrfs_join_transaction(root, 1);
+       if (IS_ERR(trans))
+               return PTR_ERR(trans);
        btrfs_commit_transaction(trans, root);
        ret = btrfs_write_and_wait_transaction(NULL, root);
        BUG_ON(ret);

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index bcf303204f7f5f3473eaa7a907423ef998274f4d..98ee139885ccb1ba33b4598dcf0e77aec58f46f1 100644 (file)
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -7478,7 +7478,7 @@ int btrfs_drop_dead_reloc_roots(struct btrfs_root *root)
                BUG_ON(reloc_root->commit_root != NULL);
                while (1) {
                        trans = btrfs_join_transaction(root, 1);
-                       BUG_ON(!trans);
+                       BUG_ON(IS_ERR(trans));
 
                        mutex_lock(&root->fs_info->drop_mutex);
                        ret = btrfs_drop_snapshot(trans, reloc_root);

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2b7d251d6ad12a281fb4cb7faf04868439dc7225..40fee137dd11cf4d927ee2b4b036aa3b0899aeab 100644 (file)
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -416,7 +416,7 @@ again:
        }
        if (start == 0) {
                trans = btrfs_join_transaction(root, 1);
-               BUG_ON(!trans);
+               BUG_ON(IS_ERR(trans));
                btrfs_set_trans_block_group(trans, inode);
                trans->block_rsv = &root->fs_info->delalloc_block_rsv;
 
@@ -612,6 +612,7 @@ retry:
                            GFP_NOFS);
 
                trans = btrfs_join_transaction(root, 1);
+               BUG_ON(IS_ERR(trans));
                ret = btrfs_reserve_extent(trans, root,
                                           async_extent->compressed_size,
                                           async_extent->compressed_size,
@@ -771,7 +772,7 @@ static noinline int cow_file_range(struct inode *inode,
 
        BUG_ON(root == root->fs_info->tree_root);
        trans = btrfs_join_transaction(root, 1);
-       BUG_ON(!trans);
+       BUG_ON(IS_ERR(trans));
        btrfs_set_trans_block_group(trans, inode);
        trans->block_rsv = &root->fs_info->delalloc_block_rsv;
 
@@ -1049,7 +1050,7 @@ static noinline int run_delalloc_nocow(struct inode *inode,
        } else {
                trans = btrfs_join_transaction(root, 1);
        }
-       BUG_ON(!trans);
+       BUG_ON(IS_ERR(trans));
 
        cow_start = (u64)-1;
        cur_offset = start;
@@ -1704,7 +1705,7 @@ static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end)
                                trans = btrfs_join_transaction_nolock(root, 1);
                        else
                                trans = btrfs_join_transaction(root, 1);
-                       BUG_ON(!trans);
+                       BUG_ON(IS_ERR(trans));
                        btrfs_set_trans_block_group(trans, inode);
                        trans->block_rsv = &root->fs_info->delalloc_block_rsv;
                        ret = btrfs_update_inode(trans, root, inode);
@@ -1721,6 +1722,7 @@ static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end)
                trans = btrfs_join_transaction_nolock(root, 1);
        else
                trans = btrfs_join_transaction(root, 1);
+       BUG_ON(IS_ERR(trans));
        btrfs_set_trans_block_group(trans, inode);
        trans->block_rsv = &root->fs_info->delalloc_block_rsv;
 
@@ -2382,6 +2384,7 @@ void btrfs_orphan_cleanup(struct btrfs_root *root)
 
        if (root->orphan_block_rsv || root->orphan_item_inserted) {
                trans = btrfs_join_transaction(root, 1);
+               BUG_ON(IS_ERR(trans));
                btrfs_end_transaction(trans, root);
        }
 
@@ -4350,6 +4353,8 @@ int btrfs_write_inode(struct inode *inode, struct writeback_control *wbc)
                        trans = btrfs_join_transaction_nolock(root, 1);
                else
                        trans = btrfs_join_transaction(root, 1);
+               if (IS_ERR(trans))
+                       return PTR_ERR(trans);
                btrfs_set_trans_block_group(trans, inode);
                if (nolock)
                        ret = btrfs_end_transaction_nolock(trans, root);
@@ -4375,6 +4380,7 @@ void btrfs_dirty_inode(struct inode *inode)
                return;
 
        trans = btrfs_join_transaction(root, 1);
+       BUG_ON(IS_ERR(trans));
        btrfs_set_trans_block_group(trans, inode);
 
        ret = btrfs_update_inode(trans, root, inode);
@@ -5179,6 +5185,8 @@ again:
                                em = NULL;
                                btrfs_release_path(root, path);
                                trans = btrfs_join_transaction(root, 1);
+                               if (IS_ERR(trans))
+                                       return ERR_CAST(trans);
                                goto again;
                        }
                        map = kmap(page);
@@ -5283,8 +5291,8 @@ static struct extent_map *btrfs_new_extent_direct(struct inode *inode,
        btrfs_drop_extent_cache(inode, start, start + len - 1, 0);
 
        trans = btrfs_join_transaction(root, 0);
-       if (!trans)
-               return ERR_PTR(-ENOMEM);
+       if (IS_ERR(trans))
+               return ERR_CAST(trans);
 
        trans->block_rsv = &root->fs_info->delalloc_block_rsv;
 
@@ -5508,7 +5516,7 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
                 * while we look for nocow cross refs
                 */
                trans = btrfs_join_transaction(root, 0);
-               if (!trans)
+               if (IS_ERR(trans))
                        goto must_cow;
 
                if (can_nocow_odirect(trans, inode, start, len) == 1) {
@@ -5643,7 +5651,7 @@ again:
        BUG_ON(!ordered);
 
        trans = btrfs_join_transaction(root, 1);
-       if (!trans) {
+       if (IS_ERR(trans)) {
                err = -ENOMEM;
                goto out;
        }

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index edd82becbb9e3476f9ee7c97817185e43180bd33..04b4fb9144a9162143b64219dee370a29946b980 100644 (file)
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -203,7 +203,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 
 
        trans = btrfs_join_transaction(root, 1);
-       BUG_ON(!trans);
+       BUG_ON(IS_ERR(trans));
 
        ret = btrfs_update_inode(trans, root, inode);
        BUG_ON(ret);

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 045c9c2b2d7ef7399f00370e4137d9da0b370836..ea9965430241036832e5def4255e5fc72df1016f 100644 (file)
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2147,6 +2147,12 @@ again:
        }
 
        trans = btrfs_join_transaction(rc->extent_root, 1);
+       if (IS_ERR(trans)) {
+               if (!err)
+                       btrfs_block_rsv_release(rc->extent_root,
+                                               rc->block_rsv, num_bytes);
+               return PTR_ERR(trans);
+       }
 
        if (!err) {
                if (num_bytes != rc->merging_rsv_size) {
@@ -3222,6 +3228,7 @@ truncate:
        trans = btrfs_join_transaction(root, 0);
        if (IS_ERR(trans)) {
                btrfs_free_path(path);
+               ret = PTR_ERR(trans);
                goto out;
        }
 
@@ -3628,6 +3635,7 @@ int prepare_to_relocate(struct reloc_control *rc)
        set_reloc_control(rc);
 
        trans = btrfs_join_transaction(rc->extent_root, 1);
+       BUG_ON(IS_ERR(trans));
        btrfs_commit_transaction(trans, rc->extent_root);
        return 0;
 }
@@ -3804,7 +3812,10 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
 
        /* get rid of pinned extents */
        trans = btrfs_join_transaction(rc->extent_root, 1);
-       btrfs_commit_transaction(trans, rc->extent_root);
+       if (IS_ERR(trans))
+               err = PTR_ERR(trans);
+       else
+               btrfs_commit_transaction(trans, rc->extent_root);
 out_free:
        btrfs_free_block_rsv(rc->extent_root, rc->block_rsv);
        btrfs_free_path(path);
@@ -4125,6 +4136,11 @@ int btrfs_recover_relocation(struct btrfs_root *root)
        set_reloc_control(rc);
 
        trans = btrfs_join_transaction(rc->extent_root, 1);
+       if (IS_ERR(trans)) {
+               unset_reloc_control(rc);
+               err = PTR_ERR(trans);
+               goto out_free;
+       }
 
        rc->merge_reloc_tree = 1;
 
@@ -4154,9 +4170,13 @@ int btrfs_recover_relocation(struct btrfs_root *root)
        unset_reloc_control(rc);
 
        trans = btrfs_join_transaction(rc->extent_root, 1);
-       btrfs_commit_transaction(trans, rc->extent_root);
-out:
+       if (IS_ERR(trans))
+               err = PTR_ERR(trans);
+       else
+               btrfs_commit_transaction(trans, rc->extent_root);
+out_free:
        kfree(rc);
+out:
        while (!list_empty(&reloc_roots)) {
                reloc_root = list_entry(reloc_roots.next,
                                        struct btrfs_root, root_list);

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index bae5c7b8bbe2e11028afa69d9689886f28a3486f..3d73c8d93bbb1c3f20c1d10dfa076e8dd887bd16 100644 (file)
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1161,6 +1161,11 @@ int btrfs_commit_transaction_async(struct btrfs_trans_handle *trans,
        INIT_DELAYED_WORK(&ac->work, do_async_commit);
        ac->root = root;
        ac->newtrans = btrfs_join_transaction(root, 0);
+       if (IS_ERR(ac->newtrans)) {
+               int err = PTR_ERR(ac->newtrans);
+               kfree(ac);
+               return err;
+       }
 
        /* take transaction reference */
        mutex_lock(&root->fs_info->trans_mutex);