Re: [PATCH] ext4: avoid allocate block from corrupted group in ext4_mb_find_by_goal()

["yebin (H)" <yebin10@huawei.com>]

On 2026/3/3 10:36, Baokun Li wrote:
>
> On 3/2/26 9:46 PM, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> There's issue as follows:
>> ...
>> EXT4-fs (mmcblk0p1): Delayed block allocation failed for inode 206 at logical offset 0 with max blocks 1 with error 117
>> EXT4-fs (mmcblk0p1): This should not happen!! Data will be lost
>>
>> EXT4-fs (mmcblk0p1): Delayed block allocation failed for inode 206 at logical offset 0 with max blocks 1 with error 117
>> EXT4-fs (mmcblk0p1): This should not happen!! Data will be lost
>>
>> EXT4-fs (mmcblk0p1): Delayed block allocation failed for inode 206 at logical offset 0 with max blocks 1 with error 117
>> EXT4-fs (mmcblk0p1): This should not happen!! Data will be lost
>>
>> EXT4-fs (mmcblk0p1): Delayed block allocation failed for inode 206 at logical offset 0 with max blocks 1 with error 117
>> EXT4-fs (mmcblk0p1): This should not happen!! Data will be lost
>>
>> EXT4-fs (mmcblk0p1): Delayed block allocation failed for inode 2243 at logical offset 0 with max blocks 1 with error 117
>> EXT4-fs (mmcblk0p1): This should not happen!! Data will be lost
>>
>> EXT4-fs (mmcblk0p1): Delayed block allocation failed for inode 2239 at logical offset 0 with max blocks 1 with error 117
>> EXT4-fs (mmcblk0p1): This should not happen!! Data will be lost
>>
>> EXT4-fs (mmcblk0p1): error count since last fsck: 1
>> EXT4-fs (mmcblk0p1): initial error at time 1765597433: ext4_mb_generate_buddy:760
>> EXT4-fs (mmcblk0p1): last error at time 1765597433: ext4_mb_generate_buddy:760
>> ...
>>
>> According to the log analysis, blocks are always requested from the
>> corrupted block group. This may happen as follows:
>> ext4_mb_find_by_goal
>>    ext4_mb_load_buddy
>>     ext4_mb_load_buddy_gfp
>>       ext4_mb_init_cache
>>        ext4_read_block_bitmap_nowait
>>        ext4_wait_block_bitmap
>>         ext4_validate_block_bitmap
>>          if (!grp || EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
>>           return -EFSCORRUPTED; // There's no logs.
>>   if (err)
>>    return err;  // Will return error
>> ext4_lock_group(ac->ac_sb, group);
>>    if (unlikely(EXT4_MB_GRP_BBITMAP_CORRUPT(e4b->bd_info))) // Unreachable
>>     goto out;
>>
>> After commit 9008a58e5dce ("ext4: make the bitmap read routines return
>> real error codes") merged, Commit 163a203ddb36 ("ext4: mark block group
>> as corrupt on block bitmap error") is no real solution for allocating
>> blocks from corrupted block groups. This is because if
>> 'EXT4_MB_GRP_BBITMAP_CORRUPT(e4b->bd_info)' is true, then
>> 'ext4_mb_load_buddy()' may return an error. This means that the block
>> allocation will fail.
>> Therefore, check block group if corrupted when ext4_mb_load_buddy()
>> returns error.
>
> Good catch!
>
> Agreed, we should try other groups upon failure unless it's a goal-only
> allocation.
>
> But note that e4b->bd_info might be uninitialized if ext4_mb_load_buddy()
> fails.
>
The situation you mentioned probably doesn't exist.
ext4_mb_find_by_goal
   struct ext4_group_info *grp = ext4_get_group_info(ac->ac_sb, group);
   if (!grp)   // The possibility that e4b->bd_info is not initialized 
has been avoided.
     return -EFSCORRUPTED;
   err = ext4_mb_load_buddy(ac->ac_sb, group, e4b);
      ext4_mb_load_buddy_gfp(sb, group, e4b, GFP_NOFS);
        grp = ext4_get_group_info(sb, group);
        if (!grp)   // This condition probably will not be met.
          return -EFSCORRUPTED;
        e4b->bd_info = grp;
> I think we can optimize this in ext4_mb_regular_allocator(): we can record
> the error from ext4_mb_find_by_goal() but avoid an early exit.
>
> Specifically, after checking that EXT4_MB_HINT_GOAL_ONLY is not set,
> we can assign the error to ac->ac_first_err. This way, if subsequent
> allocation attempts still fail, we can preserve the original.
>
>
> Cheers,
> Baokun
>
>> Fixes: 163a203ddb36 ("ext4: mark block group as corrupt on block bitmap error")
>> Fixes: 9008a58e5dce ("ext4: make the bitmap read routines return real error codes")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   fs/ext4/mballoc.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index e2341489f4d0..ffa6886de8a3 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -2443,8 +2443,12 @@ int ext4_mb_find_by_goal(struct ext4_allocation_context *ac,
>>   		return 0;
>>
>>   	err = ext4_mb_load_buddy(ac->ac_sb, group, e4b);
>> -	if (err)
>> +	if (err) {
>> +		if (EXT4_MB_GRP_BBITMAP_CORRUPT(e4b->bd_info) &&
>> +		    !(ac->ac_flags & EXT4_MB_HINT_GOAL_ONLY))
>> +			return 0;
>>   		return err;
>> +	}
>>
>>   	ext4_lock_group(ac->ac_sb, group);
>>   	if (unlikely(EXT4_MB_GRP_BBITMAP_CORRUPT(e4b->bd_info)))
>
>
> .
>