ext4: avoid bb_free and bb_fragments inconsistency in mb_free_blocks()

After updating bb_free in mb_free_blocks, it is possible to return without
updating bb_fragments because the block being freed is found to have
already been freed, which leads to inconsistency between bb_free and
bb_fragments.

Since the group may be unlocked in ext4_grp_locked_error(), this can lead
to problems such as dividing by zero when calculating the average fragment
length. Hence move the update of bb_free to after the block double-free
check guarantees that the corresponding statistics are updated only after
the core block bitmap is modified.

Fixes: eabe0444df90 ("ext4: speed-up releasing blocks on commit")
CC:  <stable@vger.kernel.org> # 3.10
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/20240104142040.2835097-5-libaokun1@huawei.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index c97ad0e778318dbde7f87c0f2d3f65713d5979d1..fa351aa323dcf2ec4a3ce8ddb687eca62aa9a70a 100644 (file)
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1909,11 +1909,6 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
        mb_check_buddy(e4b);
        mb_free_blocks_double(inode, e4b, first, count);
 
-       this_cpu_inc(discard_pa_seq);
-       e4b->bd_info->bb_free += count;
-       if (first < e4b->bd_info->bb_first_free)
-               e4b->bd_info->bb_first_free = first;
-
        /* access memory sequentially: check left neighbour,
         * clear range and then check right neighbour
         */
@@ -1927,23 +1922,31 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
                struct ext4_sb_info *sbi = EXT4_SB(sb);
                ext4_fsblk_t blocknr;
 
+               /*
+                * Fastcommit replay can free already freed blocks which
+                * corrupts allocation info. Regenerate it.
+                */
+               if (sbi->s_mount_state & EXT4_FC_REPLAY) {
+                       mb_regenerate_buddy(e4b);
+                       goto check;
+               }
+
                blocknr = ext4_group_first_block_no(sb, e4b->bd_group);
                blocknr += EXT4_C2B(sbi, block);
-               if (!(sbi->s_mount_state & EXT4_FC_REPLAY)) {
-                       ext4_grp_locked_error(sb, e4b->bd_group,
-                                             inode ? inode->i_ino : 0,
-                                             blocknr,
-                                             "freeing already freed block (bit %u); block bitmap corrupt.",
-                                             block);
-                       ext4_mark_group_bitmap_corrupted(
-                               sb, e4b->bd_group,
+               ext4_grp_locked_error(sb, e4b->bd_group,
+                                     inode ? inode->i_ino : 0, blocknr,
+                                     "freeing already freed block (bit %u); block bitmap corrupt.",
+                                     block);
+               ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
                                EXT4_GROUP_INFO_BBITMAP_CORRUPT);
-               } else {
-                       mb_regenerate_buddy(e4b);
-               }
-               goto done;
+               return;
        }
 
+       this_cpu_inc(discard_pa_seq);
+       e4b->bd_info->bb_free += count;
+       if (first < e4b->bd_info->bb_first_free)
+               e4b->bd_info->bb_first_free = first;
+
        /* let's maintain fragments counter */
        if (left_is_free && right_is_free)
                e4b->bd_info->bb_fragments--;
@@ -1968,9 +1971,9 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
        if (first <= last)
                mb_buddy_mark_free(e4b, first >> 1, last >> 1);
 
-done:
        mb_set_largest_free_order(sb, e4b->bd_info);
        mb_update_avg_fragment_size(sb, e4b->bd_info);
+check:
        mb_check_buddy(e4b);
 }