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

On 7/3/26 1:46 PM, Miquel Sabaté Solà wrote:
> 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: [email protected]
>>> Signed-off-by: Johannes Thumshirn <[email protected]>
>>> ---
>>>    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.

Yep that's what I did for v2. But I didn't want to send it out too 
quickly, to give other possible reviewers a chance to eventually spot 
another bug before.