[v4,02/11] btrfs-progs: block-group: Refactor how we read one block group item

Commit Message

Qu Wenruo May 5, 2020, 12:02 a.m. UTC

Structure btrfs_block_group has the following members which are
currently read from on-disk block group item and key:
- Length
  From item key.
- Used
- Flags
  From block group item.

However for incoming skinny block group tree, we are going to read those
members from different sources.

This patch will refactor such read by:
- Refactor length/used/flags initialization into one function
  The new function, fill_one_block_group() will handle the
  initialization of such members.

- Use btrfs_block_group::length to replace key::offset
  Since skinny block group item would have a different meaning for its
  key offset.

Signed-off-by: Qu Wenruo <[e-mail]>
---
 extent-tree.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

Comments

Johannes Thumshirn May 6, 2020, 5:27 p.m. UTC | #1

On 05/05/2020 02:02, Qu Wenruo wrote:
> - Use btrfs_block_group::length  to replace key::offset
>    Since skinny block group item would have a different meaning for its
>    key offset.

Nope, you still use key->offset for cache->length

> +
> +	cache->start = key->objectid;
> +	cache->length = key->offset;

Qu Wenruo May 6, 2020, 10:52 p.m. UTC | #2

On 2020/5/7 上午1:27, Johannes Thumshirn wrote:
> On 05/05/2020 02:02, Qu Wenruo wrote:
>> - Use btrfs_block_group::length  to replace key::offset
>>    Since skinny block group item would have a different meaning for its
>>    key offset.
> 
> Nope, you still use key->offset for cache->length

That's no problem for regular block group item, as in that case
key->offset is block group length.

It looks like the sentence is not clear enough, what I mean is, after
read_block_group_item(), there shouldn't be any key->offset user, but
use block_group->length instead.

Thanks,
Qu

> 
>> +
>> +	cache->start = key->objectid;
>> +	cache->length = key->offset;
>

Johannes Thumshirn May 7, 2020, 7:41 a.m. UTC | #3

On 07/05/2020 00:52, Qu Wenruo wrote:
> It looks like the sentence is not clear enough, what I mean is, after
> read_block_group_item(), there shouldn't be any key->offset user, but
> use block_group->length instead.

Ah ok, that makes more sense then.

Thanks,
	Johannes

Patch

diff --git a/extent-tree.c b/extent-tree.c
index bd7dbf551876..5fc4308336dd 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -172,6 +172,7 @@  static int btrfs_add_block_group_cache(struct btrfs_fs_info *info,
 	struct rb_node *parent = NULL;
 	struct btrfs_block_group *cache;
 
+	ASSERT(block_group->length != 0);
 	p = &info->block_group_cache_tree.rb_node;
 
 	while (*p) {
@@ -2630,6 +2631,27 @@  error:
 	return ret;
 }
 
+static int read_block_group_item(struct btrfs_block_group *cache,
+				 struct btrfs_path *path,
+				 const struct btrfs_key *key)
+{
+	struct extent_buffer *leaf = path->nodes[0];
+	struct btrfs_block_group_item bgi;
+	int slot = path->slots[0];
+
+	ASSERT(key->type == BTRFS_BLOCK_GROUP_ITEM_KEY);
+
+	cache->start = key->objectid;
+	cache->length = key->offset;
+
+	read_extent_buffer(leaf, &bgi, btrfs_item_ptr_offset(leaf, slot),
+			   sizeof(bgi));
+	cache->used = btrfs_stack_block_group_used(&bgi);
+	cache->flags = btrfs_stack_block_group_flags(&bgi);
+
+	return 0;
+}
+
 /*
  * Read out one BLOCK_GROUP_ITEM and insert it into block group cache.
  *
@@ -2642,7 +2664,6 @@  static int read_one_block_group(struct btrfs_fs_info *fs_info,
 	struct extent_buffer *leaf = path->nodes[0];
 	struct btrfs_space_info *space_info;
 	struct btrfs_block_group *cache;
-	struct btrfs_block_group_item bgi;
 	struct btrfs_key key;
 	int slot = path->slots[0];
 	int ret;
@@ -2660,14 +2681,11 @@  static int read_one_block_group(struct btrfs_fs_info *fs_info,
 	cache = kzalloc(sizeof(*cache), GFP_NOFS);
 	if (!cache)
 		return -ENOMEM;
-	read_extent_buffer(leaf, &bgi, btrfs_item_ptr_offset(leaf, slot),
-			   sizeof(bgi));
-	cache->start = key.objectid;
-	cache->length = key.offset;
-	cache->cached = 0;
-	cache->pinned = 0;
-	cache->flags = btrfs_stack_block_group_flags(&bgi);
-	cache->used = btrfs_stack_block_group_used(&bgi);
+	ret = read_block_group_item(cache, path, &key);
+	if (ret < 0) {
+		free(cache);
+		return ret;
+	}
 	INIT_LIST_HEAD(&cache->dirty_list);
 
 	set_avail_alloc_bits(fs_info, cache->flags);