Re: [PATCH v2 3/8] ext4: regenerate buddy after block freeing failed if under fc replay

[Baokun Li <libaokun1@huawei.com>]

On 2024/1/4 18:33, Jan Kara wrote:
> On Thu 21-12-23 23:05:53, Baokun Li wrote:
>> This reverts [Fixes] under fast commit replay. When we are freeing blocks
>> that have already been freed, the buddy may be corrupted, and we need to
>> regenerate the buddy when the fast commit is being replayed in order to
>> avoid using an corrupted buddy, since it will not mark the group block
>> bitmap as corrupted at that point.
> I'd rephrase the changelog as:
>
> This mostly reverts commit 6bd97bf273bd ("ext4: remove redundant
> mb_regenerate_buddy()") and reintroduces mb_regenerate_buddy(). Based on
> code in mb_free_blocks(), fast commit replay can end up marking as free
> blocks that are already marked as such. This causes corruption of the
> buddy bitmap so we need to regenerate it in that case.
>
> Otherwise the patch looks good to me so feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> 								Honza
Hello Honza, Happy New Year!

Thank you very much for your review!
This changelog looks a lot more relevant!
  I will use this changelog in the next version.
>> Reported-by: Jan Kara <jack@suse.cz>
>> Fixes: 6bd97bf273bd ("ext4: remove redundant mb_regenerate_buddy()")
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
>> ---
>>   fs/ext4/mballoc.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index a95fa6e2b0f9..f6131ba514c8 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -1233,6 +1233,24 @@ void ext4_mb_generate_buddy(struct super_block *sb,
>>   	atomic64_add(period, &sbi->s_mb_generation_time);
>>   }
>>   
>> +static void mb_regenerate_buddy(struct ext4_buddy *e4b)
>> +{
>> +	int count;
>> +	int order = 1;
>> +	void *buddy;
>> +
>> +	while ((buddy = mb_find_buddy(e4b, order++, &count)))
>> +		mb_set_bits(buddy, 0, count);
>> +
>> +	e4b->bd_info->bb_fragments = 0;
>> +	memset(e4b->bd_info->bb_counters, 0,
>> +		sizeof(*e4b->bd_info->bb_counters) *
>> +		(e4b->bd_sb->s_blocksize_bits + 2));
>> +
>> +	ext4_mb_generate_buddy(e4b->bd_sb, e4b->bd_buddy,
>> +		e4b->bd_bitmap, e4b->bd_group, e4b->bd_info);
>> +}
>> +
>>   /* The buddy information is attached the buddy cache inode
>>    * for convenience. The information regarding each group
>>    * is loaded via ext4_mb_load_buddy. The information involve
>> @@ -1921,6 +1939,8 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
>>   			ext4_mark_group_bitmap_corrupted(
>>   				sb, e4b->bd_group,
>>   				EXT4_GROUP_INFO_BBITMAP_CORRUPT);
>> +		} else {
>> +			mb_regenerate_buddy(e4b);
>>   		}
>>   		goto done;
>>   	}
>> -- 
>> 2.31.1
>>

Thanks again!
-- 
With Best Regards,
Baokun Li
.