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

Miquel Sabaté Solà <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs,org.kernel.vger.stable
Message-ID <87jyrc70tt.fsf@>
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.
signature.asc (application/pgp-signature, 897 B)
-----BEGIN PGP SIGNATURE-----

iQJiBAEBCgBMFiEEG6U8esk9yirP39qXlr6Mb9idZWUFAmpHoQ4bFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMiwyEhxtc3NvbGFAbXNzb2xhLmNvbQAKCRCWvoxv2J1l
Za2AD/4q8vKER+44H86pKGnSr+12cK98Uo510nLSg0xOZaTTw9DbPDDg1Shb/Bcg
qolF0/i5zsdXXE5CuKUduPECDgUXpMeB0DvS251BqSbOv1PMX69zXfT0bQFEQipu
w7fJIPfd08gwoyAIyHN2BPdnnyoe7f8w8oNtHPGDEg5nIp0z+iyEOdhCFkv6ta3P
xaynLdiDTCDLN43+8dtTQQoErf4r24l4TaO3ndo5gyZ294iMNvzQA9tdrmiGPA5J
vdwVNDvNOsMij3Rk5x/b2w4SJRXFYnbbP5HZk4m/dFxdBF8HgZ+vf5BC06J2TL1Y
KiNty8mKoB9zJux30/IctEs53l1ioCKjMmamClORJ0vs97Mm/sdkkt7Y8WQsCktV
2kO0vbhkr7MNOBbp6VbdLQ7gS1rwLOgj5iXDo6z5yIrcHhWzFcHnWjKcZo/r10Wx
M9mmW3pn7I52dubPHjZy8MdsK1C5dTTQXnjNf4gN0LkUAObm0a15GKytxZnRhHPT
ppNe2ah37GFFBCwkDOu3fF/9mparqD9D1ai8qDktKEkBa2ZQa/OmfOUkl9p/0eGR
IUn9GrVishpIdmwqEboiR7Vvd0WA9cq+6n9Wu9gEGreyswqTePY40lE3IUUkM+Jw
HtPd7q0XGWIc4C4whInzPBtuqq/YHatHqRn6qO8vAXzCMACosA==
=bcxr
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.