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-----