On 01/05/2018 05:36 PM, Su Yue wrote:
On 01/05/2018 04:12 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 <w...@suse.com> --- convert/main.c | 6 +- extent-tree.c | 2 +- mkfs/main.c | 14 ++--volumes.c | 220 ++++++++++++++++++++++-----------------------------------volumes.h | 5 +- 5 files changed, 98 insertions(+), 149 deletions(-) diff --git a/convert/main.c b/convert/main.c index 4a510a786394..8ee858fb2d05 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, - &cur_backup, len, - BTRFS_BLOCK_GROUP_DATA, 1); + ret = btrfs_alloc_chunk(trans, fs_info, + &cur_backup, &len, + 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 db24da3a3a8c..4231be11bd53 100644 --- a/extent-tree.c +++ b/extent-tree.c@@ -1907,7 +1907,7 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,return 0; ret = btrfs_alloc_chunk(trans, fs_info, &start, &num_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 938025bfd32e..f8e27a7ec8b8 100644 --- a/mkfs/main.c +++ b/mkfs/main.c@@ -92,7 +92,7 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,ret = btrfs_alloc_chunk(trans, fs_info, &chunk_start, &chunk_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;@@ -109,7 +109,7 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,} else { ret = btrfs_alloc_chunk(trans, fs_info, &chunk_start, &chunk_size, - BTRFS_BLOCK_GROUP_METADATA); + BTRFS_BLOCK_GROUP_METADATA, false); if (ret == -ENOSPC) { error("no space to allocate metadata chunk"); goto err;@@ -143,7 +143,7 @@ static int create_data_block_groups(struct btrfs_trans_handle *trans,if (!mixed) { ret = btrfs_alloc_chunk(trans, fs_info, &chunk_start, &chunk_size, - BTRFS_BLOCK_GROUP_DATA); + BTRFS_BLOCK_GROUP_DATA, false); if (ret == -ENOSPC) { error("no space to allocate data chunk"); goto err;@@ -251,7 +251,7 @@ static int create_one_raid_group(struct btrfs_trans_handle *trans,int ret; ret = btrfs_alloc_chunk(trans, fs_info, - &chunk_start, &chunk_size, type); + &chunk_start, &chunk_size, type, false); if (ret == -ENOSPC) { error("not enough free space to allocate chunk"); exit(1);@@ -1003,7 +1003,7 @@ static int create_chunks(struct btrfs_trans_handle *trans,for (i = 0; i < num_of_meta_chunks; i++) { ret = btrfs_alloc_chunk(trans, fs_info, - &chunk_start, &chunk_size, meta_type); + &chunk_start, &chunk_size, meta_type, false); if (ret) return ret; ret = btrfs_make_block_group(trans, fs_info, 0,@@ -1019,8 +1019,8 @@ static int create_chunks(struct btrfs_trans_handle *trans,if (size_of_data < minimum_data_chunk_size) size_of_data = minimum_data_chunk_size; - ret = btrfs_alloc_data_chunk(trans, fs_info, - &chunk_start, size_of_data, data_type, 0); + ret = btrfs_alloc_chunk(trans, fs_info, + &chunk_start, &size_of_data, data_type, false); if (ret) return ret; ret = btrfs_make_block_group(trans, fs_info, 0, diff --git a/volumes.c b/volumes.c index fa3c6de023f9..89c2f952f5b3 100644 --- a/volumes.c +++ b/volumes.c @@ -844,9 +844,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;@@ -876,10 +890,39 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,struct btrfs_key key; u64 offset; - if (list_empty(dev_list)) { + INIT_LIST_HEAD(&private_devs);private_devs is initiated twice in the function. It seems that you forgot to delete latter one.+ if (list_empty(dev_list)) return -ENOSPC; - } + if (convert) { + /* For convert, profile must be BTRFS_BLOCK_GROUP_DATA */I wonder why not also check type like: "if (type & BTRFS_BLOCK_GROUP_DATA) ..."
Sorry, Should be "if (type & BTRFS_BLOCK_GROUP_DATA == 0) ..." Thanks, Su
+ if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) { + error("convert only suports SINGLE profile"); + return -EINVAL; + } + if (!IS_ALIGNED(*start, info->sectorsize)) { + error("chunk start not aligned, start=%llu sectorsize=%u\n", + *start, info->sectorsize);error() breaks line itself. '\n' is unnecessary.+ return -EINVAL; + } + if (!IS_ALIGNED(*num_bytes, info->sectorsize)) { + error("chunk size not aligned, size=%llu sectorsize=%u\n", + *num_bytes, info->sectorsize);And line break here. Other changes are nice to me. Thanks, Su+ 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;@@ -950,6 +993,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) { @@ -980,7 +1026,8 @@ again: return ret; cur = cur->next; if (avail >= min_free) { - list_move_tail(&device->dev_list, &private_devs); + list_move_tail(&device->dev_list, + &private_devs); index++; if (type & BTRFS_BLOCK_GROUP_DUP) index++; @@ -1007,9 +1054,16 @@ again: } return -ENOSPC; } - ret = find_next_chunk(info, &offset); - if (ret) - return ret; + + /* + * Fill chunk mapping and chunk stripes + */ +alloc_chunk: + if (!convert) { + ret = find_next_chunk(info, &offset); + if (ret) + return ret; + } key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID; key.type = BTRFS_CHUNK_ITEM_KEY; key.offset = offset; @@ -1030,17 +1084,31 @@ again: index = 0; while(index < num_stripes) { struct btrfs_stripe *stripe; - BUG_ON(list_empty(&private_devs)); - cur = private_devs.next; - device = list_entry(cur, struct btrfs_device, dev_list); - /* loop over this device again if we're doing a dup group */ - if (!(type & BTRFS_BLOCK_GROUP_DUP) || - (index == num_stripes - 1)) - list_move_tail(&device->dev_list, dev_list); + if (!convert) { + if (list_empty(&private_devs)) + return -ENODEV; + cur = private_devs.next; + device = list_entry(cur, struct btrfs_device, dev_list); + if (!(type & BTRFS_BLOCK_GROUP_DUP) || + (index == num_stripes - 1)) { + /* + * loop over this device again if we're doing a + * dup group + */ + list_move_tail(&device->dev_list, dev_list); + } + } else { + /* Only SINGLE is accepted in convert case */ + BUG_ON(num_stripes > 1); + device = list_entry(dev_list->next, struct btrfs_device, + dev_list); + key.offset = *start; + dev_offset = *start; + } ret = btrfs_alloc_dev_extent(trans, device, key.offset, - calc_size, &dev_offset, 0); + calc_size, &dev_offset, convert); if (ret < 0) goto out_chunk_map; @@ -1077,6 +1145,9 @@ again: map->num_stripes = num_stripes; map->sub_stripes = sub_stripes; + /* + * Insert chunk item and chunk mapping. + */ ret = btrfs_insert_item(trans, chunk_root, &key, chunk, btrfs_chunk_item_size(num_stripes)); BUG_ON(ret); @@ -1106,125 +1177,6 @@ out_chunk: return ret; } -/* - * Alloc a DATA chunk with SINGLE profile. - * - * If 'convert' is set, it will alloc a chunk with 1:1 mapping - * (btrfs logical bytenr == on-disk bytenr) - * For that case, caller must make sure the chunk and dev_extent are not - * occupied. - */ -int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans, - struct btrfs_fs_info *info, u64 *start, - u64 num_bytes, u64 type, int convert) -{ - u64 dev_offset; - struct btrfs_root *extent_root = info->extent_root; - struct btrfs_root *chunk_root = info->chunk_root; - struct btrfs_stripe *stripes; - struct btrfs_device *device = NULL; - struct btrfs_chunk *chunk; - struct list_head *dev_list = &info->fs_devices->devices; - struct list_head *cur; - struct map_lookup *map; - u64 calc_size = SZ_8M; - int num_stripes = 1; - int sub_stripes = 0; - int ret; - int index; - int stripe_len = BTRFS_STRIPE_LEN; - struct btrfs_key key; - - key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID; - key.type = BTRFS_CHUNK_ITEM_KEY; - if (convert) { - if (*start != round_down(*start, info->sectorsize)) { - error("DATA chunk start not sectorsize aligned: %llu", - (unsigned long long)*start); - return -EINVAL; - } - key.offset = *start; - dev_offset = *start; - } else { - u64 tmp; - - ret = find_next_chunk(info, &tmp); - key.offset = tmp; - if (ret) - return ret; - } - - chunk = kmalloc(btrfs_chunk_item_size(num_stripes), GFP_NOFS); - if (!chunk) - return -ENOMEM; - - map = kmalloc(btrfs_map_lookup_size(num_stripes), GFP_NOFS); - if (!map) { - kfree(chunk); - return -ENOMEM; - } - - stripes = &chunk->stripe; - calc_size = num_bytes; - - index = 0; - cur = dev_list->next; - device = list_entry(cur, struct btrfs_device, dev_list); - - while (index < num_stripes) { - struct btrfs_stripe *stripe; - - ret = btrfs_alloc_dev_extent(trans, device, key.offset, - calc_size, &dev_offset, convert); - BUG_ON(ret); - - device->bytes_used += calc_size; - ret = btrfs_update_device(trans, device); - BUG_ON(ret); - - map->stripes[index].dev = device; - map->stripes[index].physical = dev_offset; - stripe = stripes + index; - btrfs_set_stack_stripe_devid(stripe, device->devid); - btrfs_set_stack_stripe_offset(stripe, dev_offset); - memcpy(stripe->dev_uuid, device->uuid, BTRFS_UUID_SIZE); - index++; - } - - /* key was set above */ - btrfs_set_stack_chunk_length(chunk, num_bytes); - btrfs_set_stack_chunk_owner(chunk, extent_root->root_key.objectid); - btrfs_set_stack_chunk_stripe_len(chunk, stripe_len); - btrfs_set_stack_chunk_type(chunk, type); - btrfs_set_stack_chunk_num_stripes(chunk, num_stripes); - btrfs_set_stack_chunk_io_align(chunk, stripe_len); - btrfs_set_stack_chunk_io_width(chunk, stripe_len); - btrfs_set_stack_chunk_sector_size(chunk, info->sectorsize); - btrfs_set_stack_chunk_sub_stripes(chunk, sub_stripes); - map->sector_size = info->sectorsize; - map->stripe_len = stripe_len; - map->io_align = stripe_len; - map->io_width = stripe_len; - map->type = type; - map->num_stripes = num_stripes; - map->sub_stripes = sub_stripes; - - ret = btrfs_insert_item(trans, chunk_root, &key, chunk, - btrfs_chunk_item_size(num_stripes)); - BUG_ON(ret); - if (!convert) - *start = key.offset; - - map->ce.start = key.offset; - map->ce.size = num_bytes; - - ret = insert_cache_extent(&info->mapping_tree.cache_tree, &map->ce); - BUG_ON(ret); - - kfree(chunk); - return ret; -} -int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len){ struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; diff --git a/volumes.h b/volumes.h index 11572e78c04f..7bbdf615d31a 100644 --- a/volumes.h +++ b/volumes.h@@ -208,10 +208,7 @@ int btrfs_read_sys_array(struct btrfs_fs_info *fs_info);int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info); int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, u64 *start, - u64 *num_bytes, u64 type); -int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans, - struct btrfs_fs_info *fs_info, u64 *start, - u64 num_bytes, u64 type, int convert); + u64 *num_bytes, u64 type, bool convert); int btrfs_open_devices(struct btrfs_fs_devices *fs_devices, int flags); int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
-- 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
