[1/6] btrfs: alloc_chunk: do not refurbish num_bytes
diff mbox series

Message ID 20181004212443.26519-2-hans.van.kranenburg@mendix.com
State New
Headers show
Series
  • Chunk allocator DUP fix and cleanups
Related show

Commit Message

Hans van Kranenburg Oct. 4, 2018, 9:24 p.m. UTC
num_bytes is used to store the chunk length of the chunk that we're
allocating. Do not reuse it for something really different in the same
function.

Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>
---
 fs/btrfs/volumes.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Nikolay Borisov Oct. 5, 2018, 8:59 a.m. UTC | #1
On  5.10.2018 00:24, Hans van Kranenburg wrote:
> num_bytes is used to store the chunk length of the chunk that we're
> allocating. Do not reuse it for something really different in the same
> function.
> 
> Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>

nit: That's a minor cleanup and brings no functional changes. I think it
even allows to give a more descriptive name of num_bytes such as
chunk_size, especially since we have a max_chunk_size. I think they
would pair nicely.

Anyway,

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/volumes.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f4405e430da6..9c9bb127eeee 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4837,8 +4837,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  		goto error_del_extent;
>  
>  	for (i = 0; i < map->num_stripes; i++) {
> -		num_bytes = map->stripes[i].dev->bytes_used + stripe_size;
> -		btrfs_device_set_bytes_used(map->stripes[i].dev, num_bytes);
> +		btrfs_device_set_bytes_used(map->stripes[i].dev,
> +					    map->stripes[i].dev->bytes_used +
> +					    stripe_size);
>  	}
>  
>  	atomic64_sub(stripe_size * map->num_stripes, &info->free_chunk_space);
>
Nikolay Borisov Oct. 5, 2018, 9 a.m. UTC | #2
On  5.10.2018 11:59, Nikolay Borisov wrote:
> 
> 
> On  5.10.2018 00:24, Hans van Kranenburg wrote:
>> num_bytes is used to store the chunk length of the chunk that we're
>> allocating. Do not reuse it for something really different in the same
>> function.
>>
>> Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>
> 
> nit: That's a minor cleanup and brings no functional changes. I think it
> even allows to give a more descriptive name of num_bytes such as
> chunk_size, especially since we have a max_chunk_size. I think they
> would pair nicely.

I saw that your patch 2 actually does that, so ignore this comment :)


> 
> Anyway,
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
>> ---
>>  fs/btrfs/volumes.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index f4405e430da6..9c9bb127eeee 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -4837,8 +4837,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>>  		goto error_del_extent;
>>  
>>  	for (i = 0; i < map->num_stripes; i++) {
>> -		num_bytes = map->stripes[i].dev->bytes_used + stripe_size;
>> -		btrfs_device_set_bytes_used(map->stripes[i].dev, num_bytes);
>> +		btrfs_device_set_bytes_used(map->stripes[i].dev,
>> +					    map->stripes[i].dev->bytes_used +
>> +					    stripe_size);
>>  	}
>>  
>>  	atomic64_sub(stripe_size * map->num_stripes, &info->free_chunk_space);
>>

Patch
diff mbox series

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f4405e430da6..9c9bb127eeee 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4837,8 +4837,9 @@  static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		goto error_del_extent;
 
 	for (i = 0; i < map->num_stripes; i++) {
-		num_bytes = map->stripes[i].dev->bytes_used + stripe_size;
-		btrfs_device_set_bytes_used(map->stripes[i].dev, num_bytes);
+		btrfs_device_set_bytes_used(map->stripes[i].dev,
+					    map->stripes[i].dev->bytes_used +
+					    stripe_size);
 	}
 
 	atomic64_sub(stripe_size * map->num_stripes, &info->free_chunk_space);