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

[Wang Yugui <wangyugui@e16-tech.com>]

[-- 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,