Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <tencent_798118A8D3600FCC5E0E08EEB2341A8EE206@qq.com>
Date:   Thu, 3 Aug 2023 00:29:58 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH] ext4: Add correct group descriptors and reserved GDT
 blocks to system zone
Content-Language: en-US
To:     Theodore Ts'o <tytso@mit.edu>
Cc:     linux-ext4@vger.kernel.org
References: <tencent_4A474CC049B9E77D0F172468991EED5B9105@qq.com>
 <20230604034559.GG1128744@mit.edu>
From:   Wang Jianjian <wangjianjian0@foxmail.com>
In-Reply-To: <20230604034559.GG1128744@mit.edu>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Thanks Ted, I send a new fix. Please help review.

On 6/4/23 11:45, Theodore Ts'o wrote:
>> diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
>> index 5504f72bbbbe..5df357763975 100644
>> --- a/fs/ext4/block_validity.c
>> +++ b/fs/ext4/block_validity.c
>> @@ -224,11 +223,14 @@ int ext4_setup_system_zone(struct super_block *sb)
>>   
>>   	for (i=0; i < ngroups; i++) {
>>   		cond_resched();
>> -		if (ext4_bg_has_super(sb, i) &&
>> -		    ((i < 5) || ((i % flex_size) == 0))) {
>> +		unsigned int sb_num = ext4_bg_has_super(sb, i);
>> +		unsigned long gdb_num = ext4_bg_num_gdb(sb, i);
>> +		unsigned int rsvd_gdt = le16_to_cpu(sbi->es->s_reserved_gdt_blocks);
>> +
>> +		if (sb_num != 0 || gdb_num != 0) {
>>   			ret = add_system_zone(system_blks,
>>   					ext4_group_first_block_no(sb, i),
>> -					ext4_bg_num_gdb(sb, i) + 1, 0);
>> +					sb_num + gdb_num + rsvd_gdt, 0);
>>   			if (ret)
>>   				goto err;
>>   		}
> 
> 
> How the reserved GDT blocks should be added to the system zone are not
> handled correctly in this patch.   It can't be unconditionally added to
> all block groups.
> 
> See the logic in ext4_num_base_meta_clusters() in fs/ext4/balloc.c ---
> without the EXT4_NUM_B2C() at the end of the function, since the
> system zone tracking is done at the block level, not the cluster
> level.
> 
> 					- Ted
>