[2/3] btrfs: Open code __btrfs_free_reserved_extent in btrfs_free_reserved_extent
diff mbox series

Message ID 20191121120331.29070-3-nborisov@suse.com
State New
Headers show
Series
  • 3 misc patches
Related show

Commit Message

Nikolay Borisov Nov. 21, 2019, 12:03 p.m. UTC
__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(-)

Comments

David Sterba Nov. 27, 2019, 6:55 p.m. UTC | #1
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.
Nikolay Borisov Nov. 29, 2019, 8:44 a.m. UTC | #2
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.

>

Patch
diff mbox series

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);
+		return -ENOSPC;
+	}
+
+	btrfs_add_free_space(cache, start, len);
+	btrfs_free_reserved_bytes(cache, len, delalloc);
+	trace_btrfs_reserved_extent_free(fs_info, start, len);
+
+	btrfs_put_block_group(cache);
+	return 0;
 }
 
 int btrfs_free_and_pin_reserved_extent(struct btrfs_fs_info *fs_info,