Re: [PATCH 3/3] btrfs: zoned: limit number of zones reclaimed in flush_space

[Johannes Thumshirn <Johannes.Thumshirn@wdc.com>]

On 3/3/26 8:41 AM, Damien Le Moal wrote:
> On 3/2/26 23:39, Johannes Thumshirn wrote:
>> Limit the number of zones reclaimed in flush_space()'s RECLAIM_ZONES
>> state.
>>
>> This prevents possibly long running reclaim sweeps to block other tasks in
>> the system, while the system is under preassure anyways, causing the
> s/preassure/pressure

Oops. M-x flyspell would've helped :(


>
>> tasks to hang.
>>
> [...]
>> To prevent these long running reclaims from blocking the system, only
>> reclaim 5 block_groups in the RECLAIM_ZONES state of flush_space(). Also
> 5 seems very arbitrary. For a device with very large zones, this could still
> take some time and cause the problem again. Why not iterate the block groups one
> by one ? Is there any benefit to batching like this ?

I thought about limiting to the reclaim to a single block-group as well, 
but this potentially leads us to a situation where we try to allocate, 
run out of space, reclaim a block-group, try to allocate, run out of 
space, reclaim a block-group, yada yada totally tanking performance.

So if we batch a few block-groups, we need to reclaim a few block-groups 
(limiting to max 5) and then we at least have a bit of room to allocate 
again. Potentially having time for a transaction commit coming in and 
triggering the real (non emergency) garbage collection.


>> as these reclaims are now constrained, it opens up the use for a
>> synchronous call to brtfs_reclaim_block_groups(), eliminating the need
>> to place the reclaim task on a workqueue and then flushing the workqueue
>> again.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>
>> -static int btrfs_reclaim_block_group(struct btrfs_block_group *bg)
>> +static int btrfs_reclaim_block_group(struct btrfs_block_group *bg, int *reclaimed)
>>   {
>>   	struct btrfs_fs_info *fs_info = bg->fs_info;
>>   	struct btrfs_space_info *space_info = bg->space_info;
>> @@ -2036,15 +2036,17 @@ static int btrfs_reclaim_block_group(struct btrfs_block_group *bg)
>>   	if (space_info->total_bytes < old_total)
>>   		btrfs_set_periodic_reclaim_ready(space_info, true);
>>   	spin_unlock(&space_info->lock);
>> +	(*reclaimed)++;
> If ret != 0, it means that btrfs_relocate_chunk() failed. So in that case,
> shouldn't you skip incrementing the reclaimed counter ?

True. We also increment the statistics if reclaim failed. Boris?


>>   
>>   	return ret;
>>   }
>
>