[PATCH v6 0/4] btrfs: sysfs: set / query btrfs chunk size

[PATCH v6 0/4] btrfs: sysfs: set / query btrfs chunk size

[Stefan Roesch]

The btrfs allocator is currently not ideal for all workloads. It tends
to suffer from overallocating data block groups and underallocating
metadata block groups. This results in filesystems becoming read-only
even though there is plenty of "free" space.

This is naturally confusing and distressing to users.

Patches:
1) Store the chunk size in the btrfs_space_info structure
2) Add a sysfs entry to read and write the above information
3) Add a sysfs entry to force a space allocation
4) Increase the default size of the metadata chunk allocation to 5GB
   for volumes greater than 50GB.

Testing:
  A new test is being added to the xfstest suite. For reference the
  corresponding patch has the title:
    [PATCH] btrfs: Test chunk allocation with different sizes

  In addition also manual testing has been performed.
    - Run xfstests with the changes and the new test. It does not
      show new diffs.
    - Test with storage devices 10G, 20G, 30G, 50G, 60G
      - Default allocation
      - Increase of chunk size
      - If the stripe size is > the free space, it allocates
        free space - 1MB. The 1MB is left as free space.
      - If the device has a storage size > 50G, it uses a 5GB
        chunk size for new allocations.

---
v6: - Update btrfs_force_chunk_alloc_store to use tree_root
      instead of extent_root.
V5: - Changed the field name in the btrfs_space_info struct from
      default_chunk_size to chunk_size and made it atomic
    - Removed the compute_chunk_size_zoned function
    - Added further checks when writing /sys/fs/btrfs/<id>/allocation/<type>/chunk_size
    - Removed the ability to query /sys/fs/btrfs/<id>/allocation/<type>/force_alloc_chunk

V4: - Patch email contained duplicate entries.

V3: - Rename sysfs entry from stripe_size to chunk_size
    - Remove max_chunk_size field in data structure btrfs_space_info
    - Rename max_stripe_size field to default_chunk_size in data
      structure btrfs_space_info
    - Remove max_chunk_size logic
    - Use stripe_size = chunk_size

V2:
   - Split the patch in 4 patches
   - Added checks for zone volumes in sysfs.c
   - Replaced the BUG() with ASSERT()
   - Changed if with fallthrough
   - Removed comments in space-info.h
--
Stefan Roesch (4):
  btrfs: store chunk size in space-info struct.
  btrfs: expose chunk size in sysfs.
  btrfs: add force_chunk_alloc sysfs entry to force allocation
  btrfs: increase metadata alloc size to 5GB for volumes > 50GB

 fs/btrfs/space-info.c |  41 ++++++++++++
 fs/btrfs/space-info.h |   3 +
 fs/btrfs/sysfs.c      | 152 ++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.c    |  28 +++-----
 4 files changed, 205 insertions(+), 19 deletions(-)


Signed-off-by: Stefan Roesch <shr@fb.com>
base-commit: 87c673b657a7e4784fb7274a77a8529712232d0c
-- 
2.30.2

[PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Stefan Roesch]

The chunk size is stored in the btrfs_space_info structure.
It is initialized at the start and is then used.

A new api is added to update the current chunk size.

This api is used to be able to expose the chunk_size
as a sysfs setting.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/btrfs/space-info.c | 41 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/space-info.h |  3 +++
 fs/btrfs/volumes.c    | 28 +++++++++-------------------
 3 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 79fe0ad17acf..437d1240f491 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -181,6 +181,46 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info)
 		found->full = 0;
 }
 
+/*
+ * Compute chunk size depending on block type for regular volumes.
+ */
+static u64 compute_chunk_size_regular(struct btrfs_fs_info *info, u64 flags)
+{
+	ASSERT(flags & BTRFS_BLOCK_GROUP_TYPE_MASK);
+
+	if (flags & BTRFS_BLOCK_GROUP_DATA)
+		return SZ_1G;
+	else if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
+		return SZ_32M;
+
+	/* Handle BTRFS_BLOCK_GROUP_METADATA */
+	if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
+		return SZ_1G;
+
+	return SZ_256M;
+}
+
+/*
+ * Compute chunk size depending on volume type.
+ */
+static u64 compute_chunk_size(struct btrfs_fs_info *info, u64 flags)
+{
+	if (btrfs_is_zoned(info))
+		return info->zone_size;
+
+	return compute_chunk_size_regular(info, flags);
+}
+
+/*
+ * Update default chunk size.
+ *
+ */
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size)
+{
+	atomic64_set(&space_info->chunk_size, chunk_size);
+}
+
 static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 
@@ -202,6 +242,7 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 	INIT_LIST_HEAD(&space_info->tickets);
 	INIT_LIST_HEAD(&space_info->priority_tickets);
 	space_info->clamp = 1;
+	btrfs_update_space_info_chunk_size(space_info, compute_chunk_size(info, flags));
 
 	ret = btrfs_sysfs_add_space_info_type(info, space_info);
 	if (ret)
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index d841fed73492..c1a64bbfb6d1 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -23,6 +23,7 @@ struct btrfs_space_info {
 	u64 max_extent_size;	/* This will hold the maximum extent size of
 				   the space info if we had an ENOSPC in the
 				   allocator. */
+	atomic64_t chunk_size;  /* chunk size in bytes */
 
 	int clamp;		/* Used to scale our threshold for preemptive
 				   flushing. The value is >> clamp, so turns
@@ -115,6 +116,8 @@ void btrfs_update_space_info(struct btrfs_fs_info *info, u64 flags,
 			     u64 total_bytes, u64 bytes_used,
 			     u64 bytes_readonly, u64 bytes_zone_unusable,
 			     struct btrfs_space_info **space_info);
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+			     u64 chunk_size);
 struct btrfs_space_info *btrfs_find_space_info(struct btrfs_fs_info *info,
 					       u64 flags);
 u64 __pure btrfs_space_info_used(struct btrfs_space_info *s_info,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f38c230111be..546c39551648 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5106,26 +5106,16 @@ static void init_alloc_chunk_ctl_policy_regular(
 				struct btrfs_fs_devices *fs_devices,
 				struct alloc_chunk_ctl *ctl)
 {
-	u64 type = ctl->type;
+	struct btrfs_space_info *space_info;
 
-	if (type & BTRFS_BLOCK_GROUP_DATA) {
-		ctl->max_stripe_size = SZ_1G;
-		ctl->max_chunk_size = BTRFS_MAX_DATA_CHUNK_SIZE;
-	} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
-		/* For larger filesystems, use larger metadata chunks */
-		if (fs_devices->total_rw_bytes > 50ULL * SZ_1G)
-			ctl->max_stripe_size = SZ_1G;
-		else
-			ctl->max_stripe_size = SZ_256M;
-		ctl->max_chunk_size = ctl->max_stripe_size;
-	} else if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
-		ctl->max_stripe_size = SZ_32M;
-		ctl->max_chunk_size = 2 * ctl->max_stripe_size;
-		ctl->devs_max = min_t(int, ctl->devs_max,
-				      BTRFS_MAX_DEVS_SYS_CHUNK);
-	} else {
-		BUG();
-	}
+	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
+	ASSERT(space_info);
+
+	ctl->max_chunk_size = atomic64_read(&space_info->chunk_size);
+	ctl->max_stripe_size = ctl->max_chunk_size;
+
+	if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM)
+		ctl->devs_max = min_t(int, ctl->devs_max, BTRFS_MAX_DEVS_SYS_CHUNK);
 
 	/* We don't want a chunk larger than 10% of writable space */
 	ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),
-- 
2.30.2

[PATCH v6 2/4] btrfs: expose chunk size in sysfs.

[Stefan Roesch]

This adds a new sysfs entry in /sys/fs/btrfs/<uuid>/allocation/<block
type>/chunk_size. This allows to query the chunk size and also set the
chunk size.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/btrfs/sysfs.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index f9eff3b0f77c..39890ea21997 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -21,6 +21,7 @@
 #include "space-info.h"
 #include "block-group.h"
 #include "qgroup.h"
+#include "misc.h"
 
 /*
  * Structure name                       Path
@@ -92,6 +93,7 @@ static struct btrfs_feature_attr btrfs_attr_features_##_name = {	     \
 
 static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj);
 static inline struct btrfs_fs_devices *to_fs_devs(struct kobject *kobj);
+static inline struct kobject *get_btrfs_kobj(struct kobject *kobj);
 
 static struct btrfs_feature_attr *to_btrfs_feature_attr(struct kobj_attribute *a)
 {
@@ -708,6 +710,81 @@ static ssize_t btrfs_space_info_show_##field(struct kobject *kobj,	\
 }									\
 BTRFS_ATTR(space_info, field, btrfs_space_info_show_##field)
 
+/*
+ * Return space info chunk size.
+ */
+static ssize_t btrfs_chunk_size_show(struct kobject *kobj,
+				     struct kobj_attribute *a, char *buf)
+{
+	struct btrfs_space_info *sinfo = to_space_info(kobj);
+
+	return sysfs_emit(buf, "%llu\n", atomic64_read(&sinfo->chunk_size));
+}
+
+/*
+ * Store new user supplied chunk size in space info.
+ *
+ * Note: If the new chunk size value is larger than 10% of free space it is
+ *       reduced to match that limit.
+ */
+static ssize_t btrfs_chunk_size_store(struct kobject *kobj,
+				      struct kobj_attribute *a,
+				      const char *buf, size_t len)
+{
+	struct btrfs_space_info *space_info = to_space_info(kobj);
+	struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj));
+	u64 val;
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (!fs_info) {
+		pr_err("couldn't get fs_info\n");
+		return -EPERM;
+	}
+
+	if (sb_rdonly(fs_info->sb))
+		return -EROFS;
+
+	if (!fs_info->fs_devices)
+		return -EINVAL;
+
+	if (btrfs_is_zoned(fs_info))
+		return -EINVAL;
+
+	if (!space_info) {
+		btrfs_err(fs_info, "couldn't get space_info\n");
+		return -EPERM;
+	}
+
+	/* System block type must not be changed. */
+	if (space_info->flags & BTRFS_BLOCK_GROUP_SYSTEM)
+		return -EINVAL;
+
+	ret = kstrtoull(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	val = min(val, BTRFS_MAX_DATA_CHUNK_SIZE);
+
+	/*
+	 * Limit stripe size to 10% of available space.
+	 */
+	val = min(div_factor(fs_info->fs_devices->total_rw_bytes, 1), val);
+
+	/* Must be multiple of 256M. */
+	val &= ~(SZ_256M - 1);
+
+	/* Must be at least 256M. */
+	if (val < SZ_256M)
+		return -EINVAL;
+
+	btrfs_update_space_info_chunk_size(space_info, val);
+
+	return val;
+}
+
 SPACE_INFO_ATTR(flags);
 SPACE_INFO_ATTR(total_bytes);
 SPACE_INFO_ATTR(bytes_used);
@@ -718,6 +795,8 @@ SPACE_INFO_ATTR(bytes_readonly);
 SPACE_INFO_ATTR(bytes_zone_unusable);
 SPACE_INFO_ATTR(disk_used);
 SPACE_INFO_ATTR(disk_total);
+BTRFS_ATTR_RW(space_info, chunk_size, btrfs_chunk_size_show,
+	      btrfs_chunk_size_store);
 
 /*
  * Allocation information about block group types.
@@ -735,6 +814,7 @@ static struct attribute *space_info_attrs[] = {
 	BTRFS_ATTR_PTR(space_info, bytes_zone_unusable),
 	BTRFS_ATTR_PTR(space_info, disk_used),
 	BTRFS_ATTR_PTR(space_info, disk_total),
+	BTRFS_ATTR_PTR(space_info, chunk_size),
 	NULL,
 };
 ATTRIBUTE_GROUPS(space_info);
@@ -1099,6 +1179,20 @@ static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj)
 	return to_fs_devs(kobj)->fs_info;
 }
 
+/*
+ * Get btrfs sysfs kobject.
+ */
+static inline struct kobject *get_btrfs_kobj(struct kobject *kobj)
+{
+	while (kobj) {
+		if (kobj->ktype == &btrfs_ktype)
+			return kobj;
+		kobj = kobj->parent;
+	}
+
+	return NULL;
+}
+
 #define NUM_FEATURE_BITS 64
 #define BTRFS_FEATURE_NAME_MAX 13
 static char btrfs_unknown_feature_names[FEAT_MAX][NUM_FEATURE_BITS][BTRFS_FEATURE_NAME_MAX];
-- 
2.30.2

[PATCH v6 3/4] btrfs: add force_chunk_alloc sysfs entry to force allocation

[Stefan Roesch]

This adds the force_chunk_alloc sysfs entry to be able to force a
storage allocation. This is a debugging and test feature and is
enabled with the CONFIG_BTRFS_DEBUG configuration option.

It is stored at
/sys/fs/btrfs/<uuid>/allocation/<block_type>/force_chunk_alloc.
---
 fs/btrfs/sysfs.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 39890ea21997..a421a95dea93 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -62,6 +62,10 @@ struct raid_kobject {
 	.store	= _store,						\
 }
 
+#define BTRFS_ATTR_W(_prefix, _name, _store)			        \
+	static struct kobj_attribute btrfs_attr_##_prefix##_##_name =	\
+			__INIT_KOBJ_ATTR(_name, 0200, NULL, _store)
+
 #define BTRFS_ATTR_RW(_prefix, _name, _show, _store)			\
 	static struct kobj_attribute btrfs_attr_##_prefix##_##_name =	\
 			__INIT_KOBJ_ATTR(_name, 0644, _show, _store)
@@ -785,6 +789,54 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj,
 	return val;
 }
 
+#ifdef CONFIG_BTRFS_DEBUG
+/*
+ * Request chunk allocation with current chunk size.
+ */
+static ssize_t btrfs_force_chunk_alloc_store(struct kobject *kobj,
+					     struct kobj_attribute *a,
+					     const char *buf, size_t len)
+{
+	struct btrfs_space_info *space_info = to_space_info(kobj);
+	struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj));
+	struct btrfs_trans_handle *trans;
+	unsigned long val;
+	int ret;
+
+	if (!fs_info) {
+		pr_err("couldn't get fs_info\n");
+		return -EPERM;
+	}
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (sb_rdonly(fs_info->sb))
+		return -EROFS;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val == 0)
+		return -EINVAL;
+
+	/*
+	 * Allocate new chunk.
+	 */
+	trans = btrfs_start_transaction(fs_info->tree_root, 0);
+	if (!trans)
+		return PTR_ERR(trans);
+	ret = btrfs_force_chunk_alloc(trans, space_info->flags);
+	btrfs_end_transaction(trans);
+
+	if (ret == 1)
+		return len;
+
+	return -ENOSPC;
+}
+#endif
+
 SPACE_INFO_ATTR(flags);
 SPACE_INFO_ATTR(total_bytes);
 SPACE_INFO_ATTR(bytes_used);
@@ -797,6 +849,9 @@ SPACE_INFO_ATTR(disk_used);
 SPACE_INFO_ATTR(disk_total);
 BTRFS_ATTR_RW(space_info, chunk_size, btrfs_chunk_size_show,
 	      btrfs_chunk_size_store);
+#ifdef CONFIG_BTRFS_DEBUG
+BTRFS_ATTR_W(space_info, force_chunk_alloc, btrfs_force_chunk_alloc_store);
+#endif
 
 /*
  * Allocation information about block group types.
@@ -815,6 +870,9 @@ static struct attribute *space_info_attrs[] = {
 	BTRFS_ATTR_PTR(space_info, disk_used),
 	BTRFS_ATTR_PTR(space_info, disk_total),
 	BTRFS_ATTR_PTR(space_info, chunk_size),
+#ifdef CONFIG_BTRFS_DEBUG
+	BTRFS_ATTR_PTR(space_info, force_chunk_alloc),
+#endif
 	NULL,
 };
 ATTRIBUTE_GROUPS(space_info);
-- 
2.30.2

[PATCH v6 4/4] btrfs: increase metadata alloc size to 5GB for volumes > 50GB

[Stefan Roesch]

This increases the metadata default allocation size from 1GB to 5GB for
volumes with a size greater than 50GB.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/btrfs/space-info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 437d1240f491..cda78aa1ee4f 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -195,7 +195,7 @@ static u64 compute_chunk_size_regular(struct btrfs_fs_info *info, u64 flags)
 
 	/* Handle BTRFS_BLOCK_GROUP_METADATA */
 	if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
-		return SZ_1G;
+		return 5ULL * SZ_1G;
 
 	return SZ_256M;
 }
-- 
2.30.2

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Wang Yugui]

Hi,

In this patch, the max chunk size is changed from 
BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/07/23

> The chunk size is stored in the btrfs_space_info structure.
> It is initialized at the start and is then used.
> 
> A new api is added to update the current chunk size.
> 
> This api is used to be able to expose the chunk_size
> as a sysfs setting.
> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>  fs/btrfs/space-info.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/space-info.h |  3 +++
>  fs/btrfs/volumes.c    | 28 +++++++++-------------------
>  3 files changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 79fe0ad17acf..437d1240f491 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -181,6 +181,46 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info)
>  		found->full = 0;
>  }
>  
> +/*
> + * Compute chunk size depending on block type for regular volumes.
> + */
> +static u64 compute_chunk_size_regular(struct btrfs_fs_info *info, u64 flags)
> +{
> +	ASSERT(flags & BTRFS_BLOCK_GROUP_TYPE_MASK);
> +
> +	if (flags & BTRFS_BLOCK_GROUP_DATA)
> +		return SZ_1G;
> +	else if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
> +		return SZ_32M;
> +
> +	/* Handle BTRFS_BLOCK_GROUP_METADATA */
> +	if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
> +		return SZ_1G;
> +
> +	return SZ_256M;
> +}
> +
> +/*
> + * Compute chunk size depending on volume type.
> + */
> +static u64 compute_chunk_size(struct btrfs_fs_info *info, u64 flags)
> +{
> +	if (btrfs_is_zoned(info))
> +		return info->zone_size;
> +
> +	return compute_chunk_size_regular(info, flags);
> +}
> +
> +/*
> + * Update default chunk size.
> + *
> + */
> +void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
> +					u64 chunk_size)
> +{
> +	atomic64_set(&space_info->chunk_size, chunk_size);
> +}
> +
>  static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>  {
>  
> @@ -202,6 +242,7 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>  	INIT_LIST_HEAD(&space_info->tickets);
>  	INIT_LIST_HEAD(&space_info->priority_tickets);
>  	space_info->clamp = 1;
> +	btrfs_update_space_info_chunk_size(space_info, compute_chunk_size(info, flags));
>  
>  	ret = btrfs_sysfs_add_space_info_type(info, space_info);
>  	if (ret)
> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
> index d841fed73492..c1a64bbfb6d1 100644
> --- a/fs/btrfs/space-info.h
> +++ b/fs/btrfs/space-info.h
> @@ -23,6 +23,7 @@ struct btrfs_space_info {
>  	u64 max_extent_size;	/* This will hold the maximum extent size of
>  				   the space info if we had an ENOSPC in the
>  				   allocator. */
> +	atomic64_t chunk_size;  /* chunk size in bytes */
>  
>  	int clamp;		/* Used to scale our threshold for preemptive
>  				   flushing. The value is >> clamp, so turns
> @@ -115,6 +116,8 @@ void btrfs_update_space_info(struct btrfs_fs_info *info, u64 flags,
>  			     u64 total_bytes, u64 bytes_used,
>  			     u64 bytes_readonly, u64 bytes_zone_unusable,
>  			     struct btrfs_space_info **space_info);
> +void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
> +			     u64 chunk_size);
>  struct btrfs_space_info *btrfs_find_space_info(struct btrfs_fs_info *info,
>  					       u64 flags);
>  u64 __pure btrfs_space_info_used(struct btrfs_space_info *s_info,
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f38c230111be..546c39551648 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5106,26 +5106,16 @@ static void init_alloc_chunk_ctl_policy_regular(
>  				struct btrfs_fs_devices *fs_devices,
>  				struct alloc_chunk_ctl *ctl)
>  {
> -	u64 type = ctl->type;
> +	struct btrfs_space_info *space_info;
>  
> -	if (type & BTRFS_BLOCK_GROUP_DATA) {
> -		ctl->max_stripe_size = SZ_1G;
> -		ctl->max_chunk_size = BTRFS_MAX_DATA_CHUNK_SIZE;
> -	} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
> -		/* For larger filesystems, use larger metadata chunks */
> -		if (fs_devices->total_rw_bytes > 50ULL * SZ_1G)
> -			ctl->max_stripe_size = SZ_1G;
> -		else
> -			ctl->max_stripe_size = SZ_256M;
> -		ctl->max_chunk_size = ctl->max_stripe_size;
> -	} else if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
> -		ctl->max_stripe_size = SZ_32M;
> -		ctl->max_chunk_size = 2 * ctl->max_stripe_size;
> -		ctl->devs_max = min_t(int, ctl->devs_max,
> -				      BTRFS_MAX_DEVS_SYS_CHUNK);
> -	} else {
> -		BUG();
> -	}
> +	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
> +	ASSERT(space_info);
> +
> +	ctl->max_chunk_size = atomic64_read(&space_info->chunk_size);
> +	ctl->max_stripe_size = ctl->max_chunk_size;
> +
> +	if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM)
> +		ctl->devs_max = min_t(int, ctl->devs_max, BTRFS_MAX_DEVS_SYS_CHUNK);
>  
>  	/* We don't want a chunk larger than 10% of writable space */
>  	ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),
> -- 
> 2.30.2

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[David Sterba]

On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> Hi,
> 
> In this patch, the max chunk size is changed from 
> BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?

The patch hasn't been merged, the change from 1G to 10G without proper
evaluation won't happen. The sysfs knob is available for users who want
to test it or know that the non-default value works in their
environment.

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Wang Yugui]

Hi,

> On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> > Hi,
> > 
> > In this patch, the max chunk size is changed from 
> > BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
> 
> The patch hasn't been merged, the change from 1G to 10G without proper
> evaluation won't happen. The sysfs knob is available for users who want
> to test it or know that the non-default value works in their
> environment.

this patch is in misc-next( 5.19.0-rc8 based, 5.19.0-rc7 based) now.

5.19.0-rc8 based:
f6fca3917b4d btrfs: store chunk size in space-info struct

The sysfs knob show that current default chunk size is 1G, not 10G as
older version.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/07/26

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[David Sterba]

On Tue, Jul 26, 2022 at 06:25:38AM +0800, Wang Yugui wrote:
> > On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> > > In this patch, the max chunk size is changed from 
> > > BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
> > 
> > The patch hasn't been merged, the change from 1G to 10G without proper
> > evaluation won't happen. The sysfs knob is available for users who want
> > to test it or know that the non-default value works in their
> > environment.
> 
> this patch is in misc-next( 5.19.0-rc8 based, 5.19.0-rc7 based) now.
> 
> 5.19.0-rc8 based:
> f6fca3917b4d btrfs: store chunk size in space-info struct
>
> The sysfs knob show that current default chunk size is 1G, not 10G as
> older version.

So there are two things regarding chunk size, the default size and that
it's settable by user (with some limitations). I was replying to the
default size change while you are concerned about the max_chunk_size.

You're right that the value changed in the patch, but as I'm reading the
code it should not have any effect. When user sets a value in
btrfs_chunk_size_store() it's limited inside the sysfs handler to the
10G. Also there are various adjustments when the chunk size is
initialized (init_alloc_chunk_ctl_policy_regular).

The only difference I can see comparing master and misc-next is in
decide_stripe_size_regular()

5259         /*
5260          * Use the number of data stripes to figure out how big this chunk is
5261          * really going to be in terms of logical address space, and compare
5262          * that answer with the max chunk size. If it's higher, we try to
5263          * reduce stripe_size.
5264          */
5265         if (ctl->stripe_size * data_stripes > ctl->max_chunk_size) {
^^^^
5266                 /*
5267                  * Reduce stripe_size, round it up to a 16MB boundary again and
5268                  * then use it, unless it ends up being even bigger than the
5269                  * previous value we had already.
5270                  */
5271                 ctl->stripe_size = min(round_up(div_u64(ctl->max_chunk_size,
5272                                                         data_stripes), SZ_16M),
5273                                        ctl->stripe_size);
5274         }

Here it could lead to a different stripe_size when max_chunk_size would
be 1G vs 10G, though the other adjustments could change the upper value.

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Wang Yugui]

[-- Attachment #1: Type: text/plain, Size: 2828 bytes --]

Hi,

I tried to fold my fix/comment into the attachement file(folded.patch).

Just the following is new, others are just orig feature or refactor.

+	if(ctl->max_stripe_size > ctl->max_chunk_size)
+		ctl->max_stripe_size = ctl->max_chunk_size;

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/07/29

> On Tue, Jul 26, 2022 at 06:25:38AM +0800, Wang Yugui wrote:
> > > On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> > > > In this patch, the max chunk size is changed from 
> > > > BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
> > > 
> > > The patch hasn't been merged, the change from 1G to 10G without proper
> > > evaluation won't happen. The sysfs knob is available for users who want
> > > to test it or know that the non-default value works in their
> > > environment.
> > 
> > this patch is in misc-next( 5.19.0-rc8 based, 5.19.0-rc7 based) now.
> > 
> > 5.19.0-rc8 based:
> > f6fca3917b4d btrfs: store chunk size in space-info struct
> >
> > The sysfs knob show that current default chunk size is 1G, not 10G as
> > older version.
> 
> So there are two things regarding chunk size, the default size and that
> it's settable by user (with some limitations). I was replying to the
> default size change while you are concerned about the max_chunk_size.
> 
> You're right that the value changed in the patch, but as I'm reading the
> code it should not have any effect. When user sets a value in
> btrfs_chunk_size_store() it's limited inside the sysfs handler to the
> 10G. Also there are various adjustments when the chunk size is
> initialized (init_alloc_chunk_ctl_policy_regular).
> 
> The only difference I can see comparing master and misc-next is in
> decide_stripe_size_regular()
> 
> 5259         /*
> 5260          * Use the number of data stripes to figure out how big this chunk is
> 5261          * really going to be in terms of logical address space, and compare
> 5262          * that answer with the max chunk size. If it's higher, we try to
> 5263          * reduce stripe_size.
> 5264          */
> 5265         if (ctl->stripe_size * data_stripes > ctl->max_chunk_size) {
> ^^^^
> 5266                 /*
> 5267                  * Reduce stripe_size, round it up to a 16MB boundary again and
> 5268                  * then use it, unless it ends up being even bigger than the
> 5269                  * previous value we had already.
> 5270                  */
> 5271                 ctl->stripe_size = min(round_up(div_u64(ctl->max_chunk_size,
> 5272                                                         data_stripes), SZ_16M),
> 5273                                        ctl->stripe_size);
> 5274         }
> 
> Here it could lead to a different stripe_size when max_chunk_size would
> be 1G vs 10G, though the other adjustments could change the upper value.


[-- Attachment #2: folded.patch --]
[-- Type: application/octet-stream, Size: 3429 bytes --]

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 2cf8da1116eb..bfe884f13934 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -187,6 +187,15 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info)
  */
 #define BTRFS_DEFAULT_ZONED_RECLAIM_THRESH			(75)
 
+/*
+ * Update default chunk size.
+ */
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size)
+{
+	WRITE_ONCE(space_info->chunk_size, chunk_size);
+}
+
 static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index c096695598c1..e7de24a529cf 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -25,6 +25,8 @@ struct btrfs_space_info {
 	u64 max_extent_size;	/* This will hold the maximum extent size of
 				   the space info if we had an ENOSPC in the
 				   allocator. */
+	/* Chunk size in bytes */
+	u64 chunk_size;
 
 	/*
 	 * Once a block group drops below this threshold (percents) we'll
@@ -123,6 +125,8 @@ void btrfs_update_space_info(struct btrfs_fs_info *info, u64 flags,
 			     u64 total_bytes, u64 bytes_used,
 			     u64 bytes_readonly, u64 bytes_zone_unusable,
 			     struct btrfs_space_info **space_info);
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size);
 struct btrfs_space_info *btrfs_find_space_info(struct btrfs_fs_info *info,
 					       u64 flags);
 u64 __pure btrfs_space_info_used(struct btrfs_space_info *s_info,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a2bb0928dc06..b7b7d254ecf0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5072,6 +5072,7 @@ static void init_alloc_chunk_ctl_policy_regular(
 				struct alloc_chunk_ctl *ctl)
 {
 	u64 type = ctl->type;
+	struct btrfs_space_info *space_info;
 
 	if (type & BTRFS_BLOCK_GROUP_DATA) {
 		ctl->max_stripe_size = SZ_1G;
@@ -5095,7 +5096,17 @@ static void init_alloc_chunk_ctl_policy_regular(
 	/* We don't want a chunk larger than 10% of writable space */
 	ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),
 				  ctl->max_chunk_size);
+	if(ctl->max_stripe_size > ctl->max_chunk_size)
+		ctl->max_stripe_size = ctl->max_chunk_size;
+
 	ctl->dev_extent_min = BTRFS_STRIPE_LEN * ctl->dev_stripes;
+
+	if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM)
+		ctl->devs_max = min_t(int, ctl->devs_max, BTRFS_MAX_DEVS_SYS_CHUNK);
+
+	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
+	ASSERT(space_info);
+	btrfs_update_space_info_chunk_size(space_info, ctl->max_chunk_size);
 }
 
 static void init_alloc_chunk_ctl_policy_zoned(
@@ -5108,6 +5119,7 @@ static void init_alloc_chunk_ctl_policy_zoned(
 	int min_data_stripes = (min_num_stripes - ctl->nparity) / ctl->ncopies;
 	u64 min_chunk_size = min_data_stripes * zone_size;
 	u64 type = ctl->type;
+	struct btrfs_space_info *space_info;
 
 	ctl->max_stripe_size = zone_size;
 	if (type & BTRFS_BLOCK_GROUP_DATA) {
@@ -5129,6 +5141,10 @@ static void init_alloc_chunk_ctl_policy_zoned(
 		    min_chunk_size);
 	ctl->max_chunk_size = min(limit, ctl->max_chunk_size);
 	ctl->dev_extent_min = zone_size * ctl->dev_stripes;
+
+	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
+	ASSERT(space_info);
+	btrfs_update_space_info_chunk_size(space_info, ctl->max_chunk_size);
 }
 
 static void init_alloc_chunk_ctl(struct btrfs_fs_devices *fs_devices,

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Wang Yugui]

[-- Attachment #1: Type: text/plain, Size: 3141 bytes --]

Hi,

attachement file(folded-v2.patch):

changes:
   move ASSERT(space_info) into btrfs_update_space_info_chunk_size();

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/07/29

> Hi,
> 
> I tried to fold my fix/comment into the attachement file(folded.patch).
> 
> Just the following is new, others are just orig feature or refactor.
> 
> +	if(ctl->max_stripe_size > ctl->max_chunk_size)
> +		ctl->max_stripe_size = ctl->max_chunk_size;
> 
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/07/29
> 
> > On Tue, Jul 26, 2022 at 06:25:38AM +0800, Wang Yugui wrote:
> > > > On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> > > > > In this patch, the max chunk size is changed from 
> > > > > BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
> > > > 
> > > > The patch hasn't been merged, the change from 1G to 10G without proper
> > > > evaluation won't happen. The sysfs knob is available for users who want
> > > > to test it or know that the non-default value works in their
> > > > environment.
> > > 
> > > this patch is in misc-next( 5.19.0-rc8 based, 5.19.0-rc7 based) now.
> > > 
> > > 5.19.0-rc8 based:
> > > f6fca3917b4d btrfs: store chunk size in space-info struct
> > >
> > > The sysfs knob show that current default chunk size is 1G, not 10G as
> > > older version.
> > 
> > So there are two things regarding chunk size, the default size and that
> > it's settable by user (with some limitations). I was replying to the
> > default size change while you are concerned about the max_chunk_size.
> > 
> > You're right that the value changed in the patch, but as I'm reading the
> > code it should not have any effect. When user sets a value in
> > btrfs_chunk_size_store() it's limited inside the sysfs handler to the
> > 10G. Also there are various adjustments when the chunk size is
> > initialized (init_alloc_chunk_ctl_policy_regular).
> > 
> > The only difference I can see comparing master and misc-next is in
> > decide_stripe_size_regular()
> > 
> > 5259         /*
> > 5260          * Use the number of data stripes to figure out how big this chunk is
> > 5261          * really going to be in terms of logical address space, and compare
> > 5262          * that answer with the max chunk size. If it's higher, we try to
> > 5263          * reduce stripe_size.
> > 5264          */
> > 5265         if (ctl->stripe_size * data_stripes > ctl->max_chunk_size) {
> > ^^^^
> > 5266                 /*
> > 5267                  * Reduce stripe_size, round it up to a 16MB boundary again and
> > 5268                  * then use it, unless it ends up being even bigger than the
> > 5269                  * previous value we had already.
> > 5270                  */
> > 5271                 ctl->stripe_size = min(round_up(div_u64(ctl->max_chunk_size,
> > 5272                                                         data_stripes), SZ_16M),
> > 5273                                        ctl->stripe_size);
> > 5274         }
> > 
> > Here it could lead to a different stripe_size when max_chunk_size would
> > be 1G vs 10G, though the other adjustments could change the upper value.
> 


[-- Attachment #2: folded-v2.patch --]
[-- Type: application/octet-stream, Size: 3406 bytes --]

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 2cf8da1116eb..bfe884f13934 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -187,6 +187,16 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info)
  */
 #define BTRFS_DEFAULT_ZONED_RECLAIM_THRESH			(75)
 
+/*
+ * Update default chunk size.
+ */
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size)
+{
+	ASSERT(space_info);
+	WRITE_ONCE(space_info->chunk_size, chunk_size);
+}
+
 static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index c096695598c1..e7de24a529cf 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -25,6 +25,8 @@ struct btrfs_space_info {
 	u64 max_extent_size;	/* This will hold the maximum extent size of
 				   the space info if we had an ENOSPC in the
 				   allocator. */
+	/* Chunk size in bytes */
+	u64 chunk_size;
 
 	/*
 	 * Once a block group drops below this threshold (percents) we'll
@@ -123,6 +125,8 @@ void btrfs_update_space_info(struct btrfs_fs_info *info, u64 flags,
 			     u64 total_bytes, u64 bytes_used,
 			     u64 bytes_readonly, u64 bytes_zone_unusable,
 			     struct btrfs_space_info **space_info);
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size);
 struct btrfs_space_info *btrfs_find_space_info(struct btrfs_fs_info *info,
 					       u64 flags);
 u64 __pure btrfs_space_info_used(struct btrfs_space_info *s_info,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a2bb0928dc06..b7b7d254ecf0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5072,6 +5072,7 @@ static void init_alloc_chunk_ctl_policy_regular(
 				struct alloc_chunk_ctl *ctl)
 {
 	u64 type = ctl->type;
+	struct btrfs_space_info *space_info;
 
 	if (type & BTRFS_BLOCK_GROUP_DATA) {
 		ctl->max_stripe_size = SZ_1G;
@@ -5095,7 +5096,16 @@ static void init_alloc_chunk_ctl_policy_regular(
 	/* We don't want a chunk larger than 10% of writable space */
 	ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),
 				  ctl->max_chunk_size);
+	if(ctl->max_stripe_size > ctl->max_chunk_size)
+		ctl->max_stripe_size = ctl->max_chunk_size;
+
 	ctl->dev_extent_min = BTRFS_STRIPE_LEN * ctl->dev_stripes;
+
+	if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM)
+		ctl->devs_max = min_t(int, ctl->devs_max, BTRFS_MAX_DEVS_SYS_CHUNK);
+
+	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
+	btrfs_update_space_info_chunk_size(space_info, ctl->max_chunk_size);
 }
 
 static void init_alloc_chunk_ctl_policy_zoned(
@@ -5108,6 +5119,7 @@ static void init_alloc_chunk_ctl_policy_zoned(
 	int min_data_stripes = (min_num_stripes - ctl->nparity) / ctl->ncopies;
 	u64 min_chunk_size = min_data_stripes * zone_size;
 	u64 type = ctl->type;
+	struct btrfs_space_info *space_info;
 
 	ctl->max_stripe_size = zone_size;
 	if (type & BTRFS_BLOCK_GROUP_DATA) {
@@ -5129,6 +5141,9 @@ static void init_alloc_chunk_ctl_policy_zoned(
 		    min_chunk_size);
 	ctl->max_chunk_size = min(limit, ctl->max_chunk_size);
 	ctl->dev_extent_min = zone_size * ctl->dev_stripes;
+
+	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
+	btrfs_update_space_info_chunk_size(space_info, ctl->max_chunk_size);
 }
 
 static void init_alloc_chunk_ctl(struct btrfs_fs_devices *fs_devices,

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Wang Yugui]

[-- Attachment #1: Type: text/plain, Size: 3675 bytes --]

Hi,

attachement file(folded-v3.patch):

changes since v2:
   move the caller of btrfs_update_space_info_chunk_size() into init_alloc_chunk_ctl();
   drop the added check of BTRFS_MAX_DEVS_SYS_CHUNK,  that check is already done

changes since v1:
   move ASSERT(space_info) into btrfs_update_space_info_chunk_size();

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/07/29

> Hi,
> 
> attachement file(folded-v2.patch):
> 
> changes:
>    move ASSERT(space_info) into btrfs_update_space_info_chunk_size();
> 
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/07/29
> 
> > Hi,
> > 
> > I tried to fold my fix/comment into the attachement file(folded.patch).
> > 
> > Just the following is new, others are just orig feature or refactor.
> > 
> > +	if(ctl->max_stripe_size > ctl->max_chunk_size)
> > +		ctl->max_stripe_size = ctl->max_chunk_size;
> > 
> > Best Regards
> > Wang Yugui (wangyugui@e16-tech.com)
> > 2022/07/29
> > 
> > > On Tue, Jul 26, 2022 at 06:25:38AM +0800, Wang Yugui wrote:
> > > > > On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> > > > > > In this patch, the max chunk size is changed from 
> > > > > > BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
> > > > > 
> > > > > The patch hasn't been merged, the change from 1G to 10G without proper
> > > > > evaluation won't happen. The sysfs knob is available for users who want
> > > > > to test it or know that the non-default value works in their
> > > > > environment.
> > > > 
> > > > this patch is in misc-next( 5.19.0-rc8 based, 5.19.0-rc7 based) now.
> > > > 
> > > > 5.19.0-rc8 based:
> > > > f6fca3917b4d btrfs: store chunk size in space-info struct
> > > >
> > > > The sysfs knob show that current default chunk size is 1G, not 10G as
> > > > older version.
> > > 
> > > So there are two things regarding chunk size, the default size and that
> > > it's settable by user (with some limitations). I was replying to the
> > > default size change while you are concerned about the max_chunk_size.
> > > 
> > > You're right that the value changed in the patch, but as I'm reading the
> > > code it should not have any effect. When user sets a value in
> > > btrfs_chunk_size_store() it's limited inside the sysfs handler to the
> > > 10G. Also there are various adjustments when the chunk size is
> > > initialized (init_alloc_chunk_ctl_policy_regular).
> > > 
> > > The only difference I can see comparing master and misc-next is in
> > > decide_stripe_size_regular()
> > > 
> > > 5259         /*
> > > 5260          * Use the number of data stripes to figure out how big this chunk is
> > > 5261          * really going to be in terms of logical address space, and compare
> > > 5262          * that answer with the max chunk size. If it's higher, we try to
> > > 5263          * reduce stripe_size.
> > > 5264          */
> > > 5265         if (ctl->stripe_size * data_stripes > ctl->max_chunk_size) {
> > > ^^^^
> > > 5266                 /*
> > > 5267                  * Reduce stripe_size, round it up to a 16MB boundary again and
> > > 5268                  * then use it, unless it ends up being even bigger than the
> > > 5269                  * previous value we had already.
> > > 5270                  */
> > > 5271                 ctl->stripe_size = min(round_up(div_u64(ctl->max_chunk_size,
> > > 5272                                                         data_stripes), SZ_16M),
> > > 5273                                        ctl->stripe_size);
> > > 5274         }
> > > 
> > > Here it could lead to a different stripe_size when max_chunk_size would
> > > be 1G vs 10G, though the other adjustments could change the upper value.
> > 
> 


[-- Attachment #2: folded-v3.patch --]
[-- Type: application/octet-stream, Size: 3033 bytes --]

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 2dd8754cb990..f39e55807abb 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -187,6 +187,16 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info)
  */
 #define BTRFS_DEFAULT_ZONED_RECLAIM_THRESH			(75)
 
+/*
+ * Update default chunk size.
+ */
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size)
+{
+	ASSERT(space_info);
+	WRITE_ONCE(space_info->chunk_size, chunk_size);
+}
+
 static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index c096695598c1..e7de24a529cf 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -25,6 +25,8 @@ struct btrfs_space_info {
 	u64 max_extent_size;	/* This will hold the maximum extent size of
 				   the space info if we had an ENOSPC in the
 				   allocator. */
+	/* Chunk size in bytes */
+	u64 chunk_size;
 
 	/*
 	 * Once a block group drops below this threshold (percents) we'll
@@ -123,6 +125,8 @@ void btrfs_update_space_info(struct btrfs_fs_info *info, u64 flags,
 			     u64 total_bytes, u64 bytes_used,
 			     u64 bytes_readonly, u64 bytes_zone_unusable,
 			     struct btrfs_space_info **space_info);
+void btrfs_update_space_info_chunk_size(struct btrfs_space_info *space_info,
+					u64 chunk_size);
 struct btrfs_space_info *btrfs_find_space_info(struct btrfs_fs_info *info,
 					       u64 flags);
 u64 __pure btrfs_space_info_used(struct btrfs_space_info *s_info,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9c20049d1fec..9e7ac6e3f028 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5095,6 +5095,8 @@ static void init_alloc_chunk_ctl_policy_regular(
 	/* We don't want a chunk larger than 10% of writable space */
 	ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),
 				  ctl->max_chunk_size);
+	ctl->max_stripe_size = min(ctl->max_stripe_size, ctl->max_chunk_size);
+
 	ctl->dev_extent_min = BTRFS_STRIPE_LEN * ctl->dev_stripes;
 }
 
@@ -5128,6 +5130,8 @@ static void init_alloc_chunk_ctl_policy_zoned(
 			       zone_size),
 		    min_chunk_size);
 	ctl->max_chunk_size = min(limit, ctl->max_chunk_size);
+	ASSERT(ctl->max_stripe_size <= ctl->max_chunk_size);
+
 	ctl->dev_extent_min = zone_size * ctl->dev_stripes;
 }
 
@@ -5135,6 +5139,7 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_devices *fs_devices,
 				 struct alloc_chunk_ctl *ctl)
 {
 	int index = btrfs_bg_flags_to_raid_index(ctl->type);
+	struct btrfs_space_info *space_info;
 
 	ctl->sub_stripes = btrfs_raid_array[index].sub_stripes;
 	ctl->dev_stripes = btrfs_raid_array[index].dev_stripes;
@@ -5157,6 +5162,9 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_devices *fs_devices,
 	default:
 		BUG();
 	}
+
+	space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type);
+	btrfs_update_space_info_chunk_size(space_info, ctl->max_chunk_size);
 }
 
 static int gather_device_info(struct btrfs_fs_devices *fs_devices,

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Wang Yugui]

Hi,

I'm sorry that this folded patch is wrong.

because init_alloc_chunk_ctl() is not called in btrfs mount stage.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/07/29

> Hi,
> 
> attachement file(folded-v3.patch):
> 
> changes since v2:
>    move the caller of btrfs_update_space_info_chunk_size() into init_alloc_chunk_ctl();
>    drop the added check of BTRFS_MAX_DEVS_SYS_CHUNK,  that check is already done
> 
> changes since v1:
>    move ASSERT(space_info) into btrfs_update_space_info_chunk_size();
> 
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/07/29
> 
> > Hi,
> > 
> > attachement file(folded-v2.patch):
> > 
> > changes:
> >    move ASSERT(space_info) into btrfs_update_space_info_chunk_size();
> > 
> > Best Regards
> > Wang Yugui (wangyugui@e16-tech.com)
> > 2022/07/29
> > 
> > > Hi,
> > > 
> > > I tried to fold my fix/comment into the attachement file(folded.patch).
> > > 
> > > Just the following is new, others are just orig feature or refactor.
> > > 
> > > +	if(ctl->max_stripe_size > ctl->max_chunk_size)
> > > +		ctl->max_stripe_size = ctl->max_chunk_size;
> > > 
> > > Best Regards
> > > Wang Yugui (wangyugui@e16-tech.com)
> > > 2022/07/29
> > > 
> > > > On Tue, Jul 26, 2022 at 06:25:38AM +0800, Wang Yugui wrote:
> > > > > > On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
> > > > > > > In this patch, the max chunk size is changed from 
> > > > > > > BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
> > > > > > 
> > > > > > The patch hasn't been merged, the change from 1G to 10G without proper
> > > > > > evaluation won't happen. The sysfs knob is available for users who want
> > > > > > to test it or know that the non-default value works in their
> > > > > > environment.
> > > > > 
> > > > > this patch is in misc-next( 5.19.0-rc8 based, 5.19.0-rc7 based) now.
> > > > > 
> > > > > 5.19.0-rc8 based:
> > > > > f6fca3917b4d btrfs: store chunk size in space-info struct
> > > > >
> > > > > The sysfs knob show that current default chunk size is 1G, not 10G as
> > > > > older version.
> > > > 
> > > > So there are two things regarding chunk size, the default size and that
> > > > it's settable by user (with some limitations). I was replying to the
> > > > default size change while you are concerned about the max_chunk_size.
> > > > 
> > > > You're right that the value changed in the patch, but as I'm reading the
> > > > code it should not have any effect. When user sets a value in
> > > > btrfs_chunk_size_store() it's limited inside the sysfs handler to the
> > > > 10G. Also there are various adjustments when the chunk size is
> > > > initialized (init_alloc_chunk_ctl_policy_regular).
> > > > 
> > > > The only difference I can see comparing master and misc-next is in
> > > > decide_stripe_size_regular()
> > > > 
> > > > 5259         /*
> > > > 5260          * Use the number of data stripes to figure out how big this chunk is
> > > > 5261          * really going to be in terms of logical address space, and compare
> > > > 5262          * that answer with the max chunk size. If it's higher, we try to
> > > > 5263          * reduce stripe_size.
> > > > 5264          */
> > > > 5265         if (ctl->stripe_size * data_stripes > ctl->max_chunk_size) {
> > > > ^^^^
> > > > 5266                 /*
> > > > 5267                  * Reduce stripe_size, round it up to a 16MB boundary again and
> > > > 5268                  * then use it, unless it ends up being even bigger than the
> > > > 5269                  * previous value we had already.
> > > > 5270                  */
> > > > 5271                 ctl->stripe_size = min(round_up(div_u64(ctl->max_chunk_size,
> > > > 5272                                                         data_stripes), SZ_16M),
> > > > 5273                                        ctl->stripe_size);
> > > > 5274         }
> > > > 
> > > > Here it could lead to a different stripe_size when max_chunk_size would
> > > > be 1G vs 10G, though the other adjustments could change the upper value.
> > > 
> > 
> 

Re: [PATCH v6 1/4] btrfs: store chunk size in space-info struct.

[Qu Wenruo]

On 2022/7/25 21:41, David Sterba wrote:
> On Sat, Jul 23, 2022 at 07:49:37AM +0800, Wang Yugui wrote:
>> Hi,
>>
>> In this patch, the max chunk size is changed from
>> BTRFS_MAX_DATA_CHUNK_SIZE(10G) to SZ_1G without any comment ?
>
> The patch hasn't been merged, the change from 1G to 10G without proper
> evaluation won't happen. The sysfs knob is available for users who want
> to test it or know that the non-default value works in their
> environment.

Nope, this is already in torvalds master branch.

Furthermore, this patch now completely limits all data chunk size to 1G,
previously it can be 10G (but stripe size is still limited to 1G).

Now for any RAID0/10 based profiles, it's completely off.

For RAID0 we have at least one time more chunks, and the more device we
have the more chunks we have.

Such big behavior change is not really properly reviewed at all.

Thanks,
Qu

Re: [PATCH v6 0/4] btrfs: sysfs: set / query btrfs chunk size

[Qu Wenruo]

On 2021/12/4 06:04, Stefan Roesch wrote:
> The btrfs allocator is currently not ideal for all workloads. It tends
> to suffer from overallocating data block groups and underallocating
> metadata block groups. This results in filesystems becoming read-only
> even though there is plenty of "free" space.
>
> This is naturally confusing and distressing to users.
>
> Patches:
> 1) Store the chunk size in the btrfs_space_info structure

I guess it's too late, since the series is already in upstream, but I
believe we need to do some more review on this feature.

Firstly, I don't believe a single chunk_size (even it's per space-info)
is good enough.

The default chunk allocator has both checks on stripe_size and
chunk_size, but the series only added chunk_size knob, missing
stripe_size one.

It would be much better to add another stripe_size knob for it.

> 2) Add a sysfs entry to read and write the above information
> 3) Add a sysfs entry to force a space allocation

Any idea on how we handle the auto-reclaim on empty bgs?
AKA, what's preventing the newly allocated bgs from being immediately
reclaimed?

I didn't see any special handling in the 3rd patch.

> 4) Increase the default size of the metadata chunk allocation to 5GB
>     for volumes greater than 50GB.

It would be better to also allow users to tweak the size threshold.
(E.g. allowing a knob for threshold on different chunk/stripe size
limits, along with above mentioned stripe size knob).

Thanks,
Qu
>
> Testing:
>    A new test is being added to the xfstest suite. For reference the
>    corresponding patch has the title:
>      [PATCH] btrfs: Test chunk allocation with different sizes
>
>    In addition also manual testing has been performed.
>      - Run xfstests with the changes and the new test. It does not
>        show new diffs.
>      - Test with storage devices 10G, 20G, 30G, 50G, 60G
>        - Default allocation
>        - Increase of chunk size
>        - If the stripe size is > the free space, it allocates
>          free space - 1MB. The 1MB is left as free space.
>        - If the device has a storage size > 50G, it uses a 5GB
>          chunk size for new allocations.
>
> ---
> v6: - Update btrfs_force_chunk_alloc_store to use tree_root
>        instead of extent_root.
> V5: - Changed the field name in the btrfs_space_info struct from
>        default_chunk_size to chunk_size and made it atomic
>      - Removed the compute_chunk_size_zoned function
>      - Added further checks when writing /sys/fs/btrfs/<id>/allocation/<type>/chunk_size
>      - Removed the ability to query /sys/fs/btrfs/<id>/allocation/<type>/force_alloc_chunk
>
> V4: - Patch email contained duplicate entries.
>
> V3: - Rename sysfs entry from stripe_size to chunk_size
>      - Remove max_chunk_size field in data structure btrfs_space_info
>      - Rename max_stripe_size field to default_chunk_size in data
>        structure btrfs_space_info
>      - Remove max_chunk_size logic
>      - Use stripe_size = chunk_size
>
> V2:
>     - Split the patch in 4 patches
>     - Added checks for zone volumes in sysfs.c
>     - Replaced the BUG() with ASSERT()
>     - Changed if with fallthrough
>     - Removed comments in space-info.h
> --
> Stefan Roesch (4):
>    btrfs: store chunk size in space-info struct.
>    btrfs: expose chunk size in sysfs.
>    btrfs: add force_chunk_alloc sysfs entry to force allocation
>    btrfs: increase metadata alloc size to 5GB for volumes > 50GB
>
>   fs/btrfs/space-info.c |  41 ++++++++++++
>   fs/btrfs/space-info.h |   3 +
>   fs/btrfs/sysfs.c      | 152 ++++++++++++++++++++++++++++++++++++++++++
>   fs/btrfs/volumes.c    |  28 +++-----
>   4 files changed, 205 insertions(+), 19 deletions(-)
>
>
> Signed-off-by: Stefan Roesch <shr@fb.com>
> base-commit: 87c673b657a7e4784fb7274a77a8529712232d0c