All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation
@ 2021-04-06  8:05 Naohiro Aota
  2021-04-06  8:05 ` [PATCH 01/12] btrfs-progs: introduce chunk allocation policy Naohiro Aota
                   ` (14 more replies)
  0 siblings, 15 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

This is the userland counterpart of the following series.

https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/

This series refactors chunk allocation and device_extent allocation
functions and make them generalized to be able to implement other
allocation policy easily.

On top of this series, we can simplify userland side of the zoned series as
adding a new type of chunk allocator and extent allocator for zoned block
devices. Furthermore, we will be able to implement and test some other
allocator in the idea page of the wiki e.g. SSD caching, dedicated metadata
drive, chunk allocation groups, and so on.

This series also fixes a bug of calculating the stripe size in DUP profile,
and cleans up the code.

* Refactoring chunk/dev_extent allocator

Two functions are separated from find_free_dev_extent_start().
dev_extent_search_start() decides the starting position of the search.
dev_extent_hole_check() checks if a hole found is suitable for device
extent allocation.

Split some parts of btrfs_alloc_chunk() into three functions.
init_alloc_chunk_policy() initializes the parameters of an allocation.
decide_stripe_size() decides the size of chunk and device_extent. And,
create_chunk() creates a chunk and device extents.

* Patch organization

Patches 1 and 2 refactor find_free_dev_extent_start().

Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the code into three
other functions.

Patch 7 uses create_chunk() to simplify btrfs_alloc_data_chunk().

Patch 8 fixes a bug of calculating stripe size in DUP profile.

Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping unnecessary
parameters, and using better macro/variable name to clarify the meaning.


Naohiro Aota (12):
  btrfs-progs: introduce chunk allocation policy
  btrfs-progs: refactor find_free_dev_extent_start()
  btrfs-progs: convert type of alloc_chunk_ctl::type
  btrfs-progs: consolidate parameter initialization of regular allocator
  btrfs-progs: factor out decide_stripe_size()
  btrfs-progs: factor out create_chunk()
  btrfs-progs: rewrite btrfs_alloc_data_chunk() using create_chunk()
  btrfs-progs: fix to use half the available space for DUP profile
  btrfs-progs: use round_down for allocation calcs
  btrfs-progs: drop alloc_chunk_ctl::stripe_len
  btrfs-progs: simplify arguments of chunk_bytes_by_type()
  btrfs-progs: rename calc_size to stripe_size

 kernel-shared/volumes.c | 514 +++++++++++++++++++++-------------------
 kernel-shared/volumes.h |   6 +
 2 files changed, 274 insertions(+), 246 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/12] btrfs-progs: introduce chunk allocation policy
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 02/12] btrfs-progs: refactor find_free_dev_extent_start() Naohiro Aota
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Introduce chunk allocation policy for btrfs. This policy controls how
chunks and device extents are allocated from devices.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 1 +
 kernel-shared/volumes.h | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index ddddae62581c..7bd6af451e78 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -254,6 +254,7 @@ static int device_list_add(const char *path,
 		fs_devices->latest_devid = devid;
 		fs_devices->latest_trans = found_transid;
 		fs_devices->lowest_devid = (u64)-1;
+		fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_REGULAR;
 		device = NULL;
 	} else {
 		device = find_device(fs_devices, devid,
diff --git a/kernel-shared/volumes.h b/kernel-shared/volumes.h
index d6e92db298ca..e1d7918dd30b 100644
--- a/kernel-shared/volumes.h
+++ b/kernel-shared/volumes.h
@@ -69,6 +69,10 @@ struct btrfs_device {
 	u8 uuid[BTRFS_UUID_SIZE];
 };
 
+enum btrfs_chunk_allocation_policy {
+	BTRFS_CHUNK_ALLOC_REGULAR,
+};
+
 struct btrfs_fs_devices {
 	u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
 	u8 metadata_uuid[BTRFS_FSID_SIZE]; /* FS specific uuid */
@@ -87,6 +91,8 @@ struct btrfs_fs_devices {
 
 	int seeding;
 	struct btrfs_fs_devices *seed;
+
+	enum btrfs_chunk_allocation_policy chunk_alloc_policy;
 };
 
 struct btrfs_bio_stripe {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/12] btrfs-progs: refactor find_free_dev_extent_start()
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
  2021-04-06  8:05 ` [PATCH 01/12] btrfs-progs: introduce chunk allocation policy Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 03/12] btrfs-progs: convert type of alloc_chunk_ctl::type Naohiro Aota
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Factor out the function dev_extent_search_start() from
find_free_dev_extent_start() to decide the starting position of a device
extent search.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 7bd6af451e78..3b1b8fc0b560 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -444,6 +444,21 @@ int btrfs_scan_one_device(int fd, const char *path,
 	return ret;
 }
 
+static u64 dev_extent_search_start(struct btrfs_device *device, u64 start)
+{
+	switch (device->fs_devices->chunk_alloc_policy) {
+	case BTRFS_CHUNK_ALLOC_REGULAR:
+		/*
+		 * We don't want to overwrite the superblock on the drive nor
+		 * any area used by the boot loader (grub for example), so we
+		 * make sure to start at an offset of at least 1MB.
+		 */
+		return max(start, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
+	default:
+		BUG();
+	}
+}
+
 /*
  * find_free_dev_extent_start - find free space in the specified device
  * @device:	  the device which we search the free space in
@@ -481,15 +496,8 @@ static int find_free_dev_extent_start(struct btrfs_device *device,
 	int ret;
 	int slot;
 	struct extent_buffer *l;
-	u64 min_search_start;
 
-	/*
-	 * We don't want to overwrite the superblock on the drive nor any area
-	 * used by the boot loader (grub for example), so we make sure to start
-	 * at an offset of at least 1MB.
-	 */
-	min_search_start = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
-	search_start = max(search_start, min_search_start);
+	search_start = dev_extent_search_start(device, search_start);
 
 	path = btrfs_alloc_path();
 	if (!path)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/12] btrfs-progs: convert type of alloc_chunk_ctl::type
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
  2021-04-06  8:05 ` [PATCH 01/12] btrfs-progs: introduce chunk allocation policy Naohiro Aota
  2021-04-06  8:05 ` [PATCH 02/12] btrfs-progs: refactor find_free_dev_extent_start() Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 04/12] btrfs-progs: consolidate parameter initialization of regular allocator Naohiro Aota
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Convert alloc_chunk_ctl::type to take the original type in
btrfs_alloc_chunk(). This will help refactoring in the following commits.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 3b1b8fc0b560..ea14a9413157 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -149,7 +149,7 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 };
 
 struct alloc_chunk_ctl {
-	enum btrfs_raid_types type;
+	u64 type;
 	int num_stripes;
 	int max_stripes;
 	int min_stripes;
@@ -1008,7 +1008,7 @@ error:
 static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 				 struct alloc_chunk_ctl *ctl)
 {
-	int type = ctl->type;
+	enum btrfs_raid_types type = btrfs_bg_flags_to_raid_index(ctl->type);
 
 	ctl->num_stripes = btrfs_raid_array[type].dev_stripes;
 	ctl->min_stripes = btrfs_raid_array[type].devs_min;
@@ -1069,7 +1069,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		return -ENOSPC;
 	}
 
-	ctl.type = btrfs_bg_flags_to_raid_index(type);
+	ctl.type = type;
 	ctl.max_stripes = 0;
 	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/12] btrfs-progs: consolidate parameter initialization of regular allocator
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (2 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 03/12] btrfs-progs: convert type of alloc_chunk_ctl::type Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 05/12] btrfs-progs: factor out decide_stripe_size() Naohiro Aota
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Move parameter initialization code for regular allocator to
init_alloc_chunk_ctl_policy_regular(). This will help adding another
allocator in the future.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 112 +++++++++++++++++++++++-----------------
 1 file changed, 64 insertions(+), 48 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index ea14a9413157..1ca71a9bc430 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -154,6 +154,9 @@ struct alloc_chunk_ctl {
 	int max_stripes;
 	int min_stripes;
 	int sub_stripes;
+	u64 calc_size;
+	u64 min_stripe_size;
+	u64 max_chunk_size;
 	int stripe_len;
 	int total_devs;
 };
@@ -1005,6 +1008,40 @@ error:
 				- 2 * sizeof(struct btrfs_chunk))	\
 				/ sizeof(struct btrfs_stripe) + 1)
 
+static void init_alloc_chunk_ctl_policy_regular(struct btrfs_fs_info *info,
+						struct alloc_chunk_ctl *ctl)
+{
+	u64 type = ctl->type;
+	u64 percent_max;
+
+	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
+		if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
+			ctl->calc_size = SZ_8M;
+			ctl->max_chunk_size = ctl->calc_size * 2;
+			ctl->min_stripe_size = SZ_1M;
+			ctl->max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
+		} else if (type & BTRFS_BLOCK_GROUP_DATA) {
+			ctl->calc_size = SZ_1G;
+			ctl->max_chunk_size = 10 * ctl->calc_size;
+			ctl->min_stripe_size = SZ_64M;
+			ctl->max_stripes = BTRFS_MAX_DEVS(info);
+		} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
+			/* for larger filesystems, use larger metadata chunks */
+			if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
+				ctl->max_chunk_size = SZ_1G;
+			else
+				ctl->max_chunk_size = SZ_256M;
+			ctl->calc_size = ctl->max_chunk_size;
+			ctl->min_stripe_size = SZ_32M;
+			ctl->max_stripes = BTRFS_MAX_DEVS(info);
+		}
+	}
+
+	/* we don't want a chunk larger than 10% of the FS */
+	percent_max = div_factor(btrfs_super_total_bytes(info->super_copy), 1);
+	ctl->max_chunk_size = min(percent_max, ctl->max_chunk_size);
+}
+
 static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 				 struct alloc_chunk_ctl *ctl)
 {
@@ -1012,8 +1049,21 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 
 	ctl->num_stripes = btrfs_raid_array[type].dev_stripes;
 	ctl->min_stripes = btrfs_raid_array[type].devs_min;
+	ctl->max_stripes = 0;
 	ctl->sub_stripes = btrfs_raid_array[type].sub_stripes;
+	ctl->calc_size = SZ_8M;
+	ctl->min_stripe_size = SZ_1M;
+	ctl->max_chunk_size = 4 * ctl->calc_size;
 	ctl->stripe_len = BTRFS_STRIPE_LEN;
+	ctl->total_devs = btrfs_super_num_devices(info->super_copy);
+
+	switch (info->fs_devices->chunk_alloc_policy) {
+	case BTRFS_CHUNK_ALLOC_REGULAR:
+		init_alloc_chunk_ctl_policy_regular(info, ctl);
+		break;
+	default:
+		BUG();
+	}
 
 	switch (type) {
 	case BTRFS_RAID_DUP:
@@ -1051,13 +1101,9 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	struct list_head *dev_list = &info->fs_devices->devices;
 	struct list_head *cur;
 	struct map_lookup *map;
-	int min_stripe_size = SZ_1M;
-	u64 calc_size = SZ_8M;
 	u64 min_free;
-	u64 max_chunk_size = 4 * calc_size;
 	u64 avail = 0;
 	u64 max_avail = 0;
-	u64 percent_max;
 	struct alloc_chunk_ctl ctl;
 	int looped = 0;
 	int ret;
@@ -1070,60 +1116,30 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 
 	ctl.type = type;
-	ctl.max_stripes = 0;
-	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
-
-	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
-		if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
-			calc_size = SZ_8M;
-			max_chunk_size = calc_size * 2;
-			min_stripe_size = SZ_1M;
-			ctl.max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
-		} else if (type & BTRFS_BLOCK_GROUP_DATA) {
-			calc_size = SZ_1G;
-			max_chunk_size = 10 * calc_size;
-			min_stripe_size = SZ_64M;
-			ctl.max_stripes = BTRFS_MAX_DEVS(info);
-		} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
-			/* for larger filesystems, use larger metadata chunks */
-			if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
-				max_chunk_size = SZ_1G;
-			else
-				max_chunk_size = SZ_256M;
-			calc_size = max_chunk_size;
-			min_stripe_size = SZ_32M;
-			ctl.max_stripes = BTRFS_MAX_DEVS(info);
-		}
-	}
-
 	init_alloc_chunk_ctl(info, &ctl);
 	if (ctl.num_stripes < ctl.min_stripes)
 		return -ENOSPC;
 
-	/* we don't want a chunk larger than 10% of the FS */
-	percent_max = div_factor(btrfs_super_total_bytes(info->super_copy), 1);
-	max_chunk_size = min(percent_max, max_chunk_size);
-
 again:
-	if (chunk_bytes_by_type(type, calc_size, &ctl) > max_chunk_size) {
-		calc_size = max_chunk_size;
-		calc_size /= ctl.num_stripes;
-		calc_size /= ctl.stripe_len;
-		calc_size *= ctl.stripe_len;
+	if (chunk_bytes_by_type(type, ctl.calc_size, &ctl) > ctl.max_chunk_size) {
+		ctl.calc_size = ctl.max_chunk_size;
+		ctl.calc_size /= ctl.num_stripes;
+		ctl.calc_size /= ctl.stripe_len;
+		ctl.calc_size *= ctl.stripe_len;
 	}
 	/* we don't want tiny stripes */
-	calc_size = max_t(u64, calc_size, min_stripe_size);
+	ctl.calc_size = max_t(u64, ctl.calc_size, ctl.min_stripe_size);
 
-	calc_size /= ctl.stripe_len;
-	calc_size *= ctl.stripe_len;
+	ctl.calc_size /= ctl.stripe_len;
+	ctl.calc_size *= ctl.stripe_len;
 	INIT_LIST_HEAD(&private_devs);
 	cur = dev_list->next;
 	index = 0;
 
 	if (type & BTRFS_BLOCK_GROUP_DUP)
-		min_free = calc_size * 2;
+		min_free = ctl.calc_size * 2;
 	else
-		min_free = calc_size;
+		min_free = ctl.calc_size;
 
 	/* build a private list of devices we will allocate from */
 	while(index < ctl.num_stripes) {
@@ -1155,7 +1171,7 @@ again:
 		}
 		if (!looped && max_avail > 0) {
 			looped = 1;
-			calc_size = max_avail;
+			ctl.calc_size = max_avail;
 			goto again;
 		}
 		return -ENOSPC;
@@ -1178,7 +1194,7 @@ again:
 	}
 
 	stripes = &chunk->stripe;
-	*num_bytes = chunk_bytes_by_type(type, calc_size, &ctl);
+	*num_bytes = chunk_bytes_by_type(type, ctl.calc_size, &ctl);
 	index = 0;
 	while(index < ctl.num_stripes) {
 		struct btrfs_stripe *stripe;
@@ -1192,11 +1208,11 @@ again:
 			list_move(&device->dev_list, dev_list);
 
 		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-			     calc_size, &dev_offset);
+			     ctl.calc_size, &dev_offset);
 		if (ret < 0)
 			goto out_chunk_map;
 
-		device->bytes_used += calc_size;
+		device->bytes_used += ctl.calc_size;
 		ret = btrfs_update_device(trans, device);
 		if (ret < 0)
 			goto out_chunk_map;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/12] btrfs-progs: factor out decide_stripe_size()
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (3 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 04/12] btrfs-progs: consolidate parameter initialization of regular allocator Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 06/12] btrfs-progs: factor out create_chunk() Naohiro Aota
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Factor out decide_stripe_size() from btrfs_alloc_chunk(). This new function
calculates the actual stripe size to allocate and decides the size of a
stripe (ctl->calc_size).

This commit has no functional changes.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 44 +++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 1ca71a9bc430..95b42eab846d 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -1087,6 +1087,37 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 	}
 }
 
+static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl)
+{
+	u64 chunk_size = chunk_bytes_by_type(ctl->type, ctl->calc_size, ctl);
+
+	if (chunk_size > ctl->max_chunk_size) {
+		ctl->calc_size = ctl->max_chunk_size;
+		ctl->calc_size /= ctl->num_stripes;
+		ctl->calc_size /= ctl->stripe_len;
+		ctl->calc_size *= ctl->stripe_len;
+	}
+	/* we don't want tiny stripes */
+	ctl->calc_size = max_t(u64, ctl->calc_size, ctl->min_stripe_size);
+
+	/* Align to the stripe length */
+	ctl->calc_size /= ctl->stripe_len;
+	ctl->calc_size *= ctl->stripe_len;
+
+	return 0;
+}
+
+static int decide_stripe_size(struct btrfs_fs_info *info,
+			      struct alloc_chunk_ctl *ctl)
+{
+	switch (info->fs_devices->chunk_alloc_policy) {
+	case BTRFS_CHUNK_ALLOC_REGULAR:
+		return decide_stripe_size_regular(ctl);
+	default:
+		BUG();
+	}
+}
+
 int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		      struct btrfs_fs_info *info, u64 *start,
 		      u64 *num_bytes, u64 type)
@@ -1121,17 +1152,10 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		return -ENOSPC;
 
 again:
-	if (chunk_bytes_by_type(type, ctl.calc_size, &ctl) > ctl.max_chunk_size) {
-		ctl.calc_size = ctl.max_chunk_size;
-		ctl.calc_size /= ctl.num_stripes;
-		ctl.calc_size /= ctl.stripe_len;
-		ctl.calc_size *= ctl.stripe_len;
-	}
-	/* we don't want tiny stripes */
-	ctl.calc_size = max_t(u64, ctl.calc_size, ctl.min_stripe_size);
+	ret = decide_stripe_size(info, &ctl);
+	if (ret < 0)
+		return ret;
 
-	ctl.calc_size /= ctl.stripe_len;
-	ctl.calc_size *= ctl.stripe_len;
 	INIT_LIST_HEAD(&private_devs);
 	cur = dev_list->next;
 	index = 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/12] btrfs-progs: factor out create_chunk()
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (4 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 05/12] btrfs-progs: factor out decide_stripe_size() Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 07/12] btrfs-progs: rewrite btrfs_alloc_data_chunk() using create_chunk() Naohiro Aota
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Factor out create_chunk() from btrfs_alloc_chunk(). This new function
creates a chunk.

There is no functional changes.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 217 ++++++++++++++++++++++------------------
 1 file changed, 120 insertions(+), 97 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 95b42eab846d..a409dd3d0366 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -149,6 +149,7 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 };
 
 struct alloc_chunk_ctl {
+	u64 start;
 	u64 type;
 	int num_stripes;
 	int max_stripes;
@@ -156,6 +157,7 @@ struct alloc_chunk_ctl {
 	int sub_stripes;
 	u64 calc_size;
 	u64 min_stripe_size;
+	u64 num_bytes;
 	u64 max_chunk_size;
 	int stripe_len;
 	int total_devs;
@@ -1118,88 +1120,23 @@ static int decide_stripe_size(struct btrfs_fs_info *info,
 	}
 }
 
-int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
-		      struct btrfs_fs_info *info, u64 *start,
-		      u64 *num_bytes, u64 type)
+static int create_chunk(struct btrfs_trans_handle *trans,
+			struct btrfs_fs_info *info, struct alloc_chunk_ctl *ctl,
+			struct list_head *private_devs)
 {
-	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 private_devs;
 	struct list_head *dev_list = &info->fs_devices->devices;
 	struct list_head *cur;
 	struct map_lookup *map;
-	u64 min_free;
-	u64 avail = 0;
-	u64 max_avail = 0;
-	struct alloc_chunk_ctl ctl;
-	int looped = 0;
 	int ret;
 	int index;
 	struct btrfs_key key;
 	u64 offset;
 
-	if (list_empty(dev_list)) {
-		return -ENOSPC;
-	}
-
-	ctl.type = type;
-	init_alloc_chunk_ctl(info, &ctl);
-	if (ctl.num_stripes < ctl.min_stripes)
-		return -ENOSPC;
-
-again:
-	ret = decide_stripe_size(info, &ctl);
-	if (ret < 0)
-		return ret;
-
-	INIT_LIST_HEAD(&private_devs);
-	cur = dev_list->next;
-	index = 0;
-
-	if (type & BTRFS_BLOCK_GROUP_DUP)
-		min_free = ctl.calc_size * 2;
-	else
-		min_free = ctl.calc_size;
-
-	/* build a private list of devices we will allocate from */
-	while(index < ctl.num_stripes) {
-		device = list_entry(cur, struct btrfs_device, dev_list);
-		ret = btrfs_device_avail_bytes(trans, device, &avail);
-		if (ret)
-			return ret;
-		cur = cur->next;
-		if (avail >= min_free) {
-			list_move(&device->dev_list, &private_devs);
-			index++;
-			if (type & BTRFS_BLOCK_GROUP_DUP)
-				index++;
-		} else if (avail > max_avail)
-			max_avail = avail;
-		if (cur == dev_list)
-			break;
-	}
-	if (index < ctl.num_stripes) {
-		list_splice(&private_devs, dev_list);
-		if (index >= ctl.min_stripes) {
-			ctl.num_stripes = index;
-			if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
-				ctl.num_stripes /= ctl.sub_stripes;
-				ctl.num_stripes *= ctl.sub_stripes;
-			}
-			looped = 1;
-			goto again;
-		}
-		if (!looped && max_avail > 0) {
-			looped = 1;
-			ctl.calc_size = max_avail;
-			goto again;
-		}
-		return -ENOSPC;
-	}
 	ret = find_next_chunk(info, &offset);
 	if (ret)
 		return ret;
@@ -1207,36 +1144,38 @@ again:
 	key.type = BTRFS_CHUNK_ITEM_KEY;
 	key.offset = offset;
 
-	chunk = kmalloc(btrfs_chunk_item_size(ctl.num_stripes), GFP_NOFS);
+	chunk = kmalloc(btrfs_chunk_item_size(ctl->num_stripes), GFP_NOFS);
 	if (!chunk)
 		return -ENOMEM;
 
-	map = kmalloc(btrfs_map_lookup_size(ctl.num_stripes), GFP_NOFS);
+	map = kmalloc(btrfs_map_lookup_size(ctl->num_stripes), GFP_NOFS);
 	if (!map) {
 		kfree(chunk);
 		return -ENOMEM;
 	}
 
 	stripes = &chunk->stripe;
-	*num_bytes = chunk_bytes_by_type(type, ctl.calc_size, &ctl);
+	ctl->num_bytes = chunk_bytes_by_type(ctl->type, ctl->calc_size, ctl);
 	index = 0;
-	while(index < ctl.num_stripes) {
+	while (index < ctl->num_stripes) {
+		u64 dev_offset;
 		struct btrfs_stripe *stripe;
-		BUG_ON(list_empty(&private_devs));
-		cur = private_devs.next;
+
+		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 == ctl.num_stripes - 1))
+		if (!(ctl->type & BTRFS_BLOCK_GROUP_DUP) ||
+		    (index == ctl->num_stripes - 1))
 			list_move(&device->dev_list, dev_list);
 
 		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-			     ctl.calc_size, &dev_offset);
+			     ctl->calc_size, &dev_offset);
 		if (ret < 0)
 			goto out_chunk_map;
 
-		device->bytes_used += ctl.calc_size;
+		device->bytes_used += ctl->calc_size;
 		ret = btrfs_update_device(trans, device);
 		if (ret < 0)
 			goto out_chunk_map;
@@ -1249,41 +1188,41 @@ again:
 		memcpy(stripe->dev_uuid, device->uuid, BTRFS_UUID_SIZE);
 		index++;
 	}
-	BUG_ON(!list_empty(&private_devs));
+	BUG_ON(!list_empty(private_devs));
 
 	/* key was set above */
-	btrfs_set_stack_chunk_length(chunk, *num_bytes);
+	btrfs_set_stack_chunk_length(chunk, ctl->num_bytes);
 	btrfs_set_stack_chunk_owner(chunk, extent_root->root_key.objectid);
-	btrfs_set_stack_chunk_stripe_len(chunk, ctl.stripe_len);
-	btrfs_set_stack_chunk_type(chunk, type);
-	btrfs_set_stack_chunk_num_stripes(chunk, ctl.num_stripes);
-	btrfs_set_stack_chunk_io_align(chunk, ctl.stripe_len);
-	btrfs_set_stack_chunk_io_width(chunk, ctl.stripe_len);
+	btrfs_set_stack_chunk_stripe_len(chunk, ctl->stripe_len);
+	btrfs_set_stack_chunk_type(chunk, ctl->type);
+	btrfs_set_stack_chunk_num_stripes(chunk, ctl->num_stripes);
+	btrfs_set_stack_chunk_io_align(chunk, ctl->stripe_len);
+	btrfs_set_stack_chunk_io_width(chunk, ctl->stripe_len);
 	btrfs_set_stack_chunk_sector_size(chunk, info->sectorsize);
-	btrfs_set_stack_chunk_sub_stripes(chunk, ctl.sub_stripes);
+	btrfs_set_stack_chunk_sub_stripes(chunk, ctl->sub_stripes);
 	map->sector_size = info->sectorsize;
-	map->stripe_len = ctl.stripe_len;
-	map->io_align = ctl.stripe_len;
-	map->io_width = ctl.stripe_len;
-	map->type = type;
-	map->num_stripes = ctl.num_stripes;
-	map->sub_stripes = ctl.sub_stripes;
+	map->stripe_len = ctl->stripe_len;
+	map->io_align = ctl->stripe_len;
+	map->io_width = ctl->stripe_len;
+	map->type = ctl->type;
+	map->num_stripes = ctl->num_stripes;
+	map->sub_stripes = ctl->sub_stripes;
 
 	ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
-				btrfs_chunk_item_size(ctl.num_stripes));
+				btrfs_chunk_item_size(ctl->num_stripes));
 	BUG_ON(ret);
-	*start = key.offset;;
+	ctl->start = key.offset;
 
 	map->ce.start = key.offset;
-	map->ce.size = *num_bytes;
+	map->ce.size = ctl->num_bytes;
 
 	ret = insert_cache_extent(&info->mapping_tree.cache_tree, &map->ce);
 	if (ret < 0)
 		goto out_chunk_map;
 
-	if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
+	if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM) {
 		ret = btrfs_add_system_chunk(info, &key,
-			    chunk, btrfs_chunk_item_size(ctl.num_stripes));
+			    chunk, btrfs_chunk_item_size(ctl->num_stripes));
 		if (ret < 0)
 			goto out_chunk;
 	}
@@ -1298,6 +1237,90 @@ out_chunk:
 	return ret;
 }
 
+int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
+		      struct btrfs_fs_info *info, u64 *start,
+		      u64 *num_bytes, u64 type)
+{
+	struct btrfs_device *device = NULL;
+	struct list_head private_devs;
+	struct list_head *dev_list = &info->fs_devices->devices;
+	struct list_head *cur;
+	u64 min_free;
+	u64 avail = 0;
+	u64 max_avail = 0;
+	struct alloc_chunk_ctl ctl;
+	int looped = 0;
+	int ret;
+	int index;
+
+	if (list_empty(dev_list))
+		return -ENOSPC;
+
+	ctl.type = type;
+	/* start and num_bytes will be set by create_chunk() */
+	ctl.start = 0;
+	ctl.num_bytes = 0;
+	init_alloc_chunk_ctl(info, &ctl);
+	if (ctl.num_stripes < ctl.min_stripes)
+		return -ENOSPC;
+
+again:
+	ret = decide_stripe_size(info, &ctl);
+	if (ret < 0)
+		return ret;
+
+	INIT_LIST_HEAD(&private_devs);
+	cur = dev_list->next;
+	index = 0;
+
+	if (type & BTRFS_BLOCK_GROUP_DUP)
+		min_free = ctl.calc_size * 2;
+	else
+		min_free = ctl.calc_size;
+
+	/* build a private list of devices we will allocate from */
+	while (index < ctl.num_stripes) {
+		device = list_entry(cur, struct btrfs_device, dev_list);
+		ret = btrfs_device_avail_bytes(trans, device, &avail);
+		if (ret)
+			return ret;
+		cur = cur->next;
+		if (avail >= min_free) {
+			list_move(&device->dev_list, &private_devs);
+			index++;
+			if (type & BTRFS_BLOCK_GROUP_DUP)
+				index++;
+		} else if (avail > max_avail)
+			max_avail = avail;
+		if (cur == dev_list)
+			break;
+	}
+	if (index < ctl.num_stripes) {
+		list_splice(&private_devs, dev_list);
+		if (index >= ctl.min_stripes) {
+			ctl.num_stripes = index;
+			if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
+				ctl.num_stripes /= ctl.sub_stripes;
+				ctl.num_stripes *= ctl.sub_stripes;
+			}
+			looped = 1;
+			goto again;
+		}
+		if (!looped && max_avail > 0) {
+			looped = 1;
+			ctl.calc_size = max_avail;
+			goto again;
+		}
+		return -ENOSPC;
+	}
+
+	ret = create_chunk(trans, info, &ctl, &private_devs);
+	*start = ctl.start;
+	*num_bytes = ctl.num_bytes;
+
+	return ret;
+}
+
 /*
  * Alloc a DATA chunk with SINGLE profile.
  *
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/12] btrfs-progs: rewrite btrfs_alloc_data_chunk() using create_chunk()
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (5 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 06/12] btrfs-progs: factor out create_chunk() Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 08/12] btrfs-progs: fix to use half the available space for DUP profile Naohiro Aota
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

btrfs_alloc_data_chunk() and create_chunk() have the most part in common.
Let's rewrite btrfs_alloc_data_chunk() using create_chunk().

There are two differences between btrfs_alloc_data_chunk() and
create_chunk(). create_chunk() uses find_next_chunk() to decide the logical
address of the chunk, and it uses btrfs_alloc_dev_extent() to decide the
physical address of a device extent. On the other hand,
btrfs_alloc_data_chunk() uses *start for both logical and physical
addresses.

To support the btrfs_alloc_data_chunk()'s use case,  we use ctl->start and
ctl->dev_offset. If these values are set (non-zero), use the specified
values as the address. It is safe to use 0 to indicate the value is not set
here. Because both lower addresses of logical
(0..BTRFS_FIRST_CHUNK_TREE_OBJECT_ID) and physical
(0..BTRFS_BLOCK_RESERVED_1M_FOR_SUPER) are reserved.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 136 +++++++++++++---------------------------
 1 file changed, 43 insertions(+), 93 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index a409dd3d0366..2a094eab4971 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -161,6 +161,7 @@ struct alloc_chunk_ctl {
 	u64 max_chunk_size;
 	int stripe_len;
 	int total_devs;
+	u64 dev_offset;
 };
 
 struct stripe {
@@ -1058,6 +1059,7 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 	ctl->max_chunk_size = 4 * ctl->calc_size;
 	ctl->stripe_len = BTRFS_STRIPE_LEN;
 	ctl->total_devs = btrfs_super_num_devices(info->super_copy);
+	ctl->dev_offset = 0;
 
 	switch (info->fs_devices->chunk_alloc_policy) {
 	case BTRFS_CHUNK_ALLOC_REGULAR:
@@ -1137,9 +1139,14 @@ static int create_chunk(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	u64 offset;
 
-	ret = find_next_chunk(info, &offset);
-	if (ret)
-		return ret;
+	if (!ctl->start) {
+		ret = find_next_chunk(info, &offset);
+		if (ret)
+			return ret;
+	} else {
+		offset = ctl->start;
+	}
+
 	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
 	key.type = BTRFS_CHUNK_ITEM_KEY;
 	key.offset = offset;
@@ -1170,10 +1177,18 @@ static int create_chunk(struct btrfs_trans_handle *trans,
 		    (index == ctl->num_stripes - 1))
 			list_move(&device->dev_list, dev_list);
 
-		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-			     ctl->calc_size, &dev_offset);
-		if (ret < 0)
-			goto out_chunk_map;
+		if (!ctl->dev_offset) {
+			ret = btrfs_alloc_dev_extent(trans, device, key.offset,
+				ctl->calc_size, &dev_offset);
+			if (ret < 0)
+				goto out_chunk_map;
+		} else {
+			dev_offset = ctl->dev_offset;
+			ret = btrfs_insert_dev_extent(trans, device, key.offset,
+						      ctl->calc_size,
+						      ctl->dev_offset);
+			BUG_ON(ret);
+		}
 
 		device->bytes_used += ctl->calc_size;
 		ret = btrfs_update_device(trans, device);
@@ -1330,23 +1345,10 @@ again:
 int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 			   struct btrfs_fs_info *info, u64 *start, u64 num_bytes)
 {
-	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 = 1;
-	int ret;
-	int index;
-	int stripe_len = BTRFS_STRIPE_LEN;
-	struct btrfs_key key;
-
+	struct list_head private_devs;
+	struct btrfs_device *device;
+	struct alloc_chunk_ctl ctl;
 
 	if (*start != round_down(*start, info->sectorsize)) {
 		error("DATA chunk start not sectorsize aligned: %llu",
@@ -1354,78 +1356,26 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 		return -EINVAL;
 	}
 
-	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
-	key.type = BTRFS_CHUNK_ITEM_KEY;
-	key.offset = *start;
-	dev_offset = *start;
-
-	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_insert_dev_extent(trans, device, key.offset, calc_size,
-				dev_offset);
-		BUG_ON(ret);
+	ctl.start = *start;
+	ctl.type = BTRFS_BLOCK_GROUP_DATA;
+	ctl.num_stripes = 1;
+	ctl.max_stripes = 1;
+	ctl.min_stripes = 1;
+	ctl.sub_stripes = 1;
+	ctl.calc_size = num_bytes;
+	ctl.min_stripe_size = num_bytes;
+	ctl.num_bytes = num_bytes;
+	ctl.max_chunk_size = num_bytes;
+	ctl.stripe_len = BTRFS_STRIPE_LEN;
+	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
+	ctl.dev_offset = *start;
 
-		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, BTRFS_BLOCK_GROUP_DATA);
-	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 = BTRFS_BLOCK_GROUP_DATA;
-	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);
-
-	map->ce.start = key.offset;
-	map->ce.size = num_bytes;
-
-	ret = insert_cache_extent(&info->mapping_tree.cache_tree, &map->ce);
-	BUG_ON(ret);
+	INIT_LIST_HEAD(&private_devs);
+	/* Build a list containing one device */
+	device = list_entry(dev_list->next, struct btrfs_device, dev_list);
+	list_move(&device->dev_list, &private_devs);
 
-	kfree(chunk);
-	return ret;
+	return create_chunk(trans, info, &ctl, &private_devs);
 }
 
 int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/12] btrfs-progs: fix to use half the available space for DUP profile
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (6 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 07/12] btrfs-progs: rewrite btrfs_alloc_data_chunk() using create_chunk() Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 09/12] btrfs-progs: use round_down for allocation calcs Naohiro Aota
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

In the DUP profile, we can use only half of the space available in a device
extent. Fix the calculation of calc_size for it.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 2a094eab4971..ed3015bf3e0c 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -1323,7 +1323,10 @@ again:
 		}
 		if (!looped && max_avail > 0) {
 			looped = 1;
-			ctl.calc_size = max_avail;
+			if (ctl.type & BTRFS_BLOCK_GROUP_DUP)
+				ctl.calc_size = max_avail / 2;
+			else
+				ctl.calc_size = max_avail;
 			goto again;
 		}
 		return -ENOSPC;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/12] btrfs-progs: use round_down for allocation calcs
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (7 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 08/12] btrfs-progs: fix to use half the available space for DUP profile Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 10/12] btrfs-progs: drop alloc_chunk_ctl::stripe_len Naohiro Aota
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Several calculations in the chunk allocation process use this pattern.

    x /= y;
    x *= y;

Replace this pattern with round_down().

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index ed3015bf3e0c..d01d825c67ce 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -1098,15 +1098,13 @@ static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl)
 	if (chunk_size > ctl->max_chunk_size) {
 		ctl->calc_size = ctl->max_chunk_size;
 		ctl->calc_size /= ctl->num_stripes;
-		ctl->calc_size /= ctl->stripe_len;
-		ctl->calc_size *= ctl->stripe_len;
+		ctl->calc_size = round_down(ctl->calc_size, ctl->stripe_len);
 	}
 	/* we don't want tiny stripes */
 	ctl->calc_size = max_t(u64, ctl->calc_size, ctl->min_stripe_size);
 
 	/* Align to the stripe length */
-	ctl->calc_size /= ctl->stripe_len;
-	ctl->calc_size *= ctl->stripe_len;
+	ctl->calc_size = round_down(ctl->calc_size, ctl->stripe_len);
 
 	return 0;
 }
@@ -1315,8 +1313,10 @@ again:
 		if (index >= ctl.min_stripes) {
 			ctl.num_stripes = index;
 			if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
-				ctl.num_stripes /= ctl.sub_stripes;
-				ctl.num_stripes *= ctl.sub_stripes;
+				/* We know this should be 2, but just in case */
+				ASSERT(is_power_of_2(ctl.sub_stripes));
+				ctl.num_stripes = round_down(ctl.num_stripes,
+							     ctl.sub_stripes);
 			}
 			looped = 1;
 			goto again;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/12] btrfs-progs: drop alloc_chunk_ctl::stripe_len
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (8 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 09/12] btrfs-progs: use round_down for allocation calcs Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 11/12] btrfs-progs: simplify arguments of chunk_bytes_by_type() Naohiro Aota
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Since commit b9444efb6658 ("btrfs-progs: don't pretend RAID56 has a
different stripe length"), alloc_chunk_ctl::stripe_len is always fixed to
BTRFS_STRIPE_LEN. Let's replace alloc_chunk_ctl::stripe_len with
BTRFS_STRIPE_LEN, like in the kernel code.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index d01d825c67ce..280537f6549d 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -159,7 +159,6 @@ struct alloc_chunk_ctl {
 	u64 min_stripe_size;
 	u64 num_bytes;
 	u64 max_chunk_size;
-	int stripe_len;
 	int total_devs;
 	u64 dev_offset;
 };
@@ -1057,7 +1056,6 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 	ctl->calc_size = SZ_8M;
 	ctl->min_stripe_size = SZ_1M;
 	ctl->max_chunk_size = 4 * ctl->calc_size;
-	ctl->stripe_len = BTRFS_STRIPE_LEN;
 	ctl->total_devs = btrfs_super_num_devices(info->super_copy);
 	ctl->dev_offset = 0;
 
@@ -1098,13 +1096,13 @@ static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl)
 	if (chunk_size > ctl->max_chunk_size) {
 		ctl->calc_size = ctl->max_chunk_size;
 		ctl->calc_size /= ctl->num_stripes;
-		ctl->calc_size = round_down(ctl->calc_size, ctl->stripe_len);
+		ctl->calc_size = round_down(ctl->calc_size, BTRFS_STRIPE_LEN);
 	}
 	/* we don't want tiny stripes */
 	ctl->calc_size = max_t(u64, ctl->calc_size, ctl->min_stripe_size);
 
 	/* Align to the stripe length */
-	ctl->calc_size = round_down(ctl->calc_size, ctl->stripe_len);
+	ctl->calc_size = round_down(ctl->calc_size, BTRFS_STRIPE_LEN);
 
 	return 0;
 }
@@ -1206,17 +1204,17 @@ static int create_chunk(struct btrfs_trans_handle *trans,
 	/* key was set above */
 	btrfs_set_stack_chunk_length(chunk, ctl->num_bytes);
 	btrfs_set_stack_chunk_owner(chunk, extent_root->root_key.objectid);
-	btrfs_set_stack_chunk_stripe_len(chunk, ctl->stripe_len);
+	btrfs_set_stack_chunk_stripe_len(chunk, BTRFS_STRIPE_LEN);
 	btrfs_set_stack_chunk_type(chunk, ctl->type);
 	btrfs_set_stack_chunk_num_stripes(chunk, ctl->num_stripes);
-	btrfs_set_stack_chunk_io_align(chunk, ctl->stripe_len);
-	btrfs_set_stack_chunk_io_width(chunk, ctl->stripe_len);
+	btrfs_set_stack_chunk_io_align(chunk, BTRFS_STRIPE_LEN);
+	btrfs_set_stack_chunk_io_width(chunk, BTRFS_STRIPE_LEN);
 	btrfs_set_stack_chunk_sector_size(chunk, info->sectorsize);
 	btrfs_set_stack_chunk_sub_stripes(chunk, ctl->sub_stripes);
 	map->sector_size = info->sectorsize;
-	map->stripe_len = ctl->stripe_len;
-	map->io_align = ctl->stripe_len;
-	map->io_width = ctl->stripe_len;
+	map->stripe_len = BTRFS_STRIPE_LEN;
+	map->io_align = BTRFS_STRIPE_LEN;
+	map->io_width = BTRFS_STRIPE_LEN;
 	map->type = ctl->type;
 	map->num_stripes = ctl->num_stripes;
 	map->sub_stripes = ctl->sub_stripes;
@@ -1369,7 +1367,6 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	ctl.min_stripe_size = num_bytes;
 	ctl.num_bytes = num_bytes;
 	ctl.max_chunk_size = num_bytes;
-	ctl.stripe_len = BTRFS_STRIPE_LEN;
 	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
 	ctl.dev_offset = *start;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 11/12] btrfs-progs: simplify arguments of chunk_bytes_by_type()
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (9 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 10/12] btrfs-progs: drop alloc_chunk_ctl::stripe_len Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:05 ` [PATCH 12/12] btrfs-progs: rename calc_size to stripe_size Naohiro Aota
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

Chunk_bytes_by_type() takes type, calc_size, and ctl as arguments. But the
first two can be obtained from the ctl. Let's drop these arguments for
simplicity.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 280537f6549d..d4ef5d626f12 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -897,9 +897,11 @@ int btrfs_add_system_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 	return 0;
 }
 
-static u64 chunk_bytes_by_type(u64 type, u64 calc_size,
-			       struct alloc_chunk_ctl *ctl)
+static u64 chunk_bytes_by_type(struct alloc_chunk_ctl *ctl)
 {
+	u64 type = ctl->type;
+	u64 calc_size = ctl->calc_size;
+
 	if (type & (BTRFS_BLOCK_GROUP_RAID1 | BTRFS_BLOCK_GROUP_DUP))
 		return calc_size;
 	else if (type & (BTRFS_BLOCK_GROUP_RAID1C3 | BTRFS_BLOCK_GROUP_RAID1C4))
@@ -1091,9 +1093,7 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 
 static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl)
 {
-	u64 chunk_size = chunk_bytes_by_type(ctl->type, ctl->calc_size, ctl);
-
-	if (chunk_size > ctl->max_chunk_size) {
+	if (chunk_bytes_by_type(ctl) > ctl->max_chunk_size) {
 		ctl->calc_size = ctl->max_chunk_size;
 		ctl->calc_size /= ctl->num_stripes;
 		ctl->calc_size = round_down(ctl->calc_size, BTRFS_STRIPE_LEN);
@@ -1158,7 +1158,7 @@ static int create_chunk(struct btrfs_trans_handle *trans,
 	}
 
 	stripes = &chunk->stripe;
-	ctl->num_bytes = chunk_bytes_by_type(ctl->type, ctl->calc_size, ctl);
+	ctl->num_bytes = chunk_bytes_by_type(ctl);
 	index = 0;
 	while (index < ctl->num_stripes) {
 		u64 dev_offset;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 12/12] btrfs-progs: rename calc_size to stripe_size
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (10 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 11/12] btrfs-progs: simplify arguments of chunk_bytes_by_type() Naohiro Aota
@ 2021-04-06  8:05 ` Naohiro Aota
  2021-04-06  8:28 ` [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Johannes Thumshirn
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06  8:05 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs, Naohiro Aota

alloc_chunk_ctl::calc_size is actually the stripe_size in the kernel side
code.  Let's rename it to clarify what the "calc" is.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 56 ++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index d4ef5d626f12..f7dd879398d4 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -155,7 +155,7 @@ struct alloc_chunk_ctl {
 	int max_stripes;
 	int min_stripes;
 	int sub_stripes;
-	u64 calc_size;
+	u64 stripe_size;
 	u64 min_stripe_size;
 	u64 num_bytes;
 	u64 max_chunk_size;
@@ -900,20 +900,20 @@ int btrfs_add_system_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 static u64 chunk_bytes_by_type(struct alloc_chunk_ctl *ctl)
 {
 	u64 type = ctl->type;
-	u64 calc_size = ctl->calc_size;
+	u64 stripe_size = ctl->stripe_size;
 
 	if (type & (BTRFS_BLOCK_GROUP_RAID1 | BTRFS_BLOCK_GROUP_DUP))
-		return calc_size;
+		return stripe_size;
 	else if (type & (BTRFS_BLOCK_GROUP_RAID1C3 | BTRFS_BLOCK_GROUP_RAID1C4))
-		return calc_size;
+		return stripe_size;
 	else if (type & BTRFS_BLOCK_GROUP_RAID10)
-		return calc_size * (ctl->num_stripes / ctl->sub_stripes);
+		return stripe_size * (ctl->num_stripes / ctl->sub_stripes);
 	else if (type & BTRFS_BLOCK_GROUP_RAID5)
-		return calc_size * (ctl->num_stripes - 1);
+		return stripe_size * (ctl->num_stripes - 1);
 	else if (type & BTRFS_BLOCK_GROUP_RAID6)
-		return calc_size * (ctl->num_stripes - 2);
+		return stripe_size * (ctl->num_stripes - 2);
 	else
-		return calc_size * ctl->num_stripes;
+		return stripe_size * ctl->num_stripes;
 }
 
 /*
@@ -1020,13 +1020,13 @@ static void init_alloc_chunk_ctl_policy_regular(struct btrfs_fs_info *info,
 
 	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
 		if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
-			ctl->calc_size = SZ_8M;
-			ctl->max_chunk_size = ctl->calc_size * 2;
+			ctl->stripe_size = SZ_8M;
+			ctl->max_chunk_size = ctl->stripe_size * 2;
 			ctl->min_stripe_size = SZ_1M;
 			ctl->max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
 		} else if (type & BTRFS_BLOCK_GROUP_DATA) {
-			ctl->calc_size = SZ_1G;
-			ctl->max_chunk_size = 10 * ctl->calc_size;
+			ctl->stripe_size = SZ_1G;
+			ctl->max_chunk_size = 10 * ctl->stripe_size;
 			ctl->min_stripe_size = SZ_64M;
 			ctl->max_stripes = BTRFS_MAX_DEVS(info);
 		} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
@@ -1035,7 +1035,7 @@ static void init_alloc_chunk_ctl_policy_regular(struct btrfs_fs_info *info,
 				ctl->max_chunk_size = SZ_1G;
 			else
 				ctl->max_chunk_size = SZ_256M;
-			ctl->calc_size = ctl->max_chunk_size;
+			ctl->stripe_size = ctl->max_chunk_size;
 			ctl->min_stripe_size = SZ_32M;
 			ctl->max_stripes = BTRFS_MAX_DEVS(info);
 		}
@@ -1055,9 +1055,9 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 	ctl->min_stripes = btrfs_raid_array[type].devs_min;
 	ctl->max_stripes = 0;
 	ctl->sub_stripes = btrfs_raid_array[type].sub_stripes;
-	ctl->calc_size = SZ_8M;
+	ctl->stripe_size = SZ_8M;
 	ctl->min_stripe_size = SZ_1M;
-	ctl->max_chunk_size = 4 * ctl->calc_size;
+	ctl->max_chunk_size = 4 * ctl->stripe_size;
 	ctl->total_devs = btrfs_super_num_devices(info->super_copy);
 	ctl->dev_offset = 0;
 
@@ -1094,15 +1094,15 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl)
 {
 	if (chunk_bytes_by_type(ctl) > ctl->max_chunk_size) {
-		ctl->calc_size = ctl->max_chunk_size;
-		ctl->calc_size /= ctl->num_stripes;
-		ctl->calc_size = round_down(ctl->calc_size, BTRFS_STRIPE_LEN);
+		ctl->stripe_size = ctl->max_chunk_size;
+		ctl->stripe_size /= ctl->num_stripes;
+		ctl->stripe_size = round_down(ctl->stripe_size, BTRFS_STRIPE_LEN);
 	}
 	/* we don't want tiny stripes */
-	ctl->calc_size = max_t(u64, ctl->calc_size, ctl->min_stripe_size);
+	ctl->stripe_size = max_t(u64, ctl->stripe_size, ctl->min_stripe_size);
 
 	/* Align to the stripe length */
-	ctl->calc_size = round_down(ctl->calc_size, BTRFS_STRIPE_LEN);
+	ctl->stripe_size = round_down(ctl->stripe_size, BTRFS_STRIPE_LEN);
 
 	return 0;
 }
@@ -1175,18 +1175,18 @@ static int create_chunk(struct btrfs_trans_handle *trans,
 
 		if (!ctl->dev_offset) {
 			ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-				ctl->calc_size, &dev_offset);
+				ctl->stripe_size, &dev_offset);
 			if (ret < 0)
 				goto out_chunk_map;
 		} else {
 			dev_offset = ctl->dev_offset;
 			ret = btrfs_insert_dev_extent(trans, device, key.offset,
-						      ctl->calc_size,
+						      ctl->stripe_size,
 						      ctl->dev_offset);
 			BUG_ON(ret);
 		}
 
-		device->bytes_used += ctl->calc_size;
+		device->bytes_used += ctl->stripe_size;
 		ret = btrfs_update_device(trans, device);
 		if (ret < 0)
 			goto out_chunk_map;
@@ -1285,9 +1285,9 @@ again:
 	index = 0;
 
 	if (type & BTRFS_BLOCK_GROUP_DUP)
-		min_free = ctl.calc_size * 2;
+		min_free = ctl.stripe_size * 2;
 	else
-		min_free = ctl.calc_size;
+		min_free = ctl.stripe_size;
 
 	/* build a private list of devices we will allocate from */
 	while (index < ctl.num_stripes) {
@@ -1322,9 +1322,9 @@ again:
 		if (!looped && max_avail > 0) {
 			looped = 1;
 			if (ctl.type & BTRFS_BLOCK_GROUP_DUP)
-				ctl.calc_size = max_avail / 2;
+				ctl.stripe_size = max_avail / 2;
 			else
-				ctl.calc_size = max_avail;
+				ctl.stripe_size = max_avail;
 			goto again;
 		}
 		return -ENOSPC;
@@ -1363,7 +1363,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	ctl.max_stripes = 1;
 	ctl.min_stripes = 1;
 	ctl.sub_stripes = 1;
-	ctl.calc_size = num_bytes;
+	ctl.stripe_size = num_bytes;
 	ctl.min_stripe_size = num_bytes;
 	ctl.num_bytes = num_bytes;
 	ctl.max_chunk_size = num_bytes;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (11 preceding siblings ...)
  2021-04-06  8:05 ` [PATCH 12/12] btrfs-progs: rename calc_size to stripe_size Naohiro Aota
@ 2021-04-06  8:28 ` Johannes Thumshirn
  2021-04-06 10:54 ` Su Yue
  2021-04-29 14:20 ` David Sterba
  14 siblings, 0 replies; 18+ messages in thread
From: Johannes Thumshirn @ 2021-04-06  8:28 UTC (permalink / raw)
  To: Naohiro Aota, David Sterba; +Cc: linux-btrfs

On 06/04/2021 10:07, Naohiro Aota wrote:
> This is the userland counterpart of the following series.
> 
> https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/
> 
> This series refactors chunk allocation and device_extent allocation
> functions and make them generalized to be able to implement other
> allocation policy easily.
> 
> On top of this series, we can simplify userland side of the zoned series as
> adding a new type of chunk allocator and extent allocator for zoned block
> devices. Furthermore, we will be able to implement and test some other
> allocator in the idea page of the wiki e.g. SSD caching, dedicated metadata
> drive, chunk allocation groups, and so on.
> 
> This series also fixes a bug of calculating the stripe size in DUP profile,
> and cleans up the code.
> 
> * Refactoring chunk/dev_extent allocator
> 
> Two functions are separated from find_free_dev_extent_start().
> dev_extent_search_start() decides the starting position of the search.
> dev_extent_hole_check() checks if a hole found is suitable for device
> extent allocation.
> 
> Split some parts of btrfs_alloc_chunk() into three functions.
> init_alloc_chunk_policy() initializes the parameters of an allocation.
> decide_stripe_size() decides the size of chunk and device_extent. And,
> create_chunk() creates a chunk and device extents.
> 
> * Patch organization
> 
> Patches 1 and 2 refactor find_free_dev_extent_start().
> 
> Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the code into three
> other functions.
> 
> Patch 7 uses create_chunk() to simplify btrfs_alloc_data_chunk().
> 
> Patch 8 fixes a bug of calculating stripe size in DUP profile.
> 
> Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping unnecessary
> parameters, and using better macro/variable name to clarify the meaning.
> 

For the whole series,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (12 preceding siblings ...)
  2021-04-06  8:28 ` [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Johannes Thumshirn
@ 2021-04-06 10:54 ` Su Yue
  2021-04-06 13:24   ` Naohiro Aota
  2021-04-29 14:20 ` David Sterba
  14 siblings, 1 reply; 18+ messages in thread
From: Su Yue @ 2021-04-06 10:54 UTC (permalink / raw)
  To: Naohiro Aota; +Cc: David Sterba, linux-btrfs


On Tue 06 Apr 2021 at 16:05, Naohiro Aota <naohiro.aota@wdc.com> 
wrote:

> This is the userland counterpart of the following series.
>
> https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/
>
> This series refactors chunk allocation and device_extent 
> allocation
> functions and make them generalized to be able to implement 
> other
> allocation policy easily.
>
> On top of this series, we can simplify userland side of the 
> zoned series as
> adding a new type of chunk allocator and extent allocator for 
> zoned block
> devices. Furthermore, we will be able to implement and test some 
> other
> allocator in the idea page of the wiki e.g. SSD caching, 
> dedicated metadata
> drive, chunk allocation groups, and so on.
>
> This series also fixes a bug of calculating the stripe size in 
> DUP profile,
> and cleans up the code.
>
> * Refactoring chunk/dev_extent allocator
>
> Two functions are separated from find_free_dev_extent_start().
> dev_extent_search_start() decides the starting position of the 
> search.
> dev_extent_hole_check() checks if a hole found is suitable for 
> device
> extent allocation.
>
> Split some parts of btrfs_alloc_chunk() into three functions.
> init_alloc_chunk_policy() initializes the parameters of an 
> allocation.
> decide_stripe_size() decides the size of chunk and 
> device_extent. And,
> create_chunk() creates a chunk and device extents.
>
> * Patch organization
>
> Patches 1 and 2 refactor find_free_dev_extent_start().
>
> Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the code 
> into three
> other functions.
>
> Patch 7 uses create_chunk() to simplify 
> btrfs_alloc_data_chunk().
>
> Patch 8 fixes a bug of calculating stripe size in DUP profile.
>
> Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping 
> unnecessary
> parameters, and using better macro/variable name to clarify the 
> meaning.
>
>
gcc10 complains following warnings:
kernel-shared/volumes.c: In function ‘decide_stripe_size’:
kernel-shared/volumes.c:1119:1: warning: control reaches end of 
non-void function [-Wreturn-type]
 1119 | }
      | ^
kernel-shared/volumes.c: In function ‘dev_extent_search_start’:
kernel-shared/volumes.c:465:1: warning: control reaches end of 
non-void function [-Wreturn-type]
  465 | }
      | ^

Looked at locations just two nits about 'switch'. Care to fix?

--
Su
> Naohiro Aota (12):
>   btrfs-progs: introduce chunk allocation policy
>   btrfs-progs: refactor find_free_dev_extent_start()
>   btrfs-progs: convert type of alloc_chunk_ctl::type
>   btrfs-progs: consolidate parameter initialization of regular 
>   allocator
>   btrfs-progs: factor out decide_stripe_size()
>   btrfs-progs: factor out create_chunk()
>   btrfs-progs: rewrite btrfs_alloc_data_chunk() using 
>   create_chunk()
>   btrfs-progs: fix to use half the available space for DUP 
>   profile
>   btrfs-progs: use round_down for allocation calcs
>   btrfs-progs: drop alloc_chunk_ctl::stripe_len
>   btrfs-progs: simplify arguments of chunk_bytes_by_type()
>   btrfs-progs: rename calc_size to stripe_size
>
>  kernel-shared/volumes.c | 514 
>  +++++++++++++++++++++-------------------
>  kernel-shared/volumes.h |   6 +
>  2 files changed, 274 insertions(+), 246 deletions(-)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation
  2021-04-06 10:54 ` Su Yue
@ 2021-04-06 13:24   ` Naohiro Aota
  2021-04-06 14:40     ` Su Yue
  0 siblings, 1 reply; 18+ messages in thread
From: Naohiro Aota @ 2021-04-06 13:24 UTC (permalink / raw)
  To: Su Yue; +Cc: David Sterba, linux-btrfs

On Tue, Apr 06, 2021 at 06:54:37PM +0800, Su Yue wrote:
> 
> On Tue 06 Apr 2021 at 16:05, Naohiro Aota <naohiro.aota@wdc.com> wrote:
> 
> > This is the userland counterpart of the following series.
> > 
> > https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/
> > 
> > This series refactors chunk allocation and device_extent allocation
> > functions and make them generalized to be able to implement other
> > allocation policy easily.
> > 
> > On top of this series, we can simplify userland side of the zoned series
> > as
> > adding a new type of chunk allocator and extent allocator for zoned
> > block
> > devices. Furthermore, we will be able to implement and test some other
> > allocator in the idea page of the wiki e.g. SSD caching, dedicated
> > metadata
> > drive, chunk allocation groups, and so on.
> > 
> > This series also fixes a bug of calculating the stripe size in DUP
> > profile,
> > and cleans up the code.
> > 
> > * Refactoring chunk/dev_extent allocator
> > 
> > Two functions are separated from find_free_dev_extent_start().
> > dev_extent_search_start() decides the starting position of the search.
> > dev_extent_hole_check() checks if a hole found is suitable for device
> > extent allocation.
> > 
> > Split some parts of btrfs_alloc_chunk() into three functions.
> > init_alloc_chunk_policy() initializes the parameters of an allocation.
> > decide_stripe_size() decides the size of chunk and device_extent. And,
> > create_chunk() creates a chunk and device extents.
> > 
> > * Patch organization
> > 
> > Patches 1 and 2 refactor find_free_dev_extent_start().
> > 
> > Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the code into
> > three
> > other functions.
> > 
> > Patch 7 uses create_chunk() to simplify btrfs_alloc_data_chunk().
> > 
> > Patch 8 fixes a bug of calculating stripe size in DUP profile.
> > 
> > Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping
> > unnecessary
> > parameters, and using better macro/variable name to clarify the meaning.
> > 
> > 
> gcc10 complains following warnings:
> kernel-shared/volumes.c: In function ‘decide_stripe_size’:
> kernel-shared/volumes.c:1119:1: warning: control reaches end of non-void
> function [-Wreturn-type]
> 1119 | }
>      | ^
> kernel-shared/volumes.c: In function ‘dev_extent_search_start’:
> kernel-shared/volumes.c:465:1: warning: control reaches end of non-void
> function [-Wreturn-type]
>  465 | }
>      | ^
> 
> Looked at locations just two nits about 'switch'. Care to fix?

These are actually false-positve warnings. They never reach the end of
the function because BUG() in the default case will abort the program.

The warnings are showing up because the BUG() macro is not marked as
unreachable. This is addressed in the following patch. So, once the
following patch is merged, the warnings will disappear.

https://lore.kernel.org/linux-btrfs/5c7b703beca572514a28677df0caaafab28bfff8.1617265419.git.naohiro.aota@wdc.com/T/#u

> --
> Su
> > Naohiro Aota (12):
> >   btrfs-progs: introduce chunk allocation policy
> >   btrfs-progs: refactor find_free_dev_extent_start()
> >   btrfs-progs: convert type of alloc_chunk_ctl::type
> >   btrfs-progs: consolidate parameter initialization of regular
> > allocator
> >   btrfs-progs: factor out decide_stripe_size()
> >   btrfs-progs: factor out create_chunk()
> >   btrfs-progs: rewrite btrfs_alloc_data_chunk() using   create_chunk()
> >   btrfs-progs: fix to use half the available space for DUP   profile
> >   btrfs-progs: use round_down for allocation calcs
> >   btrfs-progs: drop alloc_chunk_ctl::stripe_len
> >   btrfs-progs: simplify arguments of chunk_bytes_by_type()
> >   btrfs-progs: rename calc_size to stripe_size
> > 
> >  kernel-shared/volumes.c | 514  +++++++++++++++++++++-------------------
> >  kernel-shared/volumes.h |   6 +
> >  2 files changed, 274 insertions(+), 246 deletions(-)
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation
  2021-04-06 13:24   ` Naohiro Aota
@ 2021-04-06 14:40     ` Su Yue
  0 siblings, 0 replies; 18+ messages in thread
From: Su Yue @ 2021-04-06 14:40 UTC (permalink / raw)
  To: Naohiro Aota; +Cc: David Sterba, linux-btrfs


On Tue 06 Apr 2021 at 21:24, Naohiro Aota <naohiro.aota@wdc.com> 
wrote:

> On Tue, Apr 06, 2021 at 06:54:37PM +0800, Su Yue wrote:
>>
>> On Tue 06 Apr 2021 at 16:05, Naohiro Aota 
>> <naohiro.aota@wdc.com> wrote:
>>
>> > This is the userland counterpart of the following series.
>> >
>> > https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/
>> >
>> > This series refactors chunk allocation and device_extent 
>> > allocation
>> > functions and make them generalized to be able to implement 
>> > other
>> > allocation policy easily.
>> >
>> > On top of this series, we can simplify userland side of the 
>> > zoned series
>> > as
>> > adding a new type of chunk allocator and extent allocator for 
>> > zoned
>> > block
>> > devices. Furthermore, we will be able to implement and test 
>> > some other
>> > allocator in the idea page of the wiki e.g. SSD caching, 
>> > dedicated
>> > metadata
>> > drive, chunk allocation groups, and so on.
>> >
>> > This series also fixes a bug of calculating the stripe size 
>> > in DUP
>> > profile,
>> > and cleans up the code.
>> >
>> > * Refactoring chunk/dev_extent allocator
>> >
>> > Two functions are separated from 
>> > find_free_dev_extent_start().
>> > dev_extent_search_start() decides the starting position of 
>> > the search.
>> > dev_extent_hole_check() checks if a hole found is suitable 
>> > for device
>> > extent allocation.
>> >
>> > Split some parts of btrfs_alloc_chunk() into three functions.
>> > init_alloc_chunk_policy() initializes the parameters of an 
>> > allocation.
>> > decide_stripe_size() decides the size of chunk and 
>> > device_extent. And,
>> > create_chunk() creates a chunk and device extents.
>> >
>> > * Patch organization
>> >
>> > Patches 1 and 2 refactor find_free_dev_extent_start().
>> >
>> > Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the 
>> > code into
>> > three
>> > other functions.
>> >
>> > Patch 7 uses create_chunk() to simplify 
>> > btrfs_alloc_data_chunk().
>> >
>> > Patch 8 fixes a bug of calculating stripe size in DUP 
>> > profile.
>> >
>> > Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping
>> > unnecessary
>> > parameters, and using better macro/variable name to clarify 
>> > the meaning.
>> >
>> >
>> gcc10 complains following warnings:
>> kernel-shared/volumes.c: In function ‘decide_stripe_size’:
>> kernel-shared/volumes.c:1119:1: warning: control reaches end of 
>> non-void
>> function [-Wreturn-type]
>> 1119 | }
>>      | ^
>> kernel-shared/volumes.c: In function ‘dev_extent_search_start’:
>> kernel-shared/volumes.c:465:1: warning: control reaches end of 
>> non-void
>> function [-Wreturn-type]
>>  465 | }
>>      | ^
>>
>> Looked at locations just two nits about 'switch'. Care to fix?
>
> These are actually false-positve warnings. They never reach the 
> end of
> the function because BUG() in the default case will abort the 
> program.
>
> The warnings are showing up because the BUG() macro is not 
> marked as
> unreachable. This is addressed in the following patch. So, once 
> the
> following patch is merged, the warnings will disappear.
>
I see. Thanks.

--
Su
> https://lore.kernel.org/linux-btrfs/5c7b703beca572514a28677df0caaafab28bfff8.1617265419.git.naohiro.aota@wdc.com/T/#u
>
>> --
>> Su
>> > Naohiro Aota (12):
>> >   btrfs-progs: introduce chunk allocation policy
>> >   btrfs-progs: refactor find_free_dev_extent_start()
>> >   btrfs-progs: convert type of alloc_chunk_ctl::type
>> >   btrfs-progs: consolidate parameter initialization of 
>> >   regular
>> > allocator
>> >   btrfs-progs: factor out decide_stripe_size()
>> >   btrfs-progs: factor out create_chunk()
>> >   btrfs-progs: rewrite btrfs_alloc_data_chunk() using 
>> >   create_chunk()
>> >   btrfs-progs: fix to use half the available space for DUP 
>> >   profile
>> >   btrfs-progs: use round_down for allocation calcs
>> >   btrfs-progs: drop alloc_chunk_ctl::stripe_len
>> >   btrfs-progs: simplify arguments of chunk_bytes_by_type()
>> >   btrfs-progs: rename calc_size to stripe_size
>> >
>> >  kernel-shared/volumes.c | 514 
>> >  +++++++++++++++++++++-------------------
>> >  kernel-shared/volumes.h |   6 +
>> >  2 files changed, 274 insertions(+), 246 deletions(-)
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation
  2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
                   ` (13 preceding siblings ...)
  2021-04-06 10:54 ` Su Yue
@ 2021-04-29 14:20 ` David Sterba
  14 siblings, 0 replies; 18+ messages in thread
From: David Sterba @ 2021-04-29 14:20 UTC (permalink / raw)
  To: Naohiro Aota; +Cc: David Sterba, linux-btrfs

On Tue, Apr 06, 2021 at 05:05:42PM +0900, Naohiro Aota wrote:
> This is the userland counterpart of the following series.
> 
> https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/
> 
> This series refactors chunk allocation and device_extent allocation
> functions and make them generalized to be able to implement other
> allocation policy easily.
> 
> On top of this series, we can simplify userland side of the zoned series as
> adding a new type of chunk allocator and extent allocator for zoned block
> devices. Furthermore, we will be able to implement and test some other
> allocator in the idea page of the wiki e.g. SSD caching, dedicated metadata
> drive, chunk allocation groups, and so on.
> 
> This series also fixes a bug of calculating the stripe size in DUP profile,
> and cleans up the code.
> 
> * Refactoring chunk/dev_extent allocator
> 
> Two functions are separated from find_free_dev_extent_start().
> dev_extent_search_start() decides the starting position of the search.
> dev_extent_hole_check() checks if a hole found is suitable for device
> extent allocation.
> 
> Split some parts of btrfs_alloc_chunk() into three functions.
> init_alloc_chunk_policy() initializes the parameters of an allocation.
> decide_stripe_size() decides the size of chunk and device_extent. And,
> create_chunk() creates a chunk and device extents.
> 
> * Patch organization
> 
> Patches 1 and 2 refactor find_free_dev_extent_start().
> 
> Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the code into three
> other functions.
> 
> Patch 7 uses create_chunk() to simplify btrfs_alloc_data_chunk().
> 
> Patch 8 fixes a bug of calculating stripe size in DUP profile.
> 
> Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping unnecessary
> parameters, and using better macro/variable name to clarify the meaning.
> 
> 
> Naohiro Aota (12):
>   btrfs-progs: introduce chunk allocation policy
>   btrfs-progs: refactor find_free_dev_extent_start()
>   btrfs-progs: convert type of alloc_chunk_ctl::type
>   btrfs-progs: consolidate parameter initialization of regular allocator
>   btrfs-progs: factor out decide_stripe_size()
>   btrfs-progs: factor out create_chunk()
>   btrfs-progs: rewrite btrfs_alloc_data_chunk() using create_chunk()
>   btrfs-progs: fix to use half the available space for DUP profile
>   btrfs-progs: use round_down for allocation calcs
>   btrfs-progs: drop alloc_chunk_ctl::stripe_len
>   btrfs-progs: simplify arguments of chunk_bytes_by_type()
>   btrfs-progs: rename calc_size to stripe_size

Added to devel, thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-04-29 14:23 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06  8:05 [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Naohiro Aota
2021-04-06  8:05 ` [PATCH 01/12] btrfs-progs: introduce chunk allocation policy Naohiro Aota
2021-04-06  8:05 ` [PATCH 02/12] btrfs-progs: refactor find_free_dev_extent_start() Naohiro Aota
2021-04-06  8:05 ` [PATCH 03/12] btrfs-progs: convert type of alloc_chunk_ctl::type Naohiro Aota
2021-04-06  8:05 ` [PATCH 04/12] btrfs-progs: consolidate parameter initialization of regular allocator Naohiro Aota
2021-04-06  8:05 ` [PATCH 05/12] btrfs-progs: factor out decide_stripe_size() Naohiro Aota
2021-04-06  8:05 ` [PATCH 06/12] btrfs-progs: factor out create_chunk() Naohiro Aota
2021-04-06  8:05 ` [PATCH 07/12] btrfs-progs: rewrite btrfs_alloc_data_chunk() using create_chunk() Naohiro Aota
2021-04-06  8:05 ` [PATCH 08/12] btrfs-progs: fix to use half the available space for DUP profile Naohiro Aota
2021-04-06  8:05 ` [PATCH 09/12] btrfs-progs: use round_down for allocation calcs Naohiro Aota
2021-04-06  8:05 ` [PATCH 10/12] btrfs-progs: drop alloc_chunk_ctl::stripe_len Naohiro Aota
2021-04-06  8:05 ` [PATCH 11/12] btrfs-progs: simplify arguments of chunk_bytes_by_type() Naohiro Aota
2021-04-06  8:05 ` [PATCH 12/12] btrfs-progs: rename calc_size to stripe_size Naohiro Aota
2021-04-06  8:28 ` [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation Johannes Thumshirn
2021-04-06 10:54 ` Su Yue
2021-04-06 13:24   ` Naohiro Aota
2021-04-06 14:40     ` Su Yue
2021-04-29 14:20 ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.