btrfs: don't BUG_ON() when unpinning extents during transaction commit

In the final phase of a transaction commit, when unpinning extents at
btrfs_finish_extent_commit(), there's no need to BUG_ON() if we fail to
unpin an extent range. All that can happen is that we fail to return the
extent range to the in-memory free space cache, meaning no future space
allocations can reuse that extent range while the fs is mounted.

So instead return the error to the caller and make it abort the
transaction, so that the error is noticed and prevent misteriously leaking
space. We keep track of the first error we get while unpinning an extent
range and keep trying to unpin all the following extent ranges, so that
we attempt to do all discards. The transaction abort will deal with all
resource cleanups.

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/extent-tree.c b/fs/btrfs/extent-tree.c
index 23aa6d06add0ffd61c568520c2cfc2106a2d7148..a1c81033ff4228a20096036921ecea827ef689cf 100644 (file)
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2821,6 +2821,7 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
        struct extent_io_tree *unpin;
        u64 start;
        u64 end;
+       int unpin_error = 0;
        int ret;
 
        unpin = &trans->transaction->pinned_extents;
@@ -2841,7 +2842,22 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
 
                btrfs_clear_extent_dirty(unpin, start, end, &cached_state);
                ret = unpin_extent_range(fs_info, start, end, true);
-               BUG_ON(ret);
+               /*
+                * If we get an error unpinning an extent range, store the first
+                * error to return later after trying to unpin all ranges and do
+                * the sync discards. Our caller will abort the transaction
+                * (which already wrote new superblocks) and on the next mount
+                * the space will be available as it was pinned by in-memory
+                * only structures in this phase.
+                */
+               if (ret) {
+                       btrfs_err_rl(fs_info,
+"failed to unpin extent range [%llu, %llu] when committing transaction %llu: %s (%d)",
+                                    start, end, trans->transid,
+                                    btrfs_decode_error(ret), ret);
+                       if (!unpin_error)
+                               unpin_error = ret;
+               }
                mutex_unlock(&fs_info->unused_bg_unpin_mutex);
                btrfs_free_extent_state(cached_state);
                cond_resched();
@@ -2888,7 +2904,7 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
                }
        }
 
-       return 0;
+       return unpin_error;
 }
 
 /*

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index b06254aba8de2574a580f4e3082fd957a10d9b1b..5a58d97a5dfcf6fd8c464a1d002dab3654ff38ab 100644 (file)
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -2554,7 +2554,9 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
        wake_up(&cur_trans->commit_wait);
        btrfs_trans_state_lockdep_release(fs_info, BTRFS_LOCKDEP_TRANS_SUPER_COMMITTED);
 
-       btrfs_finish_extent_commit(trans);
+       ret = btrfs_finish_extent_commit(trans);
+       if (ret)
+               goto scrub_continue;
 
        if (test_bit(BTRFS_TRANS_HAVE_FREE_BGS, &cur_trans->flags))
                btrfs_clear_space_info_full(fs_info);