btrfs: always abort transaction on failure to add block group to free space tree

[ Upstream commit 1f06c942aa709d397cf6bed577a0d10a61509667 ]

Only one of the callers of __add_block_group_free_space() aborts the
transaction if the call fails, while the others don't do it and it's
either never done up the call chain or much higher in the call chain.

So make sure we abort the transaction at __add_block_group_free_space()
if it fails, which brings a couple benefits:

1) If some call chain never aborts the transaction, we avoid having some
   metadata inconsistency because BLOCK_GROUP_FLAG_NEEDS_FREE_SPACE is
   cleared when we enter __add_block_group_free_space() and therefore
   __add_block_group_free_space() is never called again to add the block
   group items to the free space tree, since the function is only called
   when that flag is set in a block group;

2) If the call chain already aborts the transaction, then we get a better
   trace that points to the exact step from __add_block_group_free_space()
   which failed, which is better for analysis.

So abort the transaction at __add_block_group_free_space() if any of its
steps fails.

CC: stable@vger.kernel.org # 6.6+
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 6f5ccb7b7db9eae1cfe71ea1bcfee05e318de7e8..51f286d5d00ab3e2643fd947b989d21522cec312 100644 (file)
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1379,12 +1379,17 @@ static int __add_block_group_free_space(struct btrfs_trans_handle *trans,
        clear_bit(BLOCK_GROUP_FLAG_NEEDS_FREE_SPACE, &block_group->runtime_flags);
 
        ret = add_new_free_space_info(trans, block_group, path);
-       if (ret)
+       if (ret) {
+               btrfs_abort_transaction(trans, ret);
                return ret;
+       }
 
-       return __add_to_free_space_tree(trans, block_group, path,
-                                       block_group->start,
-                                       block_group->length);
+       ret = __add_to_free_space_tree(trans, block_group, path,
+                                      block_group->start, block_group->length);
+       if (ret)
+               btrfs_abort_transaction(trans, ret);
+
+       return 0;
 }
 
 int add_block_group_free_space(struct btrfs_trans_handle *trans,
@@ -1409,9 +1414,6 @@ int add_block_group_free_space(struct btrfs_trans_handle *trans,
        }
 
        ret = __add_block_group_free_space(trans, block_group, path);
-       if (ret)
-               btrfs_abort_transaction(trans, ret);
-
 out:
        btrfs_free_path(path);
        mutex_unlock(&block_group->free_space_lock);