Re: [PATCH 09/12] btrfs: handle deletions from remapped block group

[Mark Harmstone <mark@harmstone.com>]

On 14/06/2025 12.42 am, Boris Burkov wrote:
... snip ...
>> @@ -3002,10 +3005,22 @@ static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
>>   		return ret;
>>   	}
>>   
>> -	ret = add_to_free_space_tree(trans, bytenr, num_bytes);
>> -	if (ret) {
>> -		btrfs_abort_transaction(trans, ret);
>> -		return ret;
>> +	if (btrfs_fs_incompat(trans->fs_info, REMAP_TREE)) {
>> +		bg = btrfs_lookup_block_group(trans->fs_info, bytenr);
>> +		bg_is_remapped = bg->flags & BTRFS_BLOCK_GROUP_REMAPPED;
> 
> As mentioned in the patch that sets the flag, this feels quite
> susceptible to races. I don't have a particular one in mind, it just
> feels off to set and rely on this flag to decide what work to do without
> some kind of explicit synchronization plan.

I thought this was safe, but I think you're right. btrfs_remove_extent_from_remap_tree()
ought to return a flag saying whether it touched the FST or not, rather than
leaving us to racily guess here.