btrfs: abort transaction on error in btrfs_remove_block_group()

When btrfs_remove_block_group() fails we abort the transaction in its
single caller (btrfs_remove_chunk()). This makes it harder to find out
where exactly the failure happened, as several steps inside
btrfs_remove_block_group() can fail.

So make btrfs_remove_block_group() abort the transaction whenever an
error happens, instead of aborting in its caller.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 3a0521236ecd8011f513049e930ed6e967e631e6..7b723571501ed68486ac474ed48a504e0bd685c0 100644 (file)
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1108,8 +1108,10 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
        bool remove_rsv = false;
 
        block_group = btrfs_lookup_block_group(fs_info, map->start);
-       if (!block_group)
+       if (unlikely(!block_group)) {
+               btrfs_abort_transaction(trans, -ENOENT);
                return -ENOENT;
+       }
 
        BUG_ON(!block_group->ro && !(block_group->flags & BTRFS_BLOCK_GROUP_REMAPPED));
 
@@ -1143,8 +1145,9 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
        btrfs_clear_data_reloc_bg(block_group);
 
        path = btrfs_alloc_path();
-       if (!path) {
+       if (unlikely(!path)) {
                ret = -ENOMEM;
+               btrfs_abort_transaction(trans, ret);
                goto out;
        }
 
@@ -1180,8 +1183,10 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
        mutex_unlock(&trans->transaction->cache_write_mutex);
 
        ret = btrfs_remove_free_space_inode(trans, inode, block_group);
-       if (ret)
+       if (unlikely(ret)) {
+               btrfs_abort_transaction(trans, ret);
                goto out;
+       }
 
        write_lock(&fs_info->block_group_cache_lock);
        rb_erase_cached(&block_group->cache_node,
@@ -1268,13 +1273,17 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
         */
        if (!(block_group->flags & BTRFS_BLOCK_GROUP_REMAPPED)) {
                ret = btrfs_remove_block_group_free_space(trans, block_group);
-               if (ret)
+               if (unlikely(ret)) {
+                       btrfs_abort_transaction(trans, ret);
                        goto out;
+               }
        }
 
        ret = remove_block_group_item(trans, path, block_group);
-       if (ret < 0)
+       if (unlikely(ret < 0)) {
+               btrfs_abort_transaction(trans, ret);
                goto out;
+       }
 
        spin_lock(&block_group->lock);
        /*

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index cff2412bc879daa6d013e71b5f379e1535ef390c..d33780082b8da22344d0ab27fc39cd90a418893b 100644 (file)
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3384,11 +3384,10 @@ int btrfs_remove_chunk(struct btrfs_trans_handle *trans, u64 chunk_offset)
         */
        btrfs_trans_release_chunk_metadata(trans);
 
+       /* On error, btrfs_remove_block_group() aborts the transaction. */
        ret = btrfs_remove_block_group(trans, map);
-       if (unlikely(ret)) {
-               btrfs_abort_transaction(trans, ret);
-               goto out;
-       }
+       if (unlikely(ret))
+               ASSERT(BTRFS_FS_ERROR(fs_info) != 0);
 
 out:
        if (trans->removing_chunk) {