On 2018年02月09日 16:17, Nikolay Borisov wrote:
>
>
> On 9.02.2018 09:44, Qu Wenruo wrote:
>> Kernel uses a delayed chunk allocation behavior for metadata chunks
>>
>> KERNEL:
>> btrfs_alloc_chunk()
>> |- __btrfs_alloc_chunk(): Only allocate chunk mapping
>> |- btrfs_make_block_group(): Add corresponding bg to fs_info->new_bgs
>>
>> Then at transaction commit time, it finishes the remaining work:
>> btrfs_start_dirty_block_groups():
>> |- btrfs_create_pending_block_groups()
>> |- btrfs_insert_item(): To insert block group item
>> |- btrfs_finish_chunk_alloc(): Insert chunk items/dev extents
>>
>> Although btrfs-progs btrfs_alloc_chunk() does all the work in one
>> function, it can still benefit from function refactor like:
>> btrfs-progs:
>> btrfs_alloc_chunk(): Wrapper for both normal and convert chunks
>> |- __btrfs_alloc_chunk(): Only alloc chunk mapping
>> | |- btrfs_make_block_group(): <<Insert bg items into extent tree>>
>> |- btrfs_finish_chunk_alloc(): Insert chunk items/dev extents
>>
>> With such refactor, the following functions can share most of its code
>> with kernel now:
>> __btrfs_alloc_chunk()
>> btrfs_finish_chunk_alloc()
>> btrfs_alloc_dev_extent()
>>
>> Signed-off-by: Qu Wenruo <w...@suse.com>
>> ---
>> volumes.c | 421
>> ++++++++++++++++++++++++++++++++++++++------------------------
>> 1 file changed, 260 insertions(+), 161 deletions(-)
>>
>> diff --git a/volumes.c b/volumes.c
>> index cff54c612872..e89520326314 100644
>> --- a/volumes.c
>> +++ b/volumes.c
>> @@ -523,55 +523,40 @@ static int find_free_dev_extent(struct btrfs_device
>> *device, u64 num_bytes,
>> return find_free_dev_extent_start(device, num_bytes, 0, start, len);
>> }
>>
>> -static int btrfs_insert_dev_extents(struct btrfs_trans_handle *trans,
>> - struct btrfs_fs_info *fs_info,
>> - struct map_lookup *map, u64 stripe_size)
>> +static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
>> + struct btrfs_device *device,
>> + u64 chunk_offset, u64 physical,
>> + u64 stripe_size)
>> {
>> - int ret = 0;
>> - struct btrfs_path path;
>> + int ret;
>> + struct btrfs_path *path;
>> + struct btrfs_fs_info *fs_info = device->fs_info;
>> struct btrfs_root *root = fs_info->dev_root;
>> struct btrfs_dev_extent *extent;
>> struct extent_buffer *leaf;
>> struct btrfs_key key;
>> - int i;
>>
>> - btrfs_init_path(&path);
>> -
>> - for (i = 0; i < map->num_stripes; i++) {
>> - struct btrfs_device *device = map->stripes[i].dev;
>> -
>> - key.objectid = device->devid;
>> - key.offset = map->stripes[i].physical;
>> - key.type = BTRFS_DEV_EXTENT_KEY;
>> + path = btrfs_alloc_path();
>> + if (!path)
>> + return -ENOMEM;
>>
>> - ret = btrfs_insert_empty_item(trans, root, &path, &key,
>> - sizeof(*extent));
>> - if (ret < 0)
>> - goto out;
>> - leaf = path.nodes[0];
>> - extent = btrfs_item_ptr(leaf, path.slots[0],
>> - struct btrfs_dev_extent);
>> - btrfs_set_dev_extent_chunk_tree(leaf, extent,
>> + key.objectid = device->devid;
>> + key.offset = physical;
>> + key.type = BTRFS_DEV_EXTENT_KEY;
>> + ret = btrfs_insert_empty_item(trans, root, path, &key, sizeof(*extent));
>> + if (ret)
>> + goto out;
>> + leaf = path->nodes[0];
>> + extent = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_dev_extent);
>> + btrfs_set_dev_extent_chunk_tree(leaf, extent,
>> BTRFS_CHUNK_TREE_OBJECTID);
>> - btrfs_set_dev_extent_chunk_objectid(leaf, extent,
>> - BTRFS_FIRST_CHUNK_TREE_OBJECTID);
>> - btrfs_set_dev_extent_chunk_offset(leaf, extent, map->ce.start);
>> -
>> - write_extent_buffer(leaf, fs_info->chunk_tree_uuid,
>> - (unsigned long)btrfs_dev_extent_chunk_tree_uuid(extent),
>> - BTRFS_UUID_SIZE);
>> -
>> - btrfs_set_dev_extent_length(leaf, extent, stripe_size);
>> - btrfs_mark_buffer_dirty(leaf);
>> - btrfs_release_path(&path);
>> -
>> - device->bytes_used += stripe_size;
>> - ret = btrfs_update_device(trans, device);
>> - if (ret < 0)
>> - goto out;
>> - }
>> + btrfs_set_dev_extent_chunk_objectid(leaf, extent,
>> + BTRFS_FIRST_CHUNK_TREE_OBJECTID);
>> + btrfs_set_dev_extent_chunk_offset(leaf, extent, chunk_offset);
>> + btrfs_set_dev_extent_length(leaf, extent, stripe_size);
>> + btrfs_mark_buffer_dirty(leaf);
>> out:
>> - btrfs_release_path(&path);
>> + btrfs_free_path(path);
>> return ret;
>> }
>>
>> @@ -813,28 +798,28 @@ static int btrfs_cmp_device_info(const void *a, const
>> void *b)
>> / sizeof(struct btrfs_stripe) + 1)
>>
>> /*
>> - * Alloc a chunk, will insert dev extents, chunk item, and insert new
>> - * block group and update space info (so that extent allocator can use
>> - * newly allocated chunk).
>> + * Alloc a chunk mapping.
>> + * Will do chunk size calculation and free dev extent search, and insert
>> + * chunk mapping into chunk mapping tree.
>> + *
>> + * NOTE: This function doesn't handle any chunk item/dev extent insert.
>> + * chunk item/dev extent insert is handled by later
>> btrfs_finish_chunk_alloc().
>> + * And for convert chunk (1:1 mapped, more flex chunk location), it's
>> + * handled by __btrfs_alloc_convert_chunk().
>> + *
>> + * Qu: Block group item is still inserted in this function by
>> + * btrfs_make_block_group(), this still differs from kernel.
>> *
>
> If it still differs does that mean this function should undergo further
> refactoring to unify it with the kernel or it means it will never be the
> same as the kernel's code? I thought the idea of unifying the
> userspace/kernel code is to have only 1 set of functions which work both
> in kernel and userspace?One (temporary) solution here is, make btrfs_make_block_group() virtual. So btrfs-progs and kernel has their own version of btrfs_make_block_group(). And the unification is a progressive work, leaving btrfs_make_block_group() different is acceptable at this point, since the objective of this patchset is just to unify chunk allocator. BTW, the real unification needs both kernel and progs side to be modified, so this patchset is still "preparation". Thanks, Qu
signature.asc
Description: OpenPGP digital signature
