btrfs: do not BUG_ON after failure to migrate space during truncation

During truncation we reserve 2 metadata units when starting a transaction
(reserved space goes to fs_info->trans_block_rsv) and then attempt to
migrate 1 unit (min_size bytes) from fs_info->trans_block_rsv into the
local block reserve. If we ever fail we trigger a BUG_ON(), which should
never happen, because we reserved 2 units. However if we happen to fail
for some reason, we don't need to be so dire and hit a BUG_ON(), we can
just error out the truncation and, since this is highly unexpected,
surround the error check with WARN_ON(), to get an informative stack
trace and tag the branh as 'unlikely'. Also make the 'min_size' variable
const, since it's not supposed to ever change and any accidental change
could possibly make the space migration not so unlikely to fail.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b92c814cec97c5e67eded232cd9bda62eca14b69..dbbb67293e345c15dcf1787839d596ddeb4a40a1 100644 (file)
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8344,7 +8344,7 @@ static int btrfs_truncate(struct btrfs_inode *inode, bool skip_writeback)
        int ret;
        struct btrfs_trans_handle *trans;
        u64 mask = fs_info->sectorsize - 1;
-       u64 min_size = btrfs_calc_metadata_size(fs_info, 1);
+       const u64 min_size = btrfs_calc_metadata_size(fs_info, 1);
 
        if (!skip_writeback) {
                ret = btrfs_wait_ordered_range(&inode->vfs_inode,
@@ -8401,7 +8401,15 @@ static int btrfs_truncate(struct btrfs_inode *inode, bool skip_writeback)
        /* Migrate the slack space for the truncate to our reserve */
        ret = btrfs_block_rsv_migrate(&fs_info->trans_block_rsv, rsv,
                                      min_size, false);
-       BUG_ON(ret);
+       /*
+        * We have reserved 2 metadata units when we started the transaction and
+        * min_size matches 1 unit, so this should never fail, but if it does,
+        * it's not critical we just fail truncation.
+        */
+       if (WARN_ON(ret)) {
+               btrfs_end_transaction(trans);
+               goto out;
+       }
 
        trans->block_rsv = rsv;
 
@@ -8449,7 +8457,14 @@ static int btrfs_truncate(struct btrfs_inode *inode, bool skip_writeback)
                btrfs_block_rsv_release(fs_info, rsv, -1, NULL);
                ret = btrfs_block_rsv_migrate(&fs_info->trans_block_rsv,
                                              rsv, min_size, false);
-               BUG_ON(ret);    /* shouldn't happen */
+               /*
+                * We have reserved 2 metadata units when we started the
+                * transaction and min_size matches 1 unit, so this should never
+                * fail, but if it does, it's not critical we just fail truncation.
+                */
+               if (WARN_ON(ret))
+                       break;
+
                trans->block_rsv = rsv;
        }