| Message ID | 20180105081212.27175-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <wqu@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) ..." > + 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <wqu@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018年01月05日 17:36, 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 <wqu@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. That's completely fine, since @private_devs is empty anyway. Reinitiliazing it won't cause any problem. On the other hand, if you don't init @private_devs, you will easily hit BUG_ON(!list_empty(private_devs)); But since the unconditionally re-init seems a little annoying, I could move it do convert branch, or just enhance the later BUG_ON() to skip the check for convert == true case. >> + 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) ..." Well, wrong comment. Here I mean SINGLE profile, so the code is wrong but with wrong comment. > >> + 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. Yep, I forgot that. Will update them soon. Thanks, Qu >> + 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); >> > > >
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); + if (list_empty(dev_list)) return -ENOSPC; - } + if (convert) { + /* For convert, profile must be BTRFS_BLOCK_GROUP_DATA */ + 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); + return -EINVAL; + } + if (!IS_ALIGNED(*num_bytes, info->sectorsize)) { + error("chunk size not aligned, size=%llu sectorsize=%u\n", + *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; @@ -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);
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 <wqu@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(-)