Re: [PATCH 2/7] btrfs-progs: Merge btrfs_alloc_data_chunk into btrfs_alloc_chunk

[Su Yue]

On 02/02/2018 05:41 PM, Qu Wenruo wrote:



On 2018年02月02日 17:20, Su Yue wrote:



On 02/02/2018 04:19 PM, Qu Wenruo wrote:

We used to have two chunk allocators, btrfs_alloc_chunk() and
btrfs_alloc_data_chunk(), the former is the more generic one, while the
later is only used in mkfs and convert, to allocate SINGLE data chunk.

Although btrfs_alloc_data_chunk() has some special hacks to cooperate
with convert, it's quite simple to integrity it into the generic chunk
allocator.

So merge them into one btrfs_alloc_chunk(), with extra @convert
parameter and necessary comment, to make code less duplicated and less
thing to maintain.

Signed-off-by: Qu Wenruo 
---
   convert/main.c |   6 +-
   extent-tree.c  |   2 +-
   mkfs/main.c    |   8 +--
   volumes.c  | 219
++---

[snip]

diff --git a/volumes.c b/volumes.c
index 677d085de96c..9ee4650351c3 100644
--- a/volumes.c
+++ b/volumes.c
@@ -836,9 +836,23 @@ error:
   - 2 * sizeof(struct btrfs_chunk))    \
   / sizeof(struct btrfs_stripe) + 1)
   +/*
+ * Alloc a chunk, will insert dev extents, chunk item.
+ * NOTE: This function will not insert block group item nor mark newly
+ * allocated chunk available for later allocation.
+ * Block group item and free space update is handled by
btrfs_make_block_group()
+ *
+ * @start:    return value of allocated chunk start bytenr.
+ * @num_bytes:    return value of allocated chunk size
+ * @type:    chunk type (including both profile and type)
+ * @convert:    if the chunk is allocated for convert case.
+ * If @convert is true, chunk allocator will skip device extent
+ * search, but use *start and *num_bytes as chunk
start/num_bytes
+ * and devive offset, to build a 1:1 chunk mapping for convert.
+ */
   int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
     struct btrfs_fs_info *info, u64 *start,
-  u64 *num_bytes, u64 type)
+  u64 *num_bytes, u64 type, bool convert)
   {
   u64 dev_offset;
   struct btrfs_root *extent_root = info->extent_root;
@@ -868,10 +882,38 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle
*trans,
   struct btrfs_key key;
   u64 offset;
   -    if (list_empty(dev_list)) {
+    if (list_empty(dev_list))
   return -ENOSPC;
+
+    if (convert) {
+    /* For convert, profile must be SINGLE */
+    if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {

Maybe BTRFS_RAID_SINGLE?


Here we're using type and raid index is not introduced yet, so what
we're doing is correct.


OK.

Reviewed-by: Su Yue 


Thanks,
Qu



Thanks,
Su


+    error("convert only suports SINGLE profile");
+    return -EINVAL;
+    }
+    if (!IS_ALIGNED(*start, info->sectorsize)) {
+    error("chunk start not aligned, start=%llu sectorsize=%u",
+    *start, info->sectorsize);
+    return -EINVAL;
+    }
+    if (!IS_ALIGNED(*num_bytes, info->sectorsize)) {
+    error("chunk size not aligned, size=%llu sectorsize=%u",
+    *num_bytes, info->sectorsize);
+    return -EINVAL;
+    }
+    calc_size = *num_bytes;
+    offset = *start;
+    /*
+ * For convert, we use 1:1 chunk mapping specified by @start and
+ * @num_bytes, so there is no need to go through dev_extent
+ * searching.
+ */
+    goto alloc_chunk;
   }
   +    /*
+ * Chunk size calculation part.
+ */
   if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
   if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
   calc_size = SZ_8M;
@@ -942,6 +984,9 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle
*trans,
   percent_max =
div_factor(btrfs_super_total_bytes(info->super_copy), 1);
   max_chunk_size = min(percent_max, max_chunk_size);
   +    /*
+ * Reserve space from each device.
+ */
   again:
   if (chunk_bytes_by_type(type, calc_size, num_stripes,
sub_stripes) >
   max_chunk_size) {
@@ -972,7 +1017,8 @@ again:
   return ret;
   cur = cur->next;
   if (avail >= min_free) {
-    list_move_tail(>dev_list, _devs);
+    list_move_tail(>dev_list,
+   _devs);
   index++;
   if (type & BTRFS_BLOCK_GROUP_DUP)
   index++;
@@ -999,9 +1045,16 @@ again:
   }
   return -ENOSPC;
   }
-    ret = find_next_chunk(info, );
-    if (ret)
-    return ret;
+
+    /*
+ * Fill chunk mapping and chunk stripes
+ */
+alloc_chunk:
+    if (!convert) {
+    ret = find_next_chunk(info, );
+    if (ret)
+    return ret;
+    }
   key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
   key.type = BTRFS_CHUNK_ITEM_KEY;
   key.offset = offset;
@@ -1022,17 +1075,31 @@ again:
   index = 0;
   while(index < num_stripes) {
   struct btrfs_stripe 

Re: [PATCH 2/7] btrfs-progs: Merge btrfs_alloc_data_chunk into btrfs_alloc_chunk

[Qu Wenruo]

On 2018年02月02日 17:20, Su Yue wrote:
> 
> 
> On 02/02/2018 04:19 PM, Qu Wenruo wrote:
>> We used to have two chunk allocators, btrfs_alloc_chunk() and
>> btrfs_alloc_data_chunk(), the former is the more generic one, while the
>> later is only used in mkfs and convert, to allocate SINGLE data chunk.
>>
>> Although btrfs_alloc_data_chunk() has some special hacks to cooperate
>> with convert, it's quite simple to integrity it into the generic chunk
>> allocator.
>>
>> So merge them into one btrfs_alloc_chunk(), with extra @convert
>> parameter and necessary comment, to make code less duplicated and less
>> thing to maintain.
>>
>> Signed-off-by: Qu Wenruo 
>> ---
>>   convert/main.c |   6 +-
>>   extent-tree.c  |   2 +-
>>   mkfs/main.c    |   8 +--
>>   volumes.c  | 219
>> ++---
[snip]
>> diff --git a/volumes.c b/volumes.c
>> index 677d085de96c..9ee4650351c3 100644
>> --- a/volumes.c
>> +++ b/volumes.c
>> @@ -836,9 +836,23 @@ error:
>>   - 2 * sizeof(struct btrfs_chunk))    \
>>   / sizeof(struct btrfs_stripe) + 1)
>>   +/*
>> + * Alloc a chunk, will insert dev extents, chunk item.
>> + * NOTE: This function will not insert block group item nor mark newly
>> + * allocated chunk available for later allocation.
>> + * Block group item and free space update is handled by
>> btrfs_make_block_group()
>> + *
>> + * @start:    return value of allocated chunk start bytenr.
>> + * @num_bytes:    return value of allocated chunk size
>> + * @type:    chunk type (including both profile and type)
>> + * @convert:    if the chunk is allocated for convert case.
>> + * If @convert is true, chunk allocator will skip device extent
>> + * search, but use *start and *num_bytes as chunk
>> start/num_bytes
>> + * and devive offset, to build a 1:1 chunk mapping for convert.
>> + */
>>   int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>>     struct btrfs_fs_info *info, u64 *start,
>> -  u64 *num_bytes, u64 type)
>> +  u64 *num_bytes, u64 type, bool convert)
>>   {
>>   u64 dev_offset;
>>   struct btrfs_root *extent_root = info->extent_root;
>> @@ -868,10 +882,38 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle
>> *trans,
>>   struct btrfs_key key;
>>   u64 offset;
>>   -    if (list_empty(dev_list)) {
>> +    if (list_empty(dev_list))
>>   return -ENOSPC;
>> +
>> +    if (convert) {
>> +    /* For convert, profile must be SINGLE */
>> +    if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
> Maybe BTRFS_RAID_SINGLE?

Here we're using type and raid index is not introduced yet, so what
we're doing is correct.

Thanks,
Qu

> 
> Thanks,
> Su
> 
>> +    error("convert only suports SINGLE profile");
>> +    return -EINVAL;
>> +    }
>> +    if (!IS_ALIGNED(*start, info->sectorsize)) {
>> +    error("chunk start not aligned, start=%llu sectorsize=%u",
>> +    *start, info->sectorsize);
>> +    return -EINVAL;
>> +    }
>> +    if (!IS_ALIGNED(*num_bytes, info->sectorsize)) {
>> +    error("chunk size not aligned, size=%llu sectorsize=%u",
>> +    *num_bytes, info->sectorsize);
>> +    return -EINVAL;
>> +    }
>> +    calc_size = *num_bytes;
>> +    offset = *start;
>> +    /*
>> + * For convert, we use 1:1 chunk mapping specified by @start and
>> + * @num_bytes, so there is no need to go through dev_extent
>> + * searching.
>> + */
>> +    goto alloc_chunk;
>>   }
>>   +    /*
>> + * Chunk size calculation part.
>> + */
>>   if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
>>   if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
>>   calc_size = SZ_8M;
>> @@ -942,6 +984,9 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle
>> *trans,
>>   percent_max =
>> div_factor(btrfs_super_total_bytes(info->super_copy), 1);
>>   max_chunk_size = min(percent_max, max_chunk_size);
>>   +    /*
>> + * Reserve space from each device.
>> + */
>>   again:
>>   if (chunk_bytes_by_type(type, calc_size, num_stripes,
>> sub_stripes) >
>>   max_chunk_size) {
>> @@ -972,7 +1017,8 @@ again:
>>   return ret;
>>   cur = cur->next;
>>   if (avail >= min_free) {
>> -    list_move_tail(>dev_list, _devs);
>> +    list_move_tail(>dev_list,
>> +   _devs);
>>   index++;
>>   if (type & BTRFS_BLOCK_GROUP_DUP)
>>   index++;
>> @@ -999,9 +1045,16 @@ again:
>>   }
>>   return -ENOSPC;
>>   }
>> -    ret = find_next_chunk(info, );
>> -    if (ret)
>> -    return ret;
>> +
>> +    /*
>> + * Fill chunk mapping and chunk stripes
>> + */
>> +alloc_chunk:
>> +    if (!convert) {
>> +    ret = find_next_chunk(info, );
>> +    if (ret)
>> +    return 

Re: [PATCH 2/7] btrfs-progs: Merge btrfs_alloc_data_chunk into btrfs_alloc_chunk

[Su Yue]

On 02/02/2018 04:19 PM, Qu Wenruo wrote:

We used to have two chunk allocators, btrfs_alloc_chunk() and
btrfs_alloc_data_chunk(), the former is the more generic one, while the
later is only used in mkfs and convert, to allocate SINGLE data chunk.

Although btrfs_alloc_data_chunk() has some special hacks to cooperate
with convert, it's quite simple to integrity it into the generic chunk
allocator.

So merge them into one btrfs_alloc_chunk(), with extra @convert
parameter and necessary comment, to make code less duplicated and less
thing to maintain.

Signed-off-by: Qu Wenruo 
---
  convert/main.c |   6 +-
  extent-tree.c  |   2 +-
  mkfs/main.c|   8 +--
  volumes.c  | 219 ++---
  volumes.h  |   5 +-
  5 files changed, 94 insertions(+), 146 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index b3ea31d7af43..b2444bb2ff21 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -910,9 +910,9 @@ static int make_convert_data_block_groups(struct 
btrfs_trans_handle *trans,
  
  			len = min(max_chunk_size,

  cache->start + cache->size - cur);
-   ret = btrfs_alloc_data_chunk(trans, fs_info,
-   _backup, len,
-   BTRFS_BLOCK_GROUP_DATA, 1);
+   ret = btrfs_alloc_chunk(trans, fs_info,
+   _backup, ,
+   BTRFS_BLOCK_GROUP_DATA, true);
if (ret < 0)
break;
ret = btrfs_make_block_group(trans, fs_info, 0,
diff --git a/extent-tree.c b/extent-tree.c
index e2ae74a7fe66..b085ab0352b3 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1906,7 +1906,7 @@ static int do_chunk_alloc(struct btrfs_trans_handle 
*trans,
return 0;
  
  	ret = btrfs_alloc_chunk(trans, fs_info, , _bytes,

-   space_info->flags);
+   space_info->flags, false);
if (ret == -ENOSPC) {
space_info->full = 1;
return 0;
diff --git a/mkfs/main.c b/mkfs/main.c
index ea65c6d897b2..358395ca0250 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -82,7 +82,7 @@ static int create_metadata_block_groups(struct btrfs_root 
*root, int mixed,
ret = btrfs_alloc_chunk(trans, fs_info,
_start, _size,
BTRFS_BLOCK_GROUP_METADATA |
-   BTRFS_BLOCK_GROUP_DATA);
+   BTRFS_BLOCK_GROUP_DATA, false);
if (ret == -ENOSPC) {
error("no space to allocate data/metadata chunk");
goto err;
@@ -99,7 +99,7 @@ static int create_metadata_block_groups(struct btrfs_root 
*root, int mixed,
} else {
ret = btrfs_alloc_chunk(trans, fs_info,
_start, _size,
-   BTRFS_BLOCK_GROUP_METADATA);
+   BTRFS_BLOCK_GROUP_METADATA, false);
if (ret == -ENOSPC) {
error("no space to allocate metadata chunk");
goto err;
@@ -133,7 +133,7 @@ static int create_data_block_groups(struct 
btrfs_trans_handle *trans,
if (!mixed) {
ret = btrfs_alloc_chunk(trans, fs_info,
_start, _size,
-   BTRFS_BLOCK_GROUP_DATA);
+   BTRFS_BLOCK_GROUP_DATA, false);
if (ret == -ENOSPC) {
error("no space to allocate data chunk");
goto err;
@@ -241,7 +241,7 @@ static int create_one_raid_group(struct btrfs_trans_handle 
*trans,
int ret;
  
  	ret = btrfs_alloc_chunk(trans, fs_info,

-   _start, _size, type);
+   _start, _size, type, false);
if (ret == -ENOSPC) {
error("not enough free space to allocate chunk");
exit(1);
diff --git a/volumes.c b/volumes.c
index 677d085de96c..9ee4650351c3 100644
--- a/volumes.c
+++ b/volumes.c
@@ -836,9 +836,23 @@ error:
- 2 * sizeof(struct btrfs_chunk))   \
/ sizeof(struct btrfs_stripe) + 1)
  
+/*

+ * Alloc a chunk, will insert dev extents, chunk item.
+ * NOTE: This function will not insert block group item nor mark newly
+ * allocated chunk available for later allocation.
+ * Block group item and free space update is handled by 
btrfs_make_block_group()
+ *
+ * @start: return value of allocated chunk start bytenr.
+ * @num_bytes: return value of allocated chunk size
+ * @type:  chunk type (including both profile and type)
+ * 

[PATCH 2/7] btrfs-progs: Merge btrfs_alloc_data_chunk into btrfs_alloc_chunk

[Qu Wenruo]

We used to have two chunk allocators, btrfs_alloc_chunk() and
btrfs_alloc_data_chunk(), the former is the more generic one, while the
later is only used in mkfs and convert, to allocate SINGLE data chunk.

Although btrfs_alloc_data_chunk() has some special hacks to cooperate
with convert, it's quite simple to integrity it into the generic chunk
allocator.

So merge them into one btrfs_alloc_chunk(), with extra @convert
parameter and necessary comment, to make code less duplicated and less
thing to maintain.

Signed-off-by: Qu Wenruo 
---
 convert/main.c |   6 +-
 extent-tree.c  |   2 +-
 mkfs/main.c|   8 +--
 volumes.c  | 219 ++---
 volumes.h  |   5 +-
 5 files changed, 94 insertions(+), 146 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index b3ea31d7af43..b2444bb2ff21 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -910,9 +910,9 @@ static int make_convert_data_block_groups(struct 
btrfs_trans_handle *trans,
 
len = min(max_chunk_size,
  cache->start + cache->size - cur);
-   ret = btrfs_alloc_data_chunk(trans, fs_info,
-   _backup, len,
-   BTRFS_BLOCK_GROUP_DATA, 1);
+   ret = btrfs_alloc_chunk(trans, fs_info,
+   _backup, ,
+   BTRFS_BLOCK_GROUP_DATA, true);
if (ret < 0)
break;
ret = btrfs_make_block_group(trans, fs_info, 0,
diff --git a/extent-tree.c b/extent-tree.c
index e2ae74a7fe66..b085ab0352b3 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1906,7 +1906,7 @@ static int do_chunk_alloc(struct btrfs_trans_handle 
*trans,
return 0;
 
ret = btrfs_alloc_chunk(trans, fs_info, , _bytes,
-   space_info->flags);
+   space_info->flags, false);
if (ret == -ENOSPC) {
space_info->full = 1;
return 0;
diff --git a/mkfs/main.c b/mkfs/main.c
index ea65c6d897b2..358395ca0250 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -82,7 +82,7 @@ static int create_metadata_block_groups(struct btrfs_root 
*root, int mixed,
ret = btrfs_alloc_chunk(trans, fs_info,
_start, _size,
BTRFS_BLOCK_GROUP_METADATA |
-   BTRFS_BLOCK_GROUP_DATA);
+   BTRFS_BLOCK_GROUP_DATA, false);
if (ret == -ENOSPC) {
error("no space to allocate data/metadata chunk");
goto err;
@@ -99,7 +99,7 @@ static int create_metadata_block_groups(struct btrfs_root 
*root, int mixed,
} else {
ret = btrfs_alloc_chunk(trans, fs_info,
_start, _size,
-   BTRFS_BLOCK_GROUP_METADATA);
+   BTRFS_BLOCK_GROUP_METADATA, false);
if (ret == -ENOSPC) {
error("no space to allocate metadata chunk");
goto err;
@@ -133,7 +133,7 @@ static int create_data_block_groups(struct 
btrfs_trans_handle *trans,
if (!mixed) {
ret = btrfs_alloc_chunk(trans, fs_info,
_start, _size,
-   BTRFS_BLOCK_GROUP_DATA);
+   BTRFS_BLOCK_GROUP_DATA, false);
if (ret == -ENOSPC) {
error("no space to allocate data chunk");
goto err;
@@ -241,7 +241,7 @@ static int create_one_raid_group(struct btrfs_trans_handle 
*trans,
int ret;
 
ret = btrfs_alloc_chunk(trans, fs_info,
-   _start, _size, type);
+   _start, _size, type, false);
if (ret == -ENOSPC) {
error("not enough free space to allocate chunk");
exit(1);
diff --git a/volumes.c b/volumes.c
index 677d085de96c..9ee4650351c3 100644
--- a/volumes.c
+++ b/volumes.c
@@ -836,9 +836,23 @@ error:
- 2 * sizeof(struct btrfs_chunk))   \
/ sizeof(struct btrfs_stripe) + 1)
 
+/*
+ * Alloc a chunk, will insert dev extents, chunk item.
+ * NOTE: This function will not insert block group item nor mark newly
+ * allocated chunk available for later allocation.
+ * Block group item and free space update is handled by 
btrfs_make_block_group()
+ *
+ * @start: return value of allocated chunk start bytenr.
+ * @num_bytes: return value of allocated chunk size
+ * @type:  chunk type (including both profile and type)
+ * @convert:   if the chunk is