Skip to contentSkip to conversation

Chunk allocator unification

PATCH Superseded Qu Wenruo 4 participants 17 messages Feb 2, 2018

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

cover Chunk allocator unification

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/libbtrfs_prepare

This patchset unified a large part of chunk allocator (free device extent search) between kernel and btrfs-progs.

Although there are still differences in the following areas, the goal of extract common code into libbtrfs is in the reach.

1) Member differences
Btrfs-progs lacks quite some members of kernel, so something like fs_devices->total_rw_bytes still makes difference.

2) @convert parameter for mkfs/convert
It should be able to extract them into a simpler function.

3) delayed bg creations vs direct bg creation
Kernel delays block group creation while btrfs-progs directly allocates block group.
This makes kernel __btrfs_alloc_chunk() more or less chunk mapping allocation only, while btrfs-progs handles extra chunk, dev extent and block group item creation.

This needs extra function refactor before we really extra code into libbtrfs.

Despite the preparation for later libbtrfs, this rework still has some benefit:

1) No minimal stripe/chunk size
Unlike the intermediate minimal stripe/chunk size in btrfs-progs, kernel doesn't really implement a minimal stripe/chunk size.

So now we don't have such limit in btrfs-progs side now. Although the minimal device size calculation doesn't make any harm so it's still kept as is.

2) Slightly less code for chunk allocator
Just about -60 lines in the last patch.

Qu Wenruo (7):
btrfs-progs: Refactor parameter of BTRFS_MAX_DEVS() from root to
fs_info
btrfs-progs: Merge btrfs_alloc_data_chunk into btrfs_alloc_chunk
btrfs-progs: Make btrfs_alloc_chunk to handle block group creation
btrfs-progs: Introduce btrfs_raid_array and related infrastructures
btrfs-progs: volumes: Allow find_free_dev_extent() to return maximum
hole size
btrfs-progs: kernel-lib: Port kernel sort() to btrfs-progs
btrfs-progs: volumes: Unify free dev extent search behavior between
kernel and btrfs-progs.

 Makefile          |   3 +-
 convert/main.c    |  10 +-
 ctree.h           |  12 +-
 extent-tree.c     |  12 +-
 kerncompat.h      |   5 +
 kernel-lib/sort.c | 104 +++++++
 kernel-lib/sort.h |  16 ++
 mkfs/main.c       |  27 +-
 volumes.c         | 838 +++++++++++++++++++++++++-----------------------------
 volumes.h         |  42 ++-
 10 files changed, 578 insertions(+), 491 deletions(-)
 create mode 100644 kernel-lib/sort.c
 create mode 100644 kernel-lib/sort.h
Signature · 1 line
2.16.1
author
#1 lore raw

PATCH 1/7 btrfs-progs: Refactor parameter of BTRFS_MAX_DEVS() from root to fs_info

---
 volumes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
volumes.c +3 -3
index edad367b593c..677d085de96c 100644
@@ -826,7 +826,7 @@ error:
 	return ret;
 }
 
-#define BTRFS_MAX_DEVS(r) ((BTRFS_LEAF_DATA_SIZE(r->fs_info)	\
+#define BTRFS_MAX_DEVS(info) ((BTRFS_LEAF_DATA_SIZE(info)	\
 			- sizeof(struct btrfs_item)		\
 			- sizeof(struct btrfs_chunk))		\
 			/ sizeof(struct btrfs_stripe) + 1)
@@ -882,12 +882,12 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 			calc_size = SZ_1G;
 			max_chunk_size = 10 * calc_size;
 			min_stripe_size = SZ_64M;
-			max_stripes = BTRFS_MAX_DEVS(chunk_root);
+			max_stripes = BTRFS_MAX_DEVS(info);
 		} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
 			calc_size = SZ_1G;
 			max_chunk_size = 4 * calc_size;
 			min_stripe_size = SZ_32M;
-			max_stripes = BTRFS_MAX_DEVS(chunk_root);
+			max_stripes = BTRFS_MAX_DEVS(info);
 		}
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1) {
Signature · 1 line
2.16.1
author
#2 lore raw

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

In reply to 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.

---
 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(-)
convert/main.c +3 -3
index b3ea31d7af43..b2444bb2ff21 100644
@@ -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,
extent-tree.c +1 -1
index e2ae74a7fe66..b085ab0352b3 100644
@@ -1906,7 +1906,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;
mkfs/main.c +4 -4
index ea65c6d897b2..358395ca0250 100644
@@ -82,7 +82,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;
@@ -99,7 +99,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;
@@ -133,7 +133,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;
@@ -241,7 +241,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);
volumes.c +85 -134

281 lines not on this page · raw message

volumes.h +1 -4
index 11572e78c04f..7bbdf615d31a 100644
@@ -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);
Signature · 1 line
2.16.1
#3 lore raw

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

Quoted 121 lines · 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.

---
  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(-)
convert/main.c +3 -3
index b3ea31d7af43..b2444bb2ff21 100644
@@ -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,
extent-tree.c +1 -1
index e2ae74a7fe66..b085ab0352b3 100644
@@ -1906,7 +1906,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;
mkfs/main.c +4 -4
index ea65c6d897b2..358395ca0250 100644
@@ -82,7 +82,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;
@@ -99,7 +99,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;
@@ -133,7 +133,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;
@@ -241,7 +241,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);
volumes.c +15 -1
index 677d085de96c..9ee4650351c3 100644
@@ -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;

volumes.c int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 890

Quoted 10 lines
volumes.c +5 -1
@@ -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?

Thanks,
Su

Quoted 271 lines
volumes.c +65 -132
+			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(&device->dev_list, &private_devs);
+			list_move_tail(&device->dev_list,
+				       &private_devs);
  			index++;
  			if (type & BTRFS_BLOCK_GROUP_DUP)
  				index++;
@@ -999,9 +1045,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;
@@ -1022,17 +1075,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;
 
@@ -1049,7 +1116,7 @@ again:
  		memcpy(stripe->dev_uuid, device->uuid, BTRFS_UUID_SIZE);
  		index++;
  	}
-	BUG_ON(!list_empty(&private_devs));
+	BUG_ON(!convert && !list_empty(&private_devs));
 
  	/* key was set above */
  	btrfs_set_stack_chunk_length(chunk, *num_bytes);
@@ -1069,6 +1136,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);
@@ -1098,125 +1168,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;
volumes.h +1 -4
index 11572e78c04f..7bbdf615d31a 100644
@@ -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);
#4 lore raw

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

Quoted 22 lines · Su Yue
Quoted 19 lines · 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.

---
  convert/main.c |   6 +-
  extent-tree.c  |   2 +-
  mkfs/main.c    |   8 +--
  volumes.c      | 219
++++++++++++++++++++++-----------------------------------

[snip]

Quoted 34 lines
Quoted 34 lines
volumes.c +0 -0
index 677d085de96c..9ee4650351c3 100644
@@ -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;

volumes.c +0 -0
@@ -868,10 +882,38 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle
 *trans,
       struct btrfs_key key;
       u64 offset;
Quoted 8 lines

volumes.c int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 890

Quoted 7 lines
volumes.c +5 -0
   -    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

Quoted 274 lines

Thanks,
Su

Quoted 268 lines
volumes.c +21 -0
+            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;

volumes.c +0 -0
@@ -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) {

volumes.c +12 -7
@@ -972,7 +1017,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++;
@@ -999,9 +1045,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;
@@ -1022,17 +1075,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;
  @@ -1049,7 +1116,7 @@ again:
          memcpy(stripe->dev_uuid, device->uuid, BTRFS_UUID_SIZE); index++;
      }
-    BUG_ON(!list_empty(&private_devs));
+    BUG_ON(!convert && !list_empty(&private_devs));
        /* key was set above */ btrfs_set_stack_chunk_length(chunk, *num_bytes);

volumes.c +0 -0
@@ -1069,6 +1136,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);

volumes.c +0 -0
@@ -1098,125 +1168,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.
- */

volumes.c +0 -111
-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;

volumes.h +1 -4
index 11572e78c04f..7bbdf615d31a 100644
@@ -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);

Reviewed-by
#5 lore raw

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

Quoted 69 lines · Qu Wenruo
Quoted 22 lines · Su Yue
Quoted 19 lines · 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.

---
   convert/main.c |   6 +-
   extent-tree.c  |   2 +-
   mkfs/main.c    |   8 +--
   volumes.c      | 219

++++++++++++++++++++++-----------------------------------

[snip]

Quoted 34 lines
Quoted 34 lines
volumes.c +11 -0
index 677d085de96c..9ee4650351c3 100644
@@ -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;

volumes.c +0 -0
@@ -868,10 +882,38 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle
 *trans,
       struct btrfs_key key;
       u64 offset;
Quoted 8 lines

volumes.c int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 890

Quoted 7 lines
volumes.c +5 -0
   -    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?

Quoted 3 lines

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

OK.

Quoted 278 lines

Thanks,
Qu

Quoted 274 lines

Thanks,
Su

Quoted 268 lines
volumes.c +25 -0
+            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) {

volumes.c +37 -130
@@ -972,7 +1017,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++;
@@ -999,9 +1045,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;
@@ -1022,17 +1075,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;
   @@ -1049,7 +1116,7 @@ again:
           memcpy(stripe->dev_uuid, device->uuid, BTRFS_UUID_SIZE);
           index++;
       }
-    BUG_ON(!list_empty(&private_devs));
+    BUG_ON(!convert && !list_empty(&private_devs));
         /* key was set above */
       btrfs_set_stack_chunk_length(chunk, *num_bytes);
@@ -1069,6 +1136,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);
@@ -1098,125 +1168,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;
volumes.h +1 -4
index 11572e78c04f..7bbdf615d31a 100644
@@ -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);

author
#6 lore raw

PATCH 3/7 btrfs-progs: Make btrfs_alloc_chunk to handle block group creation

In reply to Qu Wenruo

Before this patch, chunk allocation is split into 2 parts:

1) Chunk allocation
Handled by btrfs_alloc_chunk(), which will insert chunk and device extent items.

2) Block group allocation
Handled by btrfs_make_block_group(), which will insert block group item and update space info.

However for chunk allocation, we don't really need to split these operations as all btrfs_alloc_chunk() has btrfs_make_block_group() followed.

So it's reasonable to merge btrfs_make_block_group() call into btrfs_alloc_chunk() to save several lines, and provides the basis for later btrfs_alloc_chunk() rework.

---
 convert/main.c |  4 ----
 extent-tree.c  | 10 ++--------
 mkfs/main.c    | 19 -------------------
 volumes.c      | 10 ++++++----
 4 files changed, 8 insertions(+), 35 deletions(-)
convert/main.c +0 -4
index b2444bb2ff21..240d3aa46db9 100644
@@ -915,10 +915,6 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
 					BTRFS_BLOCK_GROUP_DATA, true);
 			if (ret < 0)
 				break;
-			ret = btrfs_make_block_group(trans, fs_info, 0,
-					BTRFS_BLOCK_GROUP_DATA, cur, len);
-			if (ret < 0)
-				break;
 			cur += len;
 		}
 	}
extent-tree.c +2 -8
index b085ab0352b3..bccd83d1bae6 100644
@@ -1909,15 +1909,9 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,
 	                        space_info->flags, false);
 	if (ret == -ENOSPC) {
 		space_info->full = 1;
-		return 0;
+		return ret;
 	}
-
-	BUG_ON(ret);
-
-	ret = btrfs_make_block_group(trans, fs_info, 0, space_info->flags,
-				     start, num_bytes);
-	BUG_ON(ret);
-	return 0;
+	return ret;
 }
 
 static int update_block_group(struct btrfs_root *root,
mkfs/main.c +0 -19
index 358395ca0250..49159ea533b9 100644
@@ -87,12 +87,6 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 			error("no space to allocate data/metadata chunk");
 			goto err;
 		}
-		if (ret)
-			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     BTRFS_BLOCK_GROUP_METADATA |
-					     BTRFS_BLOCK_GROUP_DATA,
-					     chunk_start, chunk_size);
 		if (ret)
 			return ret;
 		allocation->mixed += chunk_size;
@@ -106,12 +100,7 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 		}
 		if (ret)
 			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     BTRFS_BLOCK_GROUP_METADATA,
-					     chunk_start, chunk_size);
 		allocation->metadata += chunk_size;
-		if (ret)
-			return ret;
 	}
 
 	root->fs_info->system_allocs = 0;
@@ -140,12 +129,7 @@ static int create_data_block_groups(struct btrfs_trans_handle *trans,
 		}
 		if (ret)
 			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     BTRFS_BLOCK_GROUP_DATA,
-					     chunk_start, chunk_size);
 		allocation->data += chunk_size;
-		if (ret)
-			return ret;
 	}
 
 err:
@@ -249,9 +233,6 @@ static int create_one_raid_group(struct btrfs_trans_handle *trans,
 	if (ret)
 		return ret;
 
-	ret = btrfs_make_block_group(trans, fs_info, 0,
-				     type, chunk_start, chunk_size);
-
 	type &= BTRFS_BLOCK_GROUP_TYPE_MASK;
 	if (type == BTRFS_BLOCK_GROUP_DATA) {
 		allocation->data += chunk_size;
volumes.c +6 -4
index 9ee4650351c3..a9dc8c939dc5 100644
@@ -837,10 +837,9 @@ error:
 				/ 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()
+ * 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).
  *
  * @start:	return value of allocated chunk start bytenr.
  * @num_bytes:	return value of allocated chunk size
@@ -1159,6 +1158,9 @@ alloc_chunk:
 	}
 
 	kfree(chunk);
+
+	ret = btrfs_make_block_group(trans, info, 0, type, map->ce.start,
+				     map->ce.size);
 	return ret;
 
 out_chunk_map:
Signature · 1 line
2.16.1
#7 lore raw

Re: [PATCH 3/7] btrfs-progs: Make btrfs_alloc_chunk to handle block group creation

Quoted 135 lines · Qu Wenruo

Before this patch, chunk allocation is split into 2 parts:

1) Chunk allocation
Handled by btrfs_alloc_chunk(), which will insert chunk and device extent items.

2) Block group allocation
Handled by btrfs_make_block_group(), which will insert block group item and update space info.

However for chunk allocation, we don't really need to split these operations as all btrfs_alloc_chunk() has btrfs_make_block_group() followed.

So it's reasonable to merge btrfs_make_block_group() call into btrfs_alloc_chunk() to save several lines, and provides the basis for later btrfs_alloc_chunk() rework.

---
 convert/main.c |  4 ----
 extent-tree.c  | 10 ++--------
 mkfs/main.c    | 19 -------------------
 volumes.c      | 10 ++++++----
 4 files changed, 8 insertions(+), 35 deletions(-)
convert/main.c +0 -4
index b2444bb2ff21..240d3aa46db9 100644
@@ -915,10 +915,6 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
 					BTRFS_BLOCK_GROUP_DATA, true);
 			if (ret < 0)
 				break;
-			ret = btrfs_make_block_group(trans, fs_info, 0,
-					BTRFS_BLOCK_GROUP_DATA, cur, len);
-			if (ret < 0)
-				break;
 			cur += len;
 		}
 	}
extent-tree.c +2 -8
index b085ab0352b3..bccd83d1bae6 100644
@@ -1909,15 +1909,9 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,
 	                        space_info->flags, false);
 	if (ret == -ENOSPC) {
 		space_info->full = 1;
-		return 0;
+		return ret;
 	}
-
-	BUG_ON(ret);
-
-	ret = btrfs_make_block_group(trans, fs_info, 0, space_info->flags,
-				     start, num_bytes);
-	BUG_ON(ret);
-	return 0;
+	return ret;
 }
 
 static int update_block_group(struct btrfs_root *root,
mkfs/main.c +0 -19
index 358395ca0250..49159ea533b9 100644
@@ -87,12 +87,6 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 			error("no space to allocate data/metadata chunk");
 			goto err;
 		}
-		if (ret)
-			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     BTRFS_BLOCK_GROUP_METADATA |
-					     BTRFS_BLOCK_GROUP_DATA,
-					     chunk_start, chunk_size);
 		if (ret)
 			return ret;
 		allocation->mixed += chunk_size;
@@ -106,12 +100,7 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 		}
 		if (ret)
 			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     BTRFS_BLOCK_GROUP_METADATA,
-					     chunk_start, chunk_size);
 		allocation->metadata += chunk_size;
-		if (ret)
-			return ret;
 	}
 
 	root->fs_info->system_allocs = 0;
@@ -140,12 +129,7 @@ static int create_data_block_groups(struct btrfs_trans_handle *trans,
 		}
 		if (ret)
 			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     BTRFS_BLOCK_GROUP_DATA,
-					     chunk_start, chunk_size);
 		allocation->data += chunk_size;
-		if (ret)
-			return ret;
 	}
 
 err:
@@ -249,9 +233,6 @@ static int create_one_raid_group(struct btrfs_trans_handle *trans,
 	if (ret)
 		return ret;
 
-	ret = btrfs_make_block_group(trans, fs_info, 0,
-				     type, chunk_start, chunk_size);
-
 	type &= BTRFS_BLOCK_GROUP_TYPE_MASK;
 	if (type == BTRFS_BLOCK_GROUP_DATA) {
 		allocation->data += chunk_size;
volumes.c +3 -4
index 9ee4650351c3..a9dc8c939dc5 100644
@@ -837,10 +837,9 @@ error:
 				/ 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()
+ * 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).
  *
  * @start:	return value of allocated chunk start bytenr.
  * @num_bytes:	return value of allocated chunk size

volumes.c alloc_chunk: · line 1161

Quoted 4 lines
volumes.c +1 -0
@@ -1159,6 +1158,9 @@ alloc_chunk:
 	}
 
 	kfree(chunk);
+

Add a comment here, visually separating this step of the process, similar to your previous patch:

/*
* Create the block group describing the chunk item */

or whatever text you deem correct.

Quoted 6 lines
volumes.c +2 -0
+	ret = btrfs_make_block_group(trans, info, 0, type, map->ce.start,
+				     map->ce.size);
 	return ret;
 
 out_chunk_map:
author
#8 lore raw

PATCH 4/7 btrfs-progs: Introduce btrfs_raid_array and related infrastructures

In reply to Qu Wenruo

As part of the effort to unify code and behavior between btrfs-progs and kernel, copy the btrfs_raid_array from kernel to btrfs-progs.

So later we can use the btrfs_raid_array[] to get needed raid info other than manually do if-else branches.

---
 ctree.h   | 12 +++++++++++-
 volumes.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 volumes.h | 30 +++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)
ctree.h +11 -1
index 17cdac76c58c..c76849d8deb7 100644
@@ -958,7 +958,17 @@ struct btrfs_csum_item {
 #define BTRFS_BLOCK_GROUP_RAID5    	(1ULL << 7)
 #define BTRFS_BLOCK_GROUP_RAID6    	(1ULL << 8)
 #define BTRFS_BLOCK_GROUP_RESERVED	BTRFS_AVAIL_ALLOC_BIT_SINGLE
-#define BTRFS_NR_RAID_TYPES             7
+
+enum btrfs_raid_types {
+	BTRFS_RAID_RAID10,
+	BTRFS_RAID_RAID1,
+	BTRFS_RAID_DUP,
+	BTRFS_RAID_RAID0,
+	BTRFS_RAID_SINGLE,
+	BTRFS_RAID_RAID5,
+	BTRFS_RAID_RAID6,
+	BTRFS_NR_RAID_TYPES
+};
 
 #define BTRFS_BLOCK_GROUP_TYPE_MASK	(BTRFS_BLOCK_GROUP_DATA |    \
 					 BTRFS_BLOCK_GROUP_SYSTEM |  \
volumes.c +66 -0

72 lines not on this page · raw message

volumes.h +30 -0
index 7bbdf615d31a..612a0a7586f4 100644
@@ -108,6 +108,36 @@ struct map_lookup {
 	struct btrfs_bio_stripe stripes[];
 };
 
+struct btrfs_raid_attr {
+	int sub_stripes;	/* sub_stripes info for map */
+	int dev_stripes;	/* stripes per dev */
+	int devs_max;		/* max devs to use */
+	int devs_min;		/* min devs needed */
+	int tolerated_failures; /* max tolerated fail devs */
+	int devs_increment;	/* ndevs has to be a multiple of this */
+	int ncopies;		/* how many copies to data has */
+};
+
+extern const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES];
+
+static inline enum btrfs_raid_types btrfs_bg_flags_to_raid_index(u64 flags)
+{
+	if (flags & BTRFS_BLOCK_GROUP_RAID10)
+		return BTRFS_RAID_RAID10;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID1)
+		return BTRFS_RAID_RAID1;
+	else if (flags & BTRFS_BLOCK_GROUP_DUP)
+		return BTRFS_RAID_DUP;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID0)
+		return BTRFS_RAID_RAID0;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID5)
+		return BTRFS_RAID_RAID5;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID6)
+		return BTRFS_RAID_RAID6;
+
+	return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
+}
+
 #define btrfs_multi_bio_size(n) (sizeof(struct btrfs_multi_bio) + \
 			    (sizeof(struct btrfs_bio_stripe) * (n)))
 #define btrfs_map_lookup_size(n) (sizeof(struct map_lookup) + \
Signature · 1 line
2.16.1
Reviewed-by on v1 4/7
#9 lore raw

Re: [PATCH 4/7] btrfs-progs: Introduce btrfs_raid_array and related infrastructures

Quoted 18 lines · Qu Wenruo

As part of the effort to unify code and behavior between btrfs-progs and kernel, copy the btrfs_raid_array from kernel to btrfs-progs.

So later we can use the btrfs_raid_array[] to get needed raid info other than manually do if-else branches.

---
 ctree.h   | 12 +++++++++++-
 volumes.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 volumes.h | 30 +++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)
ctree.h +0 -0
index 17cdac76c58c..c76849d8deb7 100644

ctree.h struct btrfs_csum_item { · line 960

Quoted 3 lines
ctree.h +0 -0
@@ -958,7 +958,17 @@ struct btrfs_csum_item {
 #define BTRFS_BLOCK_GROUP_RAID5    	(1ULL << 7)
 #define BTRFS_BLOCK_GROUP_RAID6    	(1ULL << 8)
 #define BTRFS_BLOCK_GROUP_RESERVED	BTRFS_AVAIL_ALLOC_BIT_SINGLE

<not related to this patch but i just saw it>

BTRFS_BLOCK_GROUP_RESERVED differs in userspace than in kernel space. The btrfs_Tree header has it defined as:

#define BTRFS_BLOCK_GROUP_RESERVED (BTRFS_AVAIL_ALLOC_BIT_SINGLE | \
BTRFS_SPACE_INFO_GLOBAL_RSV)

Otherwise LGTM:

Quoted 134 lines
ctree.h +11 -1
-#define BTRFS_NR_RAID_TYPES             7
+
+enum btrfs_raid_types {
+	BTRFS_RAID_RAID10,
+	BTRFS_RAID_RAID1,
+	BTRFS_RAID_DUP,
+	BTRFS_RAID_RAID0,
+	BTRFS_RAID_SINGLE,
+	BTRFS_RAID_RAID5,
+	BTRFS_RAID_RAID6,
+	BTRFS_NR_RAID_TYPES
+};
 
 #define BTRFS_BLOCK_GROUP_TYPE_MASK	(BTRFS_BLOCK_GROUP_DATA |    \
 					 BTRFS_BLOCK_GROUP_SYSTEM |  \
volumes.c +66 -0
index a9dc8c939dc5..b47ff1f392b5 100644
@@ -30,6 +30,72 @@
 #include "utils.h"
 #include "kernel-lib/raid56.h"
 
+const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
+	[BTRFS_RAID_RAID10] = {
+		.sub_stripes	= 2,
+		.dev_stripes	= 1,
+		.devs_max	= 0,	/* 0 == as many as possible */
+		.devs_min	= 4,
+		.tolerated_failures = 1,
+		.devs_increment	= 2,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_RAID1] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 2,
+		.devs_min	= 2,
+		.tolerated_failures = 1,
+		.devs_increment	= 2,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_DUP] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 2,
+		.devs_max	= 1,
+		.devs_min	= 1,
+		.tolerated_failures = 0,
+		.devs_increment	= 1,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_RAID0] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 0,
+		.devs_min	= 2,
+		.tolerated_failures = 0,
+		.devs_increment	= 1,
+		.ncopies	= 1,
+	},
+	[BTRFS_RAID_SINGLE] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 1,
+		.devs_min	= 1,
+		.tolerated_failures = 0,
+		.devs_increment	= 1,
+		.ncopies	= 1,
+	},
+	[BTRFS_RAID_RAID5] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 0,
+		.devs_min	= 2,
+		.tolerated_failures = 1,
+		.devs_increment	= 1,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_RAID6] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 0,
+		.devs_min	= 3,
+		.tolerated_failures = 2,
+		.devs_increment	= 1,
+		.ncopies	= 3,
+	},
+};
+
 struct stripe {
 	struct btrfs_device *dev;
 	u64 physical;
volumes.h +30 -0
index 7bbdf615d31a..612a0a7586f4 100644
@@ -108,6 +108,36 @@ struct map_lookup {
 	struct btrfs_bio_stripe stripes[];
 };
 
+struct btrfs_raid_attr {
+	int sub_stripes;	/* sub_stripes info for map */
+	int dev_stripes;	/* stripes per dev */
+	int devs_max;		/* max devs to use */
+	int devs_min;		/* min devs needed */
+	int tolerated_failures; /* max tolerated fail devs */
+	int devs_increment;	/* ndevs has to be a multiple of this */
+	int ncopies;		/* how many copies to data has */
+};
+
+extern const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES];
+
+static inline enum btrfs_raid_types btrfs_bg_flags_to_raid_index(u64 flags)
+{
+	if (flags & BTRFS_BLOCK_GROUP_RAID10)
+		return BTRFS_RAID_RAID10;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID1)
+		return BTRFS_RAID_RAID1;
+	else if (flags & BTRFS_BLOCK_GROUP_DUP)
+		return BTRFS_RAID_DUP;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID0)
+		return BTRFS_RAID_RAID0;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID5)
+		return BTRFS_RAID_RAID5;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID6)
+		return BTRFS_RAID_RAID6;
+
+	return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
+}
+
 #define btrfs_multi_bio_size(n) (sizeof(struct btrfs_multi_bio) + \
 			    (sizeof(struct btrfs_bio_stripe) * (n)))
 #define btrfs_map_lookup_size(n) (sizeof(struct map_lookup) + \
#10 lore raw

Re: [PATCH 4/7] btrfs-progs: Introduce btrfs_raid_array and related infrastructures

Quoted 26 lines · Nikolay Borisov
Quoted 18 lines · Qu Wenruo

As part of the effort to unify code and behavior between btrfs-progs and kernel, copy the btrfs_raid_array from kernel to btrfs-progs.

So later we can use the btrfs_raid_array[] to get needed raid info other than manually do if-else branches.

---
 ctree.h   | 12 +++++++++++-
 volumes.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 volumes.h | 30 +++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)
ctree.h +0 -0
index 17cdac76c58c..c76849d8deb7 100644

ctree.h struct btrfs_csum_item { · line 960

Quoted 3 lines
ctree.h +0 -0
@@ -958,7 +958,17 @@ struct btrfs_csum_item {
 #define BTRFS_BLOCK_GROUP_RAID5    	(1ULL << 7)
 #define BTRFS_BLOCK_GROUP_RAID6    	(1ULL << 8)
 #define BTRFS_BLOCK_GROUP_RESERVED	BTRFS_AVAIL_ALLOC_BIT_SINGLE

<not related to this patch but i just saw it>

Quoted 5 lines

BTRFS_BLOCK_GROUP_RESERVED differs in userspace than in kernel space. The btrfs_Tree header has it defined as:

#define BTRFS_BLOCK_GROUP_RESERVED (BTRFS_AVAIL_ALLOC_BIT_SINGLE | \
BTRFS_SPACE_INFO_GLOBAL_RSV)

Another good place to cleanup. :)

Thanks,
Qu

Quoted 145 lines

Otherwise LGTM:

Quoted 124 lines
ctree.h +11 -1
-#define BTRFS_NR_RAID_TYPES             7
+
+enum btrfs_raid_types {
+	BTRFS_RAID_RAID10,
+	BTRFS_RAID_RAID1,
+	BTRFS_RAID_DUP,
+	BTRFS_RAID_RAID0,
+	BTRFS_RAID_SINGLE,
+	BTRFS_RAID_RAID5,
+	BTRFS_RAID_RAID6,
+	BTRFS_NR_RAID_TYPES
+};
 
 #define BTRFS_BLOCK_GROUP_TYPE_MASK	(BTRFS_BLOCK_GROUP_DATA |    \
 					 BTRFS_BLOCK_GROUP_SYSTEM |  \
volumes.c +66 -0
index a9dc8c939dc5..b47ff1f392b5 100644
@@ -30,6 +30,72 @@
 #include "utils.h"
 #include "kernel-lib/raid56.h"
 
+const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
+	[BTRFS_RAID_RAID10] = {
+		.sub_stripes	= 2,
+		.dev_stripes	= 1,
+		.devs_max	= 0,	/* 0 == as many as possible */
+		.devs_min	= 4,
+		.tolerated_failures = 1,
+		.devs_increment	= 2,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_RAID1] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 2,
+		.devs_min	= 2,
+		.tolerated_failures = 1,
+		.devs_increment	= 2,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_DUP] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 2,
+		.devs_max	= 1,
+		.devs_min	= 1,
+		.tolerated_failures = 0,
+		.devs_increment	= 1,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_RAID0] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 0,
+		.devs_min	= 2,
+		.tolerated_failures = 0,
+		.devs_increment	= 1,
+		.ncopies	= 1,
+	},
+	[BTRFS_RAID_SINGLE] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 1,
+		.devs_min	= 1,
+		.tolerated_failures = 0,
+		.devs_increment	= 1,
+		.ncopies	= 1,
+	},
+	[BTRFS_RAID_RAID5] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 0,
+		.devs_min	= 2,
+		.tolerated_failures = 1,
+		.devs_increment	= 1,
+		.ncopies	= 2,
+	},
+	[BTRFS_RAID_RAID6] = {
+		.sub_stripes	= 1,
+		.dev_stripes	= 1,
+		.devs_max	= 0,
+		.devs_min	= 3,
+		.tolerated_failures = 2,
+		.devs_increment	= 1,
+		.ncopies	= 3,
+	},
+};
+
 struct stripe {
 	struct btrfs_device *dev;
 	u64 physical;
volumes.h +23 -0
index 7bbdf615d31a..612a0a7586f4 100644
@@ -108,6 +108,36 @@ struct map_lookup {
 	struct btrfs_bio_stripe stripes[];
 };
 
+struct btrfs_raid_attr {
+	int sub_stripes;	/* sub_stripes info for map */
+	int dev_stripes;	/* stripes per dev */
+	int devs_max;		/* max devs to use */
+	int devs_min;		/* min devs needed */
+	int tolerated_failures; /* max tolerated fail devs */
+	int devs_increment;	/* ndevs has to be a multiple of this */
+	int ncopies;		/* how many copies to data has */
+};
+
+extern const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES];
+
+static inline enum btrfs_raid_types btrfs_bg_flags_to_raid_index(u64 flags)
+{
+	if (flags & BTRFS_BLOCK_GROUP_RAID10)
+		return BTRFS_RAID_RAID10;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID1)
+		return BTRFS_RAID_RAID1;
+	else if (flags & BTRFS_BLOCK_GROUP_DUP)
+		return BTRFS_RAID_DUP;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID0)
+		return BTRFS_RAID_RAID0;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID5)

volumes.h struct map_lookup { · line 143

Quoted 10 lines
volumes.h +7 -0
+		return BTRFS_RAID_RAID5;
+	else if (flags & BTRFS_BLOCK_GROUP_RAID6)
+		return BTRFS_RAID_RAID6;
+
+	return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
+}
+
 #define btrfs_multi_bio_size(n) (sizeof(struct btrfs_multi_bio) + \
 			    (sizeof(struct btrfs_bio_stripe) * (n)))
 #define btrfs_map_lookup_size(n) (sizeof(struct map_lookup) + \

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#11 lore raw

PATCH 5/7 btrfs-progs: volumes: Allow find_free_dev_extent() to return maximum hole size

In reply to Qu Wenruo

Just as kernel find_free_dev_extent(), allow it to return maximum hole size for us to build device list for later chunk allocator rework.

---
 volumes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
volumes.c +3 -3
index b47ff1f392b5..f4009ffa7c9e 100644
@@ -516,10 +516,10 @@ out:
 }
 
 static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
-				u64 *start)
+				u64 *start, u64 *len)
 {
 	/* FIXME use last free of some kind */
-	return find_free_dev_extent_start(device, num_bytes, 0, start, NULL);
+	return find_free_dev_extent_start(device, num_bytes, 0, start, len);
 }
 
 static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
@@ -543,7 +543,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * is responsible to make sure it's free.
 	 */
 	if (!convert) {
-		ret = find_free_dev_extent(device, num_bytes, start);
+		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;
 	}
Signature · 1 line
2.16.1
#12 lore raw

Re: [PATCH 5/7] btrfs-progs: volumes: Allow find_free_dev_extent() to return maximum hole size

Quoted 13 lines · Qu Wenruo

Just as kernel find_free_dev_extent(), allow it to return maximum hole size for us to build device list for later chunk allocator rework.

---
 volumes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
volumes.c +0 -0
index b47ff1f392b5..f4009ffa7c9e 100644

volumes.c out: · line 522

Quoted 9 lines
volumes.c +2 -2
@@ -516,10 +516,10 @@ out:
 }
 
 static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
-				u64 *start)
+				u64 *start, u64 *len)
 {
 	/* FIXME use last free of some kind */
-	return find_free_dev_extent_start(device, num_bytes, 0, start, NULL);
+	return find_free_dev_extent_start(device, num_bytes, 0, start, len);

Why do we need the free_dev_extent() wrapper over free_dev_extent_start at all?

Quoted 13 lines
volumes.c +1 -1
 }
 
 static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
@@ -543,7 +543,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * is responsible to make sure it's free.
 	 */
 	if (!convert) {
-		ret = find_free_dev_extent(device, num_bytes, start);
+		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;
 	}
#13 lore raw

Re: [PATCH 5/7] btrfs-progs: volumes: Allow find_free_dev_extent() to return maximum hole size

Quoted 18 lines · Nikolay Borisov
Quoted 15 lines · Qu Wenruo

Just as kernel find_free_dev_extent(), allow it to return maximum hole size for us to build device list for later chunk allocator rework.

---
 volumes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
volumes.c +0 -0
index b47ff1f392b5..f4009ffa7c9e 100644
@@ -516,10 +516,10 @@ out:
 }
 
Quoted 10 lines

volumes.c out: · line 522

Quoted 7 lines
volumes.c +2 -2
 static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
-				u64 *start)
+				u64 *start, u64 *len)
 {
 	/* FIXME use last free of some kind */
-	return find_free_dev_extent_start(device, num_bytes, 0, start, NULL);
+	return find_free_dev_extent_start(device, num_bytes, 0, start, len);

Why do we need the free_dev_extent() wrapper over free_dev_extent_start at all?

Because kernel part still needs find_free_dev_extent(), in extent-tree.c.

I'd prefer to keep the function across kernel and btrfs-progs.

Thanks,
Qu

Quoted 18 lines

volumes.c static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans, · line 549

Quoted 13 lines
volumes.c +1 -1
 }
 
 static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
@@ -543,7 +543,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * is responsible to make sure it's free.
 	 */
 	if (!convert) {
-		ret = find_free_dev_extent(device, num_bytes, start);
+		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Reviewed-by
#14 lore raw

Re: [PATCH 5/7] btrfs-progs: volumes: Allow find_free_dev_extent() to return maximum hole size

Quoted 32 lines · Qu Wenruo
Quoted 18 lines · Nikolay Borisov
Quoted 15 lines · Qu Wenruo

Just as kernel find_free_dev_extent(), allow it to return maximum hole size for us to build device list for later chunk allocator rework.

---
 volumes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
volumes.c +0 -0
index b47ff1f392b5..f4009ffa7c9e 100644
@@ -516,10 +516,10 @@ out:
 }
 
Quoted 10 lines

volumes.c out: · line 522

Quoted 7 lines
volumes.c +2 -2
 static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
-				u64 *start)
+				u64 *start, u64 *len)
 {
 	/* FIXME use last free of some kind */
-	return find_free_dev_extent_start(device, num_bytes, 0, start, NULL);
+	return find_free_dev_extent_start(device, num_bytes, 0, start, len);

Why do we need the free_dev_extent() wrapper over free_dev_extent_start at all?

Quoted 3 lines

Because kernel part still needs find_free_dev_extent(), in extent-tree.c.

I'd prefer to keep the function across kernel and btrfs-progs.

You are right, I missed it while I was grepping the first time.

Quoted 23 lines

Thanks,
Qu

Quoted 18 lines

volumes.c static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans, · line 549

Quoted 13 lines
volumes.c +1 -1
 }
 
 static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
@@ -543,7 +543,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * is responsible to make sure it's free.
 	 */
 	if (!convert) {
-		ret = find_free_dev_extent(device, num_bytes, start);
+		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#15 lore raw

PATCH 6/7 btrfs-progs: kernel-lib: Port kernel sort() to btrfs-progs

In reply to Qu Wenruo

Used by later btrfs_alloc_chunk() rework.

---
 Makefile          |   3 +-
 kernel-lib/sort.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel-lib/sort.h |  16 +++++++++
 3 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 kernel-lib/sort.c
 create mode 100644 kernel-lib/sort.h
Makefile +2 -1
index 327cdfa08eba..3db7bbe04662 100644
@@ -106,7 +106,8 @@ objects = ctree.o disk-io.o kernel-lib/radix-tree.o extent-tree.o print-tree.o \
 	  qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \
 	  kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
 	  inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \
-	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o
+	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o \
+	  kernel-lib/sort.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \
kernel-lib/sort.c new file +104 -0

104 lines not on this page · raw message

kernel-lib/sort.h new file +16 -0
new file mode 100644
index 000000000000..9355e01248f2
@@ -0,0 +1,16 @@
+/*
+ * taken from linux kernel include/list_sort.h
+ * changed include to kerncompat.h
+ */
+
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SORT_H
+#define _LINUX_SORT_H
+
+#include "kerncompat.h"
+
+void sort(void *base, size_t num, size_t size,
+	  int (*cmp)(const void *, const void *),
+	  void (*swap)(void *, void *, int));
+
+#endif
Signature · 1 line
2.16.1
author
#16 lore raw

PATCH 7/7 btrfs-progs: volumes: Unify free dev extent search behavior between kernel and btrfs-progs.

In reply to Qu Wenruo

As preparation to create libbtrfs which shares code between kernel and btrfs, this patch mainly unifies the search for free device extents.

The main modifications are:

1) Search for free device extent
Use the kernel method, by sorting the devices by its max hole capability, and use that sorted result to determine stripe size and chunk size.

The previous method, which uses available bytes of each device to search, can't handle scattered small holes in each device.

2) Chunk/stripe minimal size
Remove the minimal size requirement.
Now the real minimal stripe size is 64K (BTRFS_STRIPE_LEN), the same as kernel one.

While up limit is still kept as is, and minimal device size is still kept for a while.
But since we no longer have strange minimal stripe size limit, existing minimal device size calculation won't cause any problem.

3) How to insert device extents
Although not the same as kernel, here we follow kernel behavior to delay dev extents insert.

There is plan to follow kernel __btrfs_alloc_chunk() to make it only handle chunk mapping allocation, while do nothing with tree operation.

4) Usage of btrfs_raid_array[]
Which makes a lot of old if-else branches disappear.

There are still a lot of work to do (both kernel and btrfs-progs) before we could starting extracting code into libbtrfs, but this should make libbtrfs inside our reach.

---
 kerncompat.h |   5 +
 volumes.c    | 611 ++++++++++++++++++++++++++---------------------------------
 volumes.h    |   7 +
 3 files changed, 281 insertions(+), 342 deletions(-)
kerncompat.h +5 -0
index fa96715fb70c..658d28ed0792 100644
@@ -285,6 +285,7 @@ static inline int IS_ERR_OR_NULL(const void *ptr)
  */
 #define kmalloc(x, y) malloc(x)
 #define kzalloc(x, y) calloc(1, x)
+#define kcalloc(x, y) calloc(x, y)
 #define kstrdup(x, y) strdup(x)
 #define kfree(x) free(x)
 #define vmalloc(x) malloc(x)
@@ -394,4 +395,8 @@ struct __una_u64 { __le64 x; } __attribute__((__packed__));
 #define noinline
 #endif
 
+static inline u64 div_u64(u64 dividend, u32 divisor)
+{
+	return dividend / ((u64) divisor);
+}
 #endif
volumes.c +269 -342

738 lines not on this page · raw message

volumes.h +7 -0
index 612a0a7586f4..3741d45cae80 100644
@@ -108,6 +108,13 @@ struct map_lookup {
 	struct btrfs_bio_stripe stripes[];
 };
 
+struct btrfs_device_info {
+	struct btrfs_device *dev;
+	u64 dev_offset;
+	u64 max_avail;
+	u64 total_avail;
+};
+
 struct btrfs_raid_attr {
 	int sub_stripes;	/* sub_stripes info for map */
 	int dev_stripes;	/* stripes per dev */
Signature · 1 line
2.16.1