Re: [PATCH] btrfs: zoned: reset active_meta_bg on zone finish

["Miquel Sabaté Solà" <mssola@mssola.com>]

[-- Attachment #1: Type: text/plain, Size: 2736 bytes --]

Johannes Thumshirn @ 2026-07-03 12:01 +02:

> On 7/3/26 10:45 AM, Johannes Thumshirn wrote:
>> do_zone_finish() clears BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE and removes the
>> block group from zone_active_bgs, but only the path in
>> check_bg_is_active() resets fs_info->active_meta_bg / active_system_bg.
>> Any other finish path leaves active_meta_bg / active_system_bg pointing
>> at an inactive, fully written block group.
>>
>> Reset the corresponding active_{meta,system}_bg pointer in do_zone_finish()
>> so it can never go stale.
>>
>> Fixes: 13bb483d32ab ("btrfs: zoned: activate metadata block group on write time")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> ---
>>   fs/btrfs/zoned.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index 44a13ed6b8b2..c8c850de1702 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -2539,6 +2539,7 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
>>   	const bool is_metadata = (block_group->flags &
>>   			(BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM));
>>   	struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
>> +	struct btrfs_block_group **active_bg = NULL;
>>   	int ret = 0;
>>   	int i;
>>   @@ -2636,6 +2637,20 @@ static int do_zone_finish(struct btrfs_block_group
>> *block_group, bool fully_writ
>>   	/* For active_bg_list */
>>   	btrfs_put_block_group(block_group);
>>   +	if (block_group->flags & BTRFS_BLOCK_GROUP_SYSTEM)
>> +		active_bg = &fs_info->active_system_bg;
>> +	else if (block_group->flags & BTRFS_BLOCK_GROUP_METADATA)
>> +		active_bg = &fs_info->active_meta_bg;
>> +
>> +	if (active_bg) {
>> +		btrfs_zoned_meta_io_lock(fs_info);
>> +		if (*active_bg == block_group) {
>> +			btrfs_put_block_group(block_group);
>> +			*active_bg = NULL;
>> +		}
>> +		btrfs_zoned_meta_io_unlock(fs_info);
>> +	}
>> +
>>   	clear_and_wake_up_bit(BTRFS_FS_NEED_ZONE_FINISH, &fs_info->flags);
>>     	return 0;
>
> I think Sashiko has a point here:
>
> https://sashiko.dev/#/patchset/20260703084559.136605-1-johannes.thumshirn%40wdc.com
>
> check_bg_is_active() should take a reference before calling into
> do_zone_finish() or actually clearing fs_info->active_{meta,system}_bg can even
> be done in check_bg_is_active() after calling do_zone_finish().
>
> That'll then also eliminate Miquel's concerns.

I'd maybe take the latter to avoid adding more complexity to an already
complex do_zone_finish(). Besides, I see that do_zone_finish() is
already called in many other places throughout btrfs/zoned.c, so I
wonder if having the changes in do_zone_finish() would also "spill" over
there.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]