Re: [PATCH 2/3] btrfs: Open code __btrfs_free_reserved_extent in btrfs_free_reserved_extent

[Nikolay Borisov <nborisov@suse.com>]

On 27.11.19 г. 20:55 ч., David Sterba wrote:
> On Thu, Nov 21, 2019 at 02:03:30PM +0200, Nikolay Borisov wrote:
>> __btrfs_free_reserved_extent performs 2 entirely different operations
>> depending on whether its 'pin' argument is true or false. This patch
>> lifts the 2nd case (pin is false) into it's sole caller
>> btrfs_free_reserved_extent. No semantics changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  fs/btrfs/extent-tree.c | 28 ++++++++++++++++------------
>>  1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 613c7bbf5cbd..dae6f8d07852 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -4164,17 +4164,7 @@ static int __btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
>>  		return -ENOSPC;
>>  	}
>>  
>> -	if (pin) {
>> -		ret = pin_down_extent(cache, start, len, 1);
>> -		if (ret)
>> -			goto out;
>> -	} else {
>> -		btrfs_add_free_space(cache, start, len);
>> -		btrfs_free_reserved_bytes(cache, len, delalloc);
>> -		trace_btrfs_reserved_extent_free(fs_info, start, len);
>> -	}
>> -
>> -out:
>> +	ret = pin_down_extent(cache, start, len, 1);
>>  	btrfs_put_block_group(cache);
>>  	return ret;
>>  }
>> @@ -4182,7 +4172,21 @@ static int __btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
>>  int btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
>>  			       u64 start, u64 len, int delalloc)
>>  {
>> -	return __btrfs_free_reserved_extent(fs_info, start, len, 0, delalloc);
>> +	struct btrfs_block_group *cache;
>> +
>> +	cache = btrfs_lookup_block_group(fs_info, start);
>> +	if (!cache) {
>> +		btrfs_err(fs_info, "Unable to find block group for %llu",
>> +			  start);
> 
> I think the error message should be either removed or at least hidden
> under enospc_debug. This has little value to a normal user and also the
> function can be indirectly called many times, spamming logs.

True but in general this should never happen because if we are freeing
an extent then it must have been reserved from a particular block group.
So if this triggers then we know something is awfully amiss.

>