Re: [PATCH v2 1/2] btrfs: Refactor __get_raid_index() to btrfs_bg_flags_to_raid_index()

[David Sterba]

On Tue, Jan 30, 2018 at 06:20:45PM +0800, Qu Wenruo wrote:
> Function __get_raid_index() is used to convert block group flags into
> raid index, which can be used to get various info directly from
> btrfs_raid_array[].
> 
> Refactor this function a little:
> 
> 1) Rename to btrfs_bg_flags_to_raid_index()
>Double underline prefix is normally for internal functions, while the
>function is used by both extent-tree and volumes.
> 
>Although the name is a little longer, but it should explain its usage
>quite well.
> 
> 2) Move it to volumes.h and make it static inline
>Just several if-else branches, really no need to define it as a normal
>function.
> 
>This also makes later code re-use between kernel and btrfs-progs
>easier.
> 
> 3) Remove function get_block_group_index()
>Really no need to do such a simple thing as an exported function.

Agreed on this kind of cleanups. Patches added to next, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] btrfs: Refactor __get_raid_index() to btrfs_bg_flags_to_raid_index()

[Qu Wenruo]

Function __get_raid_index() is used to convert block group flags into
raid index, which can be used to get various info directly from
btrfs_raid_array[].

Refactor this function a little:

1) Rename to btrfs_bg_flags_to_raid_index()
   Double underline prefix is normally for internal functions, while the
   function is used by both extent-tree and volumes.

   Although the name is a little longer, but it should explain its usage
   quite well.

2) Move it to volumes.h and make it static inline
   Just several if-else branches, really no need to define it as a normal
   function.

   This also makes later code re-use between kernel and btrfs-progs
   easier.

3) Remove function get_block_group_index()
   Really no need to do such a simple thing as an exported function.

Signed-off-by: Qu Wenruo 
Reviewed-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
v2:
  Remove get_block_group_index() in this patch.
  Add extra comment for the usage of btrfs_bg_flags_to_raid_index()
---
 fs/btrfs/ctree.h   |  1 -
 fs/btrfs/extent-tree.c | 41 ++---
 fs/btrfs/volumes.c |  2 +-
 fs/btrfs/volumes.h | 22 ++
 4 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 13c260b525a1..27249240fa3e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2629,7 +2629,6 @@ struct btrfs_block_group_cache *btrfs_lookup_block_group(
 u64 bytenr);
 void btrfs_get_block_group(struct btrfs_block_group_cache *cache);
 void btrfs_put_block_group(struct btrfs_block_group_cache *cache);
-int get_block_group_index(struct btrfs_block_group_cache *cache);
 struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
 struct btrfs_root *root,
 u64 parent, u64 root_objectid,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 2f4328511ac8..2693c8617817 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -7346,29 +7346,6 @@ wait_block_group_cache_done(struct 
btrfs_block_group_cache *cache)
return ret;
 }
 
-int __get_raid_index(u64 flags)
-{
-   if (flags & BTRFS_BLOCK_GROUP_RAID10)
-   return BTRFS_RAID_RAID10;
-   else if (flags & BTRFS_BLOCK_GROUP_RAID1)
-   return BTRFS_RAID_RAID1;
-   else if (flags & BTRFS_BLOCK_GROUP_DUP)
-   return BTRFS_RAID_DUP;
-   else if (flags & BTRFS_BLOCK_GROUP_RAID0)
-   return BTRFS_RAID_RAID0;
-   else if (flags & BTRFS_BLOCK_GROUP_RAID5)
-   return BTRFS_RAID_RAID5;
-   else if (flags & BTRFS_BLOCK_GROUP_RAID6)
-   return BTRFS_RAID_RAID6;
-
-   return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
-}
-
-int get_block_group_index(struct btrfs_block_group_cache *cache)
-{
-   return __get_raid_index(cache->flags);
-}
-
 static const char *btrfs_raid_type_names[BTRFS_NR_RAID_TYPES] = {
[BTRFS_RAID_RAID10] = "raid10",
[BTRFS_RAID_RAID1]  = "raid1",
@@ -7483,7 +7460,7 @@ static noinline int find_free_extent(struct btrfs_fs_info 
*fs_info,
u64 empty_cluster = 0;
struct btrfs_space_info *space_info;
int loop = 0;
-   int index = __get_raid_index(flags);
+   int index = btrfs_bg_flags_to_raid_index(flags);
bool failed_cluster_refill = false;
bool failed_alloc = false;
bool use_cluster = true;
@@ -7569,7 +7546,8 @@ static noinline int find_free_extent(struct btrfs_fs_info 
*fs_info,
btrfs_put_block_group(block_group);
up_read(_info->groups_sem);
} else {
-   index = get_block_group_index(block_group);
+   index = btrfs_bg_flags_to_raid_index(
+   block_group->flags);
btrfs_lock_block_group(block_group, delalloc);
goto have_block_group;
}
@@ -7579,7 +7557,7 @@ static noinline int find_free_extent(struct btrfs_fs_info 
*fs_info,
}
 search:
have_caching_bg = false;
-   if (index == 0 || index == __get_raid_index(flags))
+   if (index == 0 || index == btrfs_bg_flags_to_raid_index(flags))
full_search = true;
down_read(_info->groups_sem);
list_for_each_entry(block_group, _info->block_groups[index],
@@ -7837,7 +7815,8 @@ static noinline int find_free_extent(struct btrfs_fs_info 
*fs_info,
 loop:
failed_cluster_refill = false;
failed_alloc = false;
-   BUG_ON(index != get_block_group_index(block_group));
+   BUG_ON(btrfs_bg_flags_to_raid_index(block_group->flags) !=
+  index);