Re: [PATCH 1/4] btrfs: zoned: track only sequential zones as active

["Dongjiang Zhu" <zhudongjiang@fygo.io>]

在 2026/8/24 21:55, Johannes Thumshirn 写道:
> On Mon, Aug 24, 2026 at 02:49:53PM +0800, Dongjiang Zhu wrote:
>> [CAUSE]
>> The runtime activation and finish paths apply active-zone accounting to
>> both conventional and sequential stripes.
>>
>> At mount, however, active_zones_left is rebuilt from device zone
>> conditions.  Conventional zones report NOT_WP and are therefore not
>> included.  Meanwhile, all-conventional block groups are marked active
>> and added to zone_active_bgs, so mount recovery still decreases
>> reserved_active_zones for their metadata and system stripes.
>>
>> The two counters can therefore diverge across a remount.
>>
>> [BUG]
>> Each unmatched reservation decrement lets data consume one more
>> sequential active-zone slot.  Once the reservation becomes negative,
>> data can exhaust all such slots.
>>
>> A later transition from a conventional metadata or system target to a
>> sequential one can then fail, because finishing the conventional target
>> does not release a sequential active-zone slot.
>>
>> If this happens during synchronous transaction writeback,
>> btrfs_check_meta_write_pointer() returns -EAGAIN.  The transaction is
>> then aborted and the filesystem is forced read-only.
>>
>> The reservation mismatch was reproduced by creating small files with
>> large xattrs in batches, syncing them and remounting an HC620 filesystem
>> using SINGLE profiles:
>>
>>    active_zones_left:     126
>>    reserved_active_zones: 3 -> -8
>>
>> The active list contained one partially used conventional system block
>> group, nine full conventional metadata block groups and one partially
>> used conventional metadata block group.
>>
>> [FIX]
>> Use sequential stripes as the unit of active-zone tracking.
>>
>> Do not mark all-conventional block groups active.  Only sequential
>> stripes update the active-zone bitmap and non-data reservation during
>> activation, finish and mount recovery.
>>
>> Assisted-by: LLM
>> Signed-off-by: Dongjiang Zhu<zhudongjiang@fygo.io>
> Looks good, two small nits though in case you need to resend the series:
> 1) Please don't do the [CAUSE], [BUG], [FIX] thingy, I know Qu does it but
> noone else does that.

Sure, I will remove these headings from the commit messages in v2.

> 2) We now have a repeated pattern of:
> 
>    if (!btrfs_dev_is_sequential())
> 	return
>    if (max_active_zones)
>          return
> 
> Maybe have a helper for that? Something like
> 
> static inline bool btrfs_needs_active_zone_tracking(struct btrfs_device *dev,
> 						    u64 physical)
> {
> 	if (!btrfs_dev_is_sequential(dev, physical))
> 		return false;
> 
> 	if (dev->zone_info->max_active_zones == 0)
> 		return false;
> 
> 	return true;
> }
> 

Good idea. I will add the helper and use it for the repeated checks in
v2.

Thanks for the review.

Dongjiang Zhu

> Otherwise looks good,
> Reviewed-by: Johannes Thumshirn<johannes.thumshirn@wdc.com>
>