Re: [PATCH] btrfs: abort transaction in do_remap_reloc_trans() on failure

[Mark Harmstone <mark@harmstone.com>]

On 07/04/2026 10.31 pm, Boris Burkov wrote:
> On Tue, Apr 07, 2026 at 05:16:56PM +0100, Mark Harmstone wrote:
>> If one of the calls made by do_remap_reloc_trans() fails, we can leave
>> the remap tree in an inconsistent state. Abort the transaction if this
>> happens, to prevent the corrupt state from reaching the disk.
>>
>> Signed-off-by: Mark Harmstone <mark@harmstone.com>
>> ---
>>   fs/btrfs/relocation.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 1c42c5180bddd5..277a0042967665 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -5089,6 +5089,7 @@ static int do_remap_reloc_trans(struct btrfs_fs_info *fs_info,
>>   	btrfs_free_reserved_extent(fs_info, new_addr, length, 0);
>>   
>>   	mutex_unlock(&fs_info->remap_mutex);
>> +	btrfs_abort_transaction(trans, ret);
> 
> Counterintuitively to the usual pattern of sharing code in fail: labels,
> I think it is quite nice to put a separate abort at each failing code
> path that requires an abort. That way we get better diagnostic
> information on what actually went wrong.
> 
> e.g. a recent patch from Filipe unfolding it across a function call:
> https://lore.kernel.org/linux-btrfs/99743ac0b4b9a69dcaa8f9adcaf301bd1a0f1a89.1769163248.git.fdmanana@suse.com/

Thanks Boris - if aborting a transaction logs the line number, that's a 
very good reason to do it in place. This also shows that the call to 
btrfs_add_to_free_space_tree after add_remap_entry fails is pointless, 
as the transaction is about to be aborted.

> Thanks,
> Boris
> 
>>   	btrfs_end_transaction(trans);
>>   
>>   	return ret;
>> -- 
>> 2.52.0
>>