From 25983713922494cbf823689bf4847cea3f841935 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 22 Apr 2025 15:04:51 +0100 Subject: [PATCH] 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 Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/extent-tree.c | 20 ++++++++++++++++++-- fs/btrfs/transaction.c | 4 +++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 23aa6d06add0..a1c81033ff42 100644 --- 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 b06254aba8de..5a58d97a5dfc 100644 --- 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); -- 2.50.1