btrfs: keep unused block groups queued when a pass fails

Once any block_group sets ret!=0 in the main loop of
btrfs_delete_unused_bgs(), the check
  if (ret || btrfs_mixed_space_info(space_info)) {
          btrfs_put_block_group(block_group);
          continue;
  }
skips the rest of the unused bgs while unlinking them from
fs_info->unused_bgs. There is no "level triggered" re-queueing of empty
block groups onto fs_info->unused_bgs so it is possible to leak quite a
bit of space this way and unless we happen to get a balance or
re-use/re-empty one of these bgs, they are leaked for good, which can
lead to a spurious enospc later.

While I have observed such leaked blocked groups that are empty but not
on the unused_bgs list on production systems, I have not observed that
it is definitely due to this issue. I also reproduced this behavior by
injecting an ENOSPC error from btrfs_start_trans_remove_block_group
which can also fail with ENOMEM, so this feels like a legitimate
injection point.

To fix it, instead of checking ret in the loop, just break out of the
loop when ret != 0. Also, link the bg to the retry list at the
individual failure sites so that the failing bg is not leaked.

Assisted-by: LLM (reproducer/error injection)
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index ee182369254c08ba7051ec1b0181cf210c7f60c7..2eb09c9901c9e1303239eccb32fbef816f062eb0 100644 (file)
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1612,7 +1612,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
 
                space_info = block_group->space_info;
 
-               if (ret || btrfs_mixed_space_info(space_info)) {
+               if (btrfs_mixed_space_info(space_info)) {
                        btrfs_put_block_group(block_group);
                        continue;
                }
@@ -1727,6 +1727,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
                ret = inc_block_group_ro(block_group, false);
                up_write(&space_info->groups_sem);
                if (ret < 0) {
+                       btrfs_link_bg_list(block_group, &retry_list);
                        ret = 0;
                        goto next;
                }
@@ -1749,6 +1750,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
                                                     block_group->start);
                if (IS_ERR(trans)) {
                        btrfs_dec_block_group_ro(block_group);
+                       btrfs_link_bg_list(block_group, &retry_list);
                        ret = PTR_ERR(trans);
                        goto next;
                }
@@ -1759,6 +1761,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
                 */
                if (!clean_pinned_extents(trans, block_group)) {
                        btrfs_dec_block_group_ro(block_group);
+                       btrfs_link_bg_list(block_group, &retry_list);
                        goto end_trans;
                }
 
@@ -1845,6 +1848,8 @@ end_trans:
 next:
                btrfs_put_block_group(block_group);
                spin_lock(&fs_info->unused_bgs_lock);
+               if (ret)
+                       break;
        }
        list_splice_tail(&retry_list, &fs_info->unused_bgs);
        spin_unlock(&fs_info->unused_bgs_lock);