[PATCH 0/3] Cleanup volume.c of unused parameters/variables

[PATCH 0/3] Cleanup volume.c of unused parameters/variables

[Nikolay Borisov]

Here are 3 cleanup patches which remove unused variables, parameters and 
kill a dummy function. 

Nikolay Borisov (3):
  btrfs: Remove unused variables
  btrfs: Remove unused parameters from volume.c functions
  btrfs: Remove find_raid56_stripe_len

 fs/btrfs/extent-tree.c |  6 ++----
 fs/btrfs/extent_io.c   |  2 +-
 fs/btrfs/volumes.c     | 44 ++++++++++----------------------------------
 fs/btrfs/volumes.h     |  3 +--
 4 files changed, 14 insertions(+), 41 deletions(-)

-- 
2.7.4

[PATCH 1/3] btrfs: Remove unused variables

[Nikolay Borisov]

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6466f53b1e81..9a38cbf233a1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1873,7 +1873,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	struct btrfs_fs_devices *cur_devices;
 	u64 num_devices;
 	int ret = 0;
-	bool clear_super = false;
 
 	mutex_lock(&uuid_mutex);
 
@@ -1909,7 +1908,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 		list_del_init(&device->dev_alloc_list);
 		device->fs_devices->rw_devices--;
 		mutex_unlock(&fs_info->chunk_mutex);
-		clear_super = true;
 	}
 
 	mutex_unlock(&uuid_mutex);
@@ -4128,7 +4126,6 @@ static int btrfs_uuid_scan_kthread(void *data)
 	struct btrfs_fs_info *fs_info = data;
 	struct btrfs_root *root = fs_info->tree_root;
 	struct btrfs_key key;
-	struct btrfs_key max_key;
 	struct btrfs_path *path = NULL;
 	int ret = 0;
 	struct extent_buffer *eb;
@@ -4147,10 +4144,6 @@ static int btrfs_uuid_scan_kthread(void *data)
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	key.offset = 0;
 
-	max_key.objectid = (u64)-1;
-	max_key.type = BTRFS_ROOT_ITEM_KEY;
-	max_key.offset = (u64)-1;
-
 	while (1) {
 		ret = btrfs_search_forward(root, &key, path, 0);
 		if (ret) {
@@ -6452,7 +6445,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 	struct extent_map *em;
 	u64 logical;
 	u64 length;
-	u64 stripe_len;
 	u64 devid;
 	u8 uuid[BTRFS_UUID_SIZE];
 	int num_stripes;
@@ -6461,7 +6453,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 
 	logical = key->offset;
 	length = btrfs_chunk_length(leaf, chunk);
-	stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
 	num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
 
 	ret = btrfs_check_chunk_valid(fs_info, leaf, chunk, logical);
-- 
2.7.4

[PATCH 2/3] btrfs: Remove unused parameters from volume.c functions

[Nikolay Borisov]

This also adjusts the respective callers in other files. Those were found with
-Wunused-parameter.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent-tree.c | 6 ++----
 fs/btrfs/extent_io.c   | 2 +-
 fs/btrfs/volumes.c     | 7 ++-----
 fs/btrfs/volumes.h     | 3 +--
 4 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f8aa512e9de5..d57f1f161fa2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -5819,7 +5819,7 @@ int btrfs_orphan_reserve_metadata(struct btrfs_trans_handle *trans,
 	 */
 	u64 num_bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
 
-	trace_btrfs_space_reservation(fs_info, "orphan", btrfs_ino(inode), 
+	trace_btrfs_space_reservation(fs_info, "orphan", btrfs_ino(inode),
 			num_bytes, 1);
 	return btrfs_block_rsv_migrate(src_rsv, dst_rsv, num_bytes, 1);
 }
@@ -9948,9 +9948,7 @@ btrfs_create_block_group_cache(struct btrfs_fs_info *fs_info,
 
 	cache->sectorsize = fs_info->sectorsize;
 	cache->fs_info = fs_info;
-	cache->full_stripe_len = btrfs_full_stripe_len(fs_info,
-						       &fs_info->mapping_tree,
-						       start);
+	cache->full_stripe_len = btrfs_full_stripe_len(fs_info, start);
 	set_free_space_tree_thresholds(cache);
 
 	atomic_set(&cache->count, 1);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 2e6f69908303..a6a71629190e 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1997,7 +1997,7 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,
 	 * read repair operation.
 	 */
 	btrfs_bio_counter_inc_blocked(fs_info);
-	if (btrfs_is_parity_mirror(fs_info, logical, length, mirror_num)) {
+	if (btrfs_is_parity_mirror(fs_info, logical, length)) {
 		/*
 		 * Note that we don't use BTRFS_MAP_WRITE because it's supposed
 		 * to update all raid stripes, but here we just want to correct
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9a38cbf233a1..171a8e79332f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3311,7 +3311,6 @@ static int chunk_devid_filter(struct extent_buffer *leaf,
 /* [pstart, pend) */
 static int chunk_drange_filter(struct extent_buffer *leaf,
 			       struct btrfs_chunk *chunk,
-			       u64 chunk_offset,
 			       struct btrfs_balance_args *bargs)
 {
 	struct btrfs_stripe *stripe;
@@ -3438,7 +3437,7 @@ static int should_balance_chunk(struct btrfs_fs_info *fs_info,
 
 	/* drange filter, makes sense only with devid filter */
 	if ((bargs->flags & BTRFS_BALANCE_ARGS_DRANGE) &&
-	    chunk_drange_filter(leaf, chunk, chunk_offset, bargs)) {
+	    chunk_drange_filter(leaf, chunk, bargs)) {
 		return 0;
 	}
 
@@ -5166,7 +5165,6 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 }
 
 unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
-				    struct btrfs_mapping_tree *map_tree,
 				    u64 logical)
 {
 	struct extent_map *em;
@@ -5184,8 +5182,7 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
 	return len;
 }
 
-int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info,
-			   u64 logical, u64 len, int mirror_num)
+int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 {
 	struct extent_map *em;
 	struct map_lookup *map;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 6f45fd60d15a..706ca5c2e974 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -481,9 +481,8 @@ void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info,
 					      struct btrfs_device *tgtdev);
 void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path);
 int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info,
-			   u64 logical, u64 len, int mirror_num);
+			   u64 logical, u64 len);
 unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
-				    struct btrfs_mapping_tree *map_tree,
 				    u64 logical);
 int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans,
 				struct btrfs_fs_info *fs_info,
-- 
2.7.4

[PATCH 3/3] btrfs: Remove find_raid56_stripe_len

[Nikolay Borisov]

find_raid56_stripe_len statically returns SZ_64K which equals BTRFS_STRIPE_LEN.
It's sole caller is __btrfs_alloc_chunk and it assigns the return value to ai
variable which is already set to BTRFS_STRIPE_LEN. So remove the function
invocation altogether and remove the function itself. Also remove the variable
since it's only aliasing BTRFS_STRIPE_LEN and use the define directly.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 171a8e79332f..8d7c28a95a90 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4594,12 +4594,6 @@ static int btrfs_cmp_device_info(const void *a, const void *b)
 	return 0;
 }
 
-static u32 find_raid56_stripe_len(u32 data_devices, u32 dev_stripe_target)
-{
-	/* TODO allow them to set a preferred stripe size */
-	return SZ_64K;
-}
-
 static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
 {
 	if (!(type & BTRFS_BLOCK_GROUP_RAID56_MASK))
@@ -4642,7 +4636,6 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	u64 max_chunk_size;
 	u64 stripe_size;
 	u64 num_bytes;
-	u64 raid_stripe_len = BTRFS_STRIPE_LEN;
 	int ndevs;
 	int i;
 	int j;
@@ -4784,16 +4777,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	 */
 	data_stripes = num_stripes / ncopies;
 
-	if (type & BTRFS_BLOCK_GROUP_RAID5) {
-		raid_stripe_len = find_raid56_stripe_len(ndevs - 1,
-							 info->stripesize);
+	if (type & BTRFS_BLOCK_GROUP_RAID5)
 		data_stripes = num_stripes - 1;
-	}
-	if (type & BTRFS_BLOCK_GROUP_RAID6) {
-		raid_stripe_len = find_raid56_stripe_len(ndevs - 2,
-							 info->stripesize);
+
+	if (type & BTRFS_BLOCK_GROUP_RAID6)
 		data_stripes = num_stripes - 2;
-	}
 
 	/*
 	 * Use the number of data stripes to figure out how big this chunk
@@ -4818,8 +4806,8 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	stripe_size = div_u64(stripe_size, dev_stripes);
 
 	/* align to BTRFS_STRIPE_LEN */
-	stripe_size = div64_u64(stripe_size, raid_stripe_len);
-	stripe_size *= raid_stripe_len;
+	stripe_size = div64_u64(stripe_size, BTRFS_STRIPE_LEN);
+	stripe_size *= BTRFS_STRIPE_LEN;
 
 	map = kmalloc(map_lookup_size(num_stripes), GFP_NOFS);
 	if (!map) {
@@ -4837,9 +4825,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		}
 	}
 	map->sector_size = info->sectorsize;
-	map->stripe_len = raid_stripe_len;
-	map->io_align = raid_stripe_len;
-	map->io_width = raid_stripe_len;
+	map->stripe_len = BTRFS_STRIPE_LEN;
+	map->io_align = BTRFS_STRIPE_LEN;
+	map->io_width = BTRFS_STRIPE_LEN;
 	map->type = type;
 	map->sub_stripes = sub_stripes;
 
-- 
2.7.4

Re: [PATCH 3/3] btrfs: Remove find_raid56_stripe_len

[Qu Wenruo]

On 2017年07月13日 16:45, Nikolay Borisov wrote:
> find_raid56_stripe_len statically returns SZ_64K which equals BTRFS_STRIPE_LEN.
> It's sole caller is __btrfs_alloc_chunk and it assigns the return value to ai
> variable which is already set to BTRFS_STRIPE_LEN. So remove the function
> invocation altogether and remove the function itself. Also remove the variable
> since it's only aliasing BTRFS_STRIPE_LEN and use the define directly.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>   fs/btrfs/volumes.c | 28 ++++++++--------------------
>   1 file changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 171a8e79332f..8d7c28a95a90 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4594,12 +4594,6 @@ static int btrfs_cmp_device_info(const void *a, const void *b)
>   	return 0;
>   }
>   
> -static u32 find_raid56_stripe_len(u32 data_devices, u32 dev_stripe_target)
> -{
> -	/* TODO allow them to set a preferred stripe size */
> -	return SZ_64K;
> -}
> -
>   static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
>   {
>   	if (!(type & BTRFS_BLOCK_GROUP_RAID56_MASK))
> @@ -4642,7 +4636,6 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   	u64 max_chunk_size;
>   	u64 stripe_size;
>   	u64 num_bytes;
> -	u64 raid_stripe_len = BTRFS_STRIPE_LEN;
>   	int ndevs;
>   	int i;
>   	int j;
> @@ -4784,16 +4777,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   	 */
>   	data_stripes = num_stripes / ncopies;
>   
> -	if (type & BTRFS_BLOCK_GROUP_RAID5) {
> -		raid_stripe_len = find_raid56_stripe_len(ndevs - 1,
> -							 info->stripesize);
> +	if (type & BTRFS_BLOCK_GROUP_RAID5)
>   		data_stripes = num_stripes - 1;
> -	}
> -	if (type & BTRFS_BLOCK_GROUP_RAID6) {
> -		raid_stripe_len = find_raid56_stripe_len(ndevs - 2,
> -							 info->stripesize);
> +
> +	if (type & BTRFS_BLOCK_GROUP_RAID6)
>   		data_stripes = num_stripes - 2;
> -	}
>   
>   	/*
>   	 * Use the number of data stripes to figure out how big this chunk
> @@ -4818,8 +4806,8 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   	stripe_size = div_u64(stripe_size, dev_stripes);
>   
>   	/* align to BTRFS_STRIPE_LEN */
> -	stripe_size = div64_u64(stripe_size, raid_stripe_len);
> -	stripe_size *= raid_stripe_len;
> +	stripe_size = div64_u64(stripe_size, BTRFS_STRIPE_LEN);
> +	stripe_size *= BTRFS_STRIPE_LEN;

The old code assumes raid_stripe_len can be other value, which may not 
be power of 2.
So it uses division then multiple to do alignment.

Now @raid_stripe_len is fixed to BTRFS_STRIPE_LEN, which is 64K and 
power of 2, so what about using round_down() macro?

Thanks,
Qu

>   
>   	map = kmalloc(map_lookup_size(num_stripes), GFP_NOFS);
>   	if (!map) {
> @@ -4837,9 +4825,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   		}
>   	}
>   	map->sector_size = info->sectorsize;
> -	map->stripe_len = raid_stripe_len;
> -	map->io_align = raid_stripe_len;
> -	map->io_width = raid_stripe_len;
> +	map->stripe_len = BTRFS_STRIPE_LEN;
> +	map->io_align = BTRFS_STRIPE_LEN;
> +	map->io_width = BTRFS_STRIPE_LEN;
>   	map->type = type;
>   	map->sub_stripes = sub_stripes;
>   
> 

Re: [PATCH 1/3] btrfs: Remove unused variables

[Qu Wenruo]

On 2017年07月13日 16:37, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Looks good.

Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>

Kernel makefile disabled unused-but-set-variable warning so we can't 
detect it in normal compile.

Thanks,
Qu

> ---
>   fs/btrfs/volumes.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6466f53b1e81..9a38cbf233a1 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1873,7 +1873,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
>   	struct btrfs_fs_devices *cur_devices;
>   	u64 num_devices;
>   	int ret = 0;
> -	bool clear_super = false;
>   
>   	mutex_lock(&uuid_mutex);
>   
> @@ -1909,7 +1908,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
>   		list_del_init(&device->dev_alloc_list);
>   		device->fs_devices->rw_devices--;
>   		mutex_unlock(&fs_info->chunk_mutex);
> -		clear_super = true;
>   	}
>   
>   	mutex_unlock(&uuid_mutex);
> @@ -4128,7 +4126,6 @@ static int btrfs_uuid_scan_kthread(void *data)
>   	struct btrfs_fs_info *fs_info = data;
>   	struct btrfs_root *root = fs_info->tree_root;
>   	struct btrfs_key key;
> -	struct btrfs_key max_key;
>   	struct btrfs_path *path = NULL;
>   	int ret = 0;
>   	struct extent_buffer *eb;
> @@ -4147,10 +4144,6 @@ static int btrfs_uuid_scan_kthread(void *data)
>   	key.type = BTRFS_ROOT_ITEM_KEY;
>   	key.offset = 0;
>   
> -	max_key.objectid = (u64)-1;
> -	max_key.type = BTRFS_ROOT_ITEM_KEY;
> -	max_key.offset = (u64)-1;
> -
>   	while (1) {
>   		ret = btrfs_search_forward(root, &key, path, 0);
>   		if (ret) {
> @@ -6452,7 +6445,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
>   	struct extent_map *em;
>   	u64 logical;
>   	u64 length;
> -	u64 stripe_len;
>   	u64 devid;
>   	u8 uuid[BTRFS_UUID_SIZE];
>   	int num_stripes;
> @@ -6461,7 +6453,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
>   
>   	logical = key->offset;
>   	length = btrfs_chunk_length(leaf, chunk);
> -	stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
>   	num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
>   
>   	ret = btrfs_check_chunk_valid(fs_info, leaf, chunk, logical);
> 

[PATCH v2] btrfs: Remove find_raid56_stripe_len

[Nikolay Borisov]

find_raid56_stripe_len statically returns SZ_64K which equals BTRFS_STRIPE_LEN.
It's sole caller is __btrfs_alloc_chunk and it assigns the return value to ai
variable which is already set to BTRFS_STRIPE_LEN. So remove the function
invocation altogether and remove the function itself. Also remove the variable
since it's only aliasing BTRFS_STRIPE_LEN and use the define directly. Use
the occassion to simplify the rounding down of stripe_size now that the value 
we want it to align is a power of 2. 

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

V2:
 * Incorporate Qu's suggestion to use round_down macro. Updated commit 
 message accordingly. 

 fs/btrfs/volumes.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 171a8e79332f..fa779875bcfb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4594,12 +4594,6 @@ static int btrfs_cmp_device_info(const void *a, const void *b)
 	return 0;
 }
 
-static u32 find_raid56_stripe_len(u32 data_devices, u32 dev_stripe_target)
-{
-	/* TODO allow them to set a preferred stripe size */
-	return SZ_64K;
-}
-
 static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
 {
 	if (!(type & BTRFS_BLOCK_GROUP_RAID56_MASK))
@@ -4642,7 +4636,6 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	u64 max_chunk_size;
 	u64 stripe_size;
 	u64 num_bytes;
-	u64 raid_stripe_len = BTRFS_STRIPE_LEN;
 	int ndevs;
 	int i;
 	int j;
@@ -4784,16 +4777,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	 */
 	data_stripes = num_stripes / ncopies;
 
-	if (type & BTRFS_BLOCK_GROUP_RAID5) {
-		raid_stripe_len = find_raid56_stripe_len(ndevs - 1,
-							 info->stripesize);
+	if (type & BTRFS_BLOCK_GROUP_RAID5)
 		data_stripes = num_stripes - 1;
-	}
-	if (type & BTRFS_BLOCK_GROUP_RAID6) {
-		raid_stripe_len = find_raid56_stripe_len(ndevs - 2,
-							 info->stripesize);
+
+	if (type & BTRFS_BLOCK_GROUP_RAID6)
 		data_stripes = num_stripes - 2;
-	}
 
 	/*
 	 * Use the number of data stripes to figure out how big this chunk
@@ -4818,8 +4806,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	stripe_size = div_u64(stripe_size, dev_stripes);
 
 	/* align to BTRFS_STRIPE_LEN */
-	stripe_size = div64_u64(stripe_size, raid_stripe_len);
-	stripe_size *= raid_stripe_len;
+	stripe_size = round_down(stripe_size, BTRFS_STRIPE_LEN);
 
 	map = kmalloc(map_lookup_size(num_stripes), GFP_NOFS);
 	if (!map) {
@@ -4837,9 +4824,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		}
 	}
 	map->sector_size = info->sectorsize;
-	map->stripe_len = raid_stripe_len;
-	map->io_align = raid_stripe_len;
-	map->io_width = raid_stripe_len;
+	map->stripe_len = BTRFS_STRIPE_LEN;
+	map->io_align = BTRFS_STRIPE_LEN;
+	map->io_width = BTRFS_STRIPE_LEN;
 	map->type = type;
 	map->sub_stripes = sub_stripes;
 
-- 
2.7.4

Re: [PATCH v2] btrfs: Remove find_raid56_stripe_len

[Qu Wenruo]

On 2017年07月14日 14:55, Nikolay Borisov wrote:
> find_raid56_stripe_len statically returns SZ_64K which equals BTRFS_STRIPE_LEN.
> It's sole caller is __btrfs_alloc_chunk and it assigns the return value to ai
> variable which is already set to BTRFS_STRIPE_LEN. So remove the function
> invocation altogether and remove the function itself. Also remove the variable
> since it's only aliasing BTRFS_STRIPE_LEN and use the define directly. Use
> the occassion to simplify the rounding down of stripe_size now that the value
> we want it to align is a power of 2.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>

Thanks,
Qu
> ---
> 
> V2:
>   * Incorporate Qu's suggestion to use round_down macro. Updated commit
>   message accordingly.
> 
>   fs/btrfs/volumes.c | 27 +++++++--------------------
>   1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 171a8e79332f..fa779875bcfb 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4594,12 +4594,6 @@ static int btrfs_cmp_device_info(const void *a, const void *b)
>   	return 0;
>   }
>   
> -static u32 find_raid56_stripe_len(u32 data_devices, u32 dev_stripe_target)
> -{
> -	/* TODO allow them to set a preferred stripe size */
> -	return SZ_64K;
> -}
> -
>   static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
>   {
>   	if (!(type & BTRFS_BLOCK_GROUP_RAID56_MASK))
> @@ -4642,7 +4636,6 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   	u64 max_chunk_size;
>   	u64 stripe_size;
>   	u64 num_bytes;
> -	u64 raid_stripe_len = BTRFS_STRIPE_LEN;
>   	int ndevs;
>   	int i;
>   	int j;
> @@ -4784,16 +4777,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   	 */
>   	data_stripes = num_stripes / ncopies;
>   
> -	if (type & BTRFS_BLOCK_GROUP_RAID5) {
> -		raid_stripe_len = find_raid56_stripe_len(ndevs - 1,
> -							 info->stripesize);
> +	if (type & BTRFS_BLOCK_GROUP_RAID5)
>   		data_stripes = num_stripes - 1;
> -	}
> -	if (type & BTRFS_BLOCK_GROUP_RAID6) {
> -		raid_stripe_len = find_raid56_stripe_len(ndevs - 2,
> -							 info->stripesize);
> +
> +	if (type & BTRFS_BLOCK_GROUP_RAID6)
>   		data_stripes = num_stripes - 2;
> -	}
>   
>   	/*
>   	 * Use the number of data stripes to figure out how big this chunk
> @@ -4818,8 +4806,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   	stripe_size = div_u64(stripe_size, dev_stripes);
>   
>   	/* align to BTRFS_STRIPE_LEN */
> -	stripe_size = div64_u64(stripe_size, raid_stripe_len);
> -	stripe_size *= raid_stripe_len;
> +	stripe_size = round_down(stripe_size, BTRFS_STRIPE_LEN);
>   
>   	map = kmalloc(map_lookup_size(num_stripes), GFP_NOFS);
>   	if (!map) {
> @@ -4837,9 +4824,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   		}
>   	}
>   	map->sector_size = info->sectorsize;
> -	map->stripe_len = raid_stripe_len;
> -	map->io_align = raid_stripe_len;
> -	map->io_width = raid_stripe_len;
> +	map->stripe_len = BTRFS_STRIPE_LEN;
> +	map->io_align = BTRFS_STRIPE_LEN;
> +	map->io_width = BTRFS_STRIPE_LEN;
>   	map->type = type;
>   	map->sub_stripes = sub_stripes;
>   
> 

Re: [PATCH 1/3] btrfs: Remove unused variables

[David Sterba]

For dead code removal, please document when it was last used and what's
the possible cause the code became unused. It's mostly for obvious
reasons, but sometimes it can reveal potential problems.

On Thu, Jul 13, 2017 at 11:37:41AM +0300, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/volumes.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6466f53b1e81..9a38cbf233a1 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1873,7 +1873,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
>  	struct btrfs_fs_devices *cur_devices;
>  	u64 num_devices;
>  	int ret = 0;
> -	bool clear_super = false;
>  
>  	mutex_lock(&uuid_mutex);
>  
> @@ -1909,7 +1908,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
>  		list_del_init(&device->dev_alloc_list);
>  		device->fs_devices->rw_devices--;
>  		mutex_unlock(&fs_info->chunk_mutex);
> -		clear_super = true;
>  	}
>  
>  	mutex_unlock(&uuid_mutex);
> @@ -4128,7 +4126,6 @@ static int btrfs_uuid_scan_kthread(void *data)
>  	struct btrfs_fs_info *fs_info = data;
>  	struct btrfs_root *root = fs_info->tree_root;
>  	struct btrfs_key key;
> -	struct btrfs_key max_key;
>  	struct btrfs_path *path = NULL;
>  	int ret = 0;
>  	struct extent_buffer *eb;
> @@ -4147,10 +4144,6 @@ static int btrfs_uuid_scan_kthread(void *data)
>  	key.type = BTRFS_ROOT_ITEM_KEY;
>  	key.offset = 0;
>  
> -	max_key.objectid = (u64)-1;
> -	max_key.type = BTRFS_ROOT_ITEM_KEY;
> -	max_key.offset = (u64)-1;
> -
>  	while (1) {
>  		ret = btrfs_search_forward(root, &key, path, 0);
>  		if (ret) {
> @@ -6452,7 +6445,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
>  	struct extent_map *em;
>  	u64 logical;
>  	u64 length;
> -	u64 stripe_len;
>  	u64 devid;
>  	u8 uuid[BTRFS_UUID_SIZE];
>  	int num_stripes;
> @@ -6461,7 +6453,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
>  
>  	logical = key->offset;
>  	length = btrfs_chunk_length(leaf, chunk);
> -	stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
>  	num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
>  
>  	ret = btrfs_check_chunk_valid(fs_info, leaf, chunk, logical);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] btrfs: Remove unused parameters from volume.c functions

[David Sterba]

On Thu, Jul 13, 2017 at 11:37:42AM +0300, Nikolay Borisov wrote:
> This also adjusts the respective callers in other files. Those were found with
> -Wunused-parameter.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Same comment as for 1/3, please document when and possibly why the
parameters became unused.

> -	trace_btrfs_space_reservation(fs_info, "orphan", btrfs_ino(inode), 
> +	trace_btrfs_space_reservation(fs_info, "orphan", btrfs_ino(inode),

So before we spend another round on whitespace discussion, please don't
sneak unrelated changes to patches.

Re: [PATCH v2] btrfs: Remove find_raid56_stripe_len

[David Sterba]

On Fri, Jul 14, 2017 at 09:55:41AM +0300, Nikolay Borisov wrote:
> find_raid56_stripe_len statically returns SZ_64K which equals BTRFS_STRIPE_LEN.
> It's sole caller is __btrfs_alloc_chunk and it assigns the return value to ai
> variable which is already set to BTRFS_STRIPE_LEN. So remove the function
> invocation altogether and remove the function itself. Also remove the variable
> since it's only aliasing BTRFS_STRIPE_LEN and use the define directly. Use
> the occassion to simplify the rounding down of stripe_size now that the value 
> we want it to align is a power of 2. 
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> 
> V2:
>  * Incorporate Qu's suggestion to use round_down macro. Updated commit 
>  message accordingly. 

The custom stripe length support would need to start from zero anyway,
so we don't lose anything if the function gets removed. All the
alignment, rounding etc will have to be checked.

Reviewed-by: David Sterba <dsterba@suse.com>

[PATCH v2] btrfs: Remove unused variables

[Nikolay Borisov]

clear_super - usage was removed in commit
cea67ab92d3d ("btrfs: clean the old superblocks before freeing the device") but
that change forgot to remove the actual variable.

max_key - commit 6174d3cb43aa ("Btrfs: remove unused max_key arg from
btrfs_search_forward") removed the max_key parameter but it forgot to remove
references from callers.

stripe_len - this one was added by e06cd3dd7cea ("Btrfs: add validadtion checks
for chunk loading") but even then it wasn't used.

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

Here's an updated change log with info when resp. variable became unused. 

 fs/btrfs/volumes.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6466f53b1e81..9a38cbf233a1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1873,7 +1873,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	struct btrfs_fs_devices *cur_devices;
 	u64 num_devices;
 	int ret = 0;
-	bool clear_super = false;
 
 	mutex_lock(&uuid_mutex);
 
@@ -1909,7 +1908,6 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 		list_del_init(&device->dev_alloc_list);
 		device->fs_devices->rw_devices--;
 		mutex_unlock(&fs_info->chunk_mutex);
-		clear_super = true;
 	}
 
 	mutex_unlock(&uuid_mutex);
@@ -4128,7 +4126,6 @@ static int btrfs_uuid_scan_kthread(void *data)
 	struct btrfs_fs_info *fs_info = data;
 	struct btrfs_root *root = fs_info->tree_root;
 	struct btrfs_key key;
-	struct btrfs_key max_key;
 	struct btrfs_path *path = NULL;
 	int ret = 0;
 	struct extent_buffer *eb;
@@ -4147,10 +4144,6 @@ static int btrfs_uuid_scan_kthread(void *data)
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	key.offset = 0;
 
-	max_key.objectid = (u64)-1;
-	max_key.type = BTRFS_ROOT_ITEM_KEY;
-	max_key.offset = (u64)-1;
-
 	while (1) {
 		ret = btrfs_search_forward(root, &key, path, 0);
 		if (ret) {
@@ -6452,7 +6445,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 	struct extent_map *em;
 	u64 logical;
 	u64 length;
-	u64 stripe_len;
 	u64 devid;
 	u8 uuid[BTRFS_UUID_SIZE];
 	int num_stripes;
@@ -6461,7 +6453,6 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 
 	logical = key->offset;
 	length = btrfs_chunk_length(leaf, chunk);
-	stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
 	num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
 
 	ret = btrfs_check_chunk_valid(fs_info, leaf, chunk, logical);
-- 
2.7.4

[PATCH v2] btrfs: Remove unused parameters from volume.c functions

[Nikolay Borisov]

This also adjusts the respective callers in other files. Those were found with
-Wunused-parameter.

btrfs_full_stripe_len's mapping_tree - introduced by
53b381b3abeb ("Btrfs: RAID5 and RAID6") but it was never really used even in
that commit

btrfs_is_parity_mirror's mirror_num - same as above

chunk_drange_filter's chunk_offset - introduced by 94e60d5a5c4b ("Btrfs: devid
subset filter") and never used.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent-tree.c | 4 +---
 fs/btrfs/extent_io.c   | 2 +-
 fs/btrfs/volumes.c     | 7 ++-----
 fs/btrfs/volumes.h     | 3 +--
 4 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f8aa512e9de5..a53441ffce71 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9948,9 +9948,7 @@ btrfs_create_block_group_cache(struct btrfs_fs_info *fs_info,
 
 	cache->sectorsize = fs_info->sectorsize;
 	cache->fs_info = fs_info;
-	cache->full_stripe_len = btrfs_full_stripe_len(fs_info,
-						       &fs_info->mapping_tree,
-						       start);
+	cache->full_stripe_len = btrfs_full_stripe_len(fs_info, start);
 	set_free_space_tree_thresholds(cache);
 
 	atomic_set(&cache->count, 1);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 2e6f69908303..a6a71629190e 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1997,7 +1997,7 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,
 	 * read repair operation.
 	 */
 	btrfs_bio_counter_inc_blocked(fs_info);
-	if (btrfs_is_parity_mirror(fs_info, logical, length, mirror_num)) {
+	if (btrfs_is_parity_mirror(fs_info, logical, length)) {
 		/*
 		 * Note that we don't use BTRFS_MAP_WRITE because it's supposed
 		 * to update all raid stripes, but here we just want to correct
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9a38cbf233a1..171a8e79332f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3311,7 +3311,6 @@ static int chunk_devid_filter(struct extent_buffer *leaf,
 /* [pstart, pend) */
 static int chunk_drange_filter(struct extent_buffer *leaf,
 			       struct btrfs_chunk *chunk,
-			       u64 chunk_offset,
 			       struct btrfs_balance_args *bargs)
 {
 	struct btrfs_stripe *stripe;
@@ -3438,7 +3437,7 @@ static int should_balance_chunk(struct btrfs_fs_info *fs_info,
 
 	/* drange filter, makes sense only with devid filter */
 	if ((bargs->flags & BTRFS_BALANCE_ARGS_DRANGE) &&
-	    chunk_drange_filter(leaf, chunk, chunk_offset, bargs)) {
+	    chunk_drange_filter(leaf, chunk, bargs)) {
 		return 0;
 	}
 
@@ -5166,7 +5165,6 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 }
 
 unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
-				    struct btrfs_mapping_tree *map_tree,
 				    u64 logical)
 {
 	struct extent_map *em;
@@ -5184,8 +5182,7 @@ unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
 	return len;
 }
 
-int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info,
-			   u64 logical, u64 len, int mirror_num)
+int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 {
 	struct extent_map *em;
 	struct map_lookup *map;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 6f45fd60d15a..706ca5c2e974 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -481,9 +481,8 @@ void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info,
 					      struct btrfs_device *tgtdev);
 void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path);
 int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info,
-			   u64 logical, u64 len, int mirror_num);
+			   u64 logical, u64 len);
 unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
-				    struct btrfs_mapping_tree *map_tree,
 				    u64 logical);
 int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans,
 				struct btrfs_fs_info *fs_info,
-- 
2.7.4

Re: [PATCH v2] btrfs: Remove unused variables

[David Sterba]

On Wed, Jul 19, 2017 at 10:47:57AM +0300, Nikolay Borisov wrote:
> clear_super - usage was removed in commit
> cea67ab92d3d ("btrfs: clean the old superblocks before freeing the device") but
> that change forgot to remove the actual variable.
> 
> max_key - commit 6174d3cb43aa ("Btrfs: remove unused max_key arg from
> btrfs_search_forward") removed the max_key parameter but it forgot to remove
> references from callers.
> 
> stripe_len - this one was added by e06cd3dd7cea ("Btrfs: add validadtion checks
> for chunk loading") but even then it wasn't used.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> 
> Here's an updated change log with info when resp. variable became unused. 

Great, thanks.

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH v2] btrfs: Remove unused parameters from volume.c functions

[David Sterba]

On Wed, Jul 19, 2017 at 10:48:42AM +0300, Nikolay Borisov wrote:
> This also adjusts the respective callers in other files. Those were found with
> -Wunused-parameter.
> 
> btrfs_full_stripe_len's mapping_tree - introduced by
> 53b381b3abeb ("Btrfs: RAID5 and RAID6") but it was never really used even in
> that commit
> 
> btrfs_is_parity_mirror's mirror_num - same as above
> 
> chunk_drange_filter's chunk_offset - introduced by 94e60d5a5c4b ("Btrfs: devid
> subset filter") and never used.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>