[PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd()

[PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd()

[Baokun Li]

Baokun Li (4):
  ext4: unify the type of flexbg_size to unsigned int
  ext4: remove unnecessary check from alloc_flex_gd()
  ext4: avoid online resizing failures due to oversized flex bg
  ext4: reduce unnecessary memory allocation in alloc_flex_gd()

 fs/ext4/resize.c | 49 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 16 deletions(-)

-- 
2.31.1

[PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int

[Baokun Li]

The maximum value of flexbg_size is 2^31, but the maximum value of int
is (2^31 - 1), so overflow may occur when the type of flexbg_size is
declared as int.

For example, when uninit_mask is initialized in ext4_alloc_group_tables(),
if flexbg_size == 2^31, the initialized uninit_mask is incorrect, and this
may causes set_flexbg_block_bitmap() to trigger a BUG_ON().

Therefore, the flexbg_size type is declared as unsigned int to avoid
overflow and memory waste.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/resize.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 4fe061edefdd..c6d4539d4c1f 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -228,7 +228,7 @@ struct ext4_new_flex_group_data {
  *
  * Returns NULL on failure otherwise address of the allocated structure.
  */
-static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned long flexbg_size)
+static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
 {
 	struct ext4_new_flex_group_data *flex_gd;
 
@@ -283,7 +283,7 @@ static void free_flex_gd(struct ext4_new_flex_group_data *flex_gd)
  */
 static int ext4_alloc_group_tables(struct super_block *sb,
 				struct ext4_new_flex_group_data *flex_gd,
-				int flexbg_size)
+				unsigned int flexbg_size)
 {
 	struct ext4_new_group_data *group_data = flex_gd->groups;
 	ext4_fsblk_t start_blk;
@@ -384,12 +384,12 @@ static int ext4_alloc_group_tables(struct super_block *sb,
 		group = group_data[0].group;
 
 		printk(KERN_DEBUG "EXT4-fs: adding a flex group with "
-		       "%d groups, flexbg size is %d:\n", flex_gd->count,
+		       "%u groups, flexbg size is %u:\n", flex_gd->count,
 		       flexbg_size);
 
 		for (i = 0; i < flex_gd->count; i++) {
 			ext4_debug(
-			       "adding %s group %u: %u blocks (%d free, %d mdata blocks)\n",
+			       "adding %s group %u: %u blocks (%u free, %u mdata blocks)\n",
 			       ext4_bg_has_super(sb, group + i) ? "normal" :
 			       "no-super", group + i,
 			       group_data[i].blocks_count,
@@ -1606,7 +1606,7 @@ static int ext4_flex_group_add(struct super_block *sb,
 static int ext4_setup_next_flex_gd(struct super_block *sb,
 				    struct ext4_new_flex_group_data *flex_gd,
 				    ext4_fsblk_t n_blocks_count,
-				    unsigned long flexbg_size)
+				    unsigned int flexbg_size)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_super_block *es = sbi->s_es;
@@ -1990,8 +1990,9 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 	ext4_fsblk_t o_blocks_count;
 	ext4_fsblk_t n_blocks_count_retry = 0;
 	unsigned long last_update_time = 0;
-	int err = 0, flexbg_size = 1 << sbi->s_log_groups_per_flex;
+	int err = 0;
 	int meta_bg;
+	unsigned int flexbg_size = ext4_flex_bg_size(sbi);
 
 	/* See if the device is actually as big as what was requested */
 	bh = ext4_sb_bread(sb, n_blocks_count - 1, 0);
-- 
2.31.1

Re: [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int

[Jan Kara]

On Wed 18-10-23 19:42:18, Baokun Li wrote:
> The maximum value of flexbg_size is 2^31, but the maximum value of int
> is (2^31 - 1), so overflow may occur when the type of flexbg_size is
> declared as int.
> 
> For example, when uninit_mask is initialized in ext4_alloc_group_tables(),
> if flexbg_size == 2^31, the initialized uninit_mask is incorrect, and this
> may causes set_flexbg_block_bitmap() to trigger a BUG_ON().
> 
> Therefore, the flexbg_size type is declared as unsigned int to avoid
> overflow and memory waste.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/resize.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 4fe061edefdd..c6d4539d4c1f 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -228,7 +228,7 @@ struct ext4_new_flex_group_data {
>   *
>   * Returns NULL on failure otherwise address of the allocated structure.
>   */
> -static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned long flexbg_size)
> +static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
>  {
>  	struct ext4_new_flex_group_data *flex_gd;
>  
> @@ -283,7 +283,7 @@ static void free_flex_gd(struct ext4_new_flex_group_data *flex_gd)
>   */
>  static int ext4_alloc_group_tables(struct super_block *sb,
>  				struct ext4_new_flex_group_data *flex_gd,
> -				int flexbg_size)
> +				unsigned int flexbg_size)
>  {
>  	struct ext4_new_group_data *group_data = flex_gd->groups;
>  	ext4_fsblk_t start_blk;
> @@ -384,12 +384,12 @@ static int ext4_alloc_group_tables(struct super_block *sb,
>  		group = group_data[0].group;
>  
>  		printk(KERN_DEBUG "EXT4-fs: adding a flex group with "
> -		       "%d groups, flexbg size is %d:\n", flex_gd->count,
> +		       "%u groups, flexbg size is %u:\n", flex_gd->count,
>  		       flexbg_size);
>  
>  		for (i = 0; i < flex_gd->count; i++) {
>  			ext4_debug(
> -			       "adding %s group %u: %u blocks (%d free, %d mdata blocks)\n",
> +			       "adding %s group %u: %u blocks (%u free, %u mdata blocks)\n",
>  			       ext4_bg_has_super(sb, group + i) ? "normal" :
>  			       "no-super", group + i,
>  			       group_data[i].blocks_count,
> @@ -1606,7 +1606,7 @@ static int ext4_flex_group_add(struct super_block *sb,
>  static int ext4_setup_next_flex_gd(struct super_block *sb,
>  				    struct ext4_new_flex_group_data *flex_gd,
>  				    ext4_fsblk_t n_blocks_count,
> -				    unsigned long flexbg_size)
> +				    unsigned int flexbg_size)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	struct ext4_super_block *es = sbi->s_es;
> @@ -1990,8 +1990,9 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
>  	ext4_fsblk_t o_blocks_count;
>  	ext4_fsblk_t n_blocks_count_retry = 0;
>  	unsigned long last_update_time = 0;
> -	int err = 0, flexbg_size = 1 << sbi->s_log_groups_per_flex;
> +	int err = 0;
>  	int meta_bg;
> +	unsigned int flexbg_size = ext4_flex_bg_size(sbi);
>  
>  	/* See if the device is actually as big as what was requested */
>  	bh = ext4_sb_bread(sb, n_blocks_count - 1, 0);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd()

[Baokun Li]

In commit 967ac8af4475 ("ext4: fix potential integer overflow in
alloc_flex_gd()"), an overflow check is added to alloc_flex_gd() to
prevent the allocated memory from being smaller than expected due to
the overflow. However, after kmalloc() is replaced with kmalloc_array()
in commit 6da2ec56059c ("treewide: kmalloc() -> kmalloc_array()"), the
kmalloc_array() function has an overflow check, so the above problem
will not occur. Therefore, the extra check is removed.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/resize.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index c6d4539d4c1f..0a57b199883c 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -236,10 +236,7 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
 	if (flex_gd == NULL)
 		goto out3;
 
-	if (flexbg_size >= UINT_MAX / sizeof(struct ext4_new_group_data))
-		goto out2;
 	flex_gd->count = flexbg_size;
-
 	flex_gd->groups = kmalloc_array(flexbg_size,
 					sizeof(struct ext4_new_group_data),
 					GFP_NOFS);
-- 
2.31.1

Re: [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd()

[Jan Kara]

On Wed 18-10-23 19:42:19, Baokun Li wrote:
> In commit 967ac8af4475 ("ext4: fix potential integer overflow in
> alloc_flex_gd()"), an overflow check is added to alloc_flex_gd() to
> prevent the allocated memory from being smaller than expected due to
> the overflow. However, after kmalloc() is replaced with kmalloc_array()
> in commit 6da2ec56059c ("treewide: kmalloc() -> kmalloc_array()"), the
> kmalloc_array() function has an overflow check, so the above problem
> will not occur. Therefore, the extra check is removed.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/resize.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index c6d4539d4c1f..0a57b199883c 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -236,10 +236,7 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
>  	if (flex_gd == NULL)
>  		goto out3;
>  
> -	if (flexbg_size >= UINT_MAX / sizeof(struct ext4_new_group_data))
> -		goto out2;
>  	flex_gd->count = flexbg_size;
> -
>  	flex_gd->groups = kmalloc_array(flexbg_size,
>  					sizeof(struct ext4_new_group_data),
>  					GFP_NOFS);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg

[Baokun Li]

When we online resize an ext4 filesystem with a oversized flexbg_size,

     mkfs.ext4 -F -G 67108864 $dev -b 4096 100M
     mount $dev $dir
     resize2fs $dev 16G

the following WARN_ON is triggered:
==================================================================
WARNING: CPU: 0 PID: 427 at mm/page_alloc.c:4402 __alloc_pages+0x411/0x550
Modules linked in: sg(E)
CPU: 0 PID: 427 Comm: resize2fs Tainted: G  E  6.6.0-rc5+ #314
RIP: 0010:__alloc_pages+0x411/0x550
Call Trace:
 <TASK>
 __kmalloc_large_node+0xa2/0x200
 __kmalloc+0x16e/0x290
 ext4_resize_fs+0x481/0xd80
 __ext4_ioctl+0x1616/0x1d90
 ext4_ioctl+0x12/0x20
 __x64_sys_ioctl+0xf0/0x150
 do_syscall_64+0x3b/0x90
==================================================================

This is because flexbg_size is too large and the size of the new_group_data
array to be allocated exceeds MAX_ORDER. Currently, the minimum value of
MAX_ORDER is 8, the minimum value of PAGE_SIZE is 4096, the corresponding
maximum number of groups that can be allocated is:

 (PAGE_SIZE << MAX_ORDER) / sizeof(struct ext4_new_group_data) ≈ 21845

And the value that is down-aligned to the power of 2 is 16384. Therefore,
this value is defined as MAX_RESIZE_BG, and the number of groups added
each time does not exceed this value during resizing, and is added multiple
times to complete the online resizing. The difference is that the metadata
in a flex_bg may be more dispersed.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/resize.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 0a57b199883c..e168a9f59600 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -218,10 +218,17 @@ struct ext4_new_flex_group_data {
 						   in the flex group */
 	__u16 *bg_flags;			/* block group flags of groups
 						   in @groups */
+	ext4_group_t resize_bg;			/* number of allocated
+						   new_group_data */
 	ext4_group_t count;			/* number of groups in @groups
 						 */
 };
 
+/*
+ * Avoiding memory allocation failures due to too many groups added each time.
+ */
+#define MAX_RESIZE_BG				16384
+
 /*
  * alloc_flex_gd() allocates a ext4_new_flex_group_data with size of
  * @flexbg_size.
@@ -236,14 +243,18 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
 	if (flex_gd == NULL)
 		goto out3;
 
-	flex_gd->count = flexbg_size;
-	flex_gd->groups = kmalloc_array(flexbg_size,
+	if (unlikely(flexbg_size > MAX_RESIZE_BG))
+		flex_gd->resize_bg = MAX_RESIZE_BG;
+	else
+		flex_gd->resize_bg = flexbg_size;
+
+	flex_gd->groups = kmalloc_array(flex_gd->resize_bg,
 					sizeof(struct ext4_new_group_data),
 					GFP_NOFS);
 	if (flex_gd->groups == NULL)
 		goto out2;
 
-	flex_gd->bg_flags = kmalloc_array(flexbg_size, sizeof(__u16),
+	flex_gd->bg_flags = kmalloc_array(flex_gd->resize_bg, sizeof(__u16),
 					  GFP_NOFS);
 	if (flex_gd->bg_flags == NULL)
 		goto out1;
@@ -1602,8 +1613,7 @@ static int ext4_flex_group_add(struct super_block *sb,
 
 static int ext4_setup_next_flex_gd(struct super_block *sb,
 				    struct ext4_new_flex_group_data *flex_gd,
-				    ext4_fsblk_t n_blocks_count,
-				    unsigned int flexbg_size)
+				    ext4_fsblk_t n_blocks_count)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_super_block *es = sbi->s_es;
@@ -1627,7 +1637,7 @@ static int ext4_setup_next_flex_gd(struct super_block *sb,
 	BUG_ON(last);
 	ext4_get_group_no_and_offset(sb, n_blocks_count - 1, &n_group, &last);
 
-	last_group = group | (flexbg_size - 1);
+	last_group = group | (flex_gd->resize_bg - 1);
 	if (last_group > n_group)
 		last_group = n_group;
 
@@ -2130,8 +2140,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 	/* Add flex groups. Note that a regular group is a
 	 * flex group with 1 group.
 	 */
-	while (ext4_setup_next_flex_gd(sb, flex_gd, n_blocks_count,
-					      flexbg_size)) {
+	while (ext4_setup_next_flex_gd(sb, flex_gd, n_blocks_count)) {
 		if (time_is_before_jiffies(last_update_time + HZ * 10)) {
 			if (last_update_time)
 				ext4_msg(sb, KERN_INFO,
-- 
2.31.1

Re: [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg

[Jan Kara]

On Wed 18-10-23 19:42:20, Baokun Li wrote:
> When we online resize an ext4 filesystem with a oversized flexbg_size,
> 
>      mkfs.ext4 -F -G 67108864 $dev -b 4096 100M
>      mount $dev $dir
>      resize2fs $dev 16G
> 
> the following WARN_ON is triggered:
> ==================================================================
> WARNING: CPU: 0 PID: 427 at mm/page_alloc.c:4402 __alloc_pages+0x411/0x550
> Modules linked in: sg(E)
> CPU: 0 PID: 427 Comm: resize2fs Tainted: G  E  6.6.0-rc5+ #314
> RIP: 0010:__alloc_pages+0x411/0x550
> Call Trace:
>  <TASK>
>  __kmalloc_large_node+0xa2/0x200
>  __kmalloc+0x16e/0x290
>  ext4_resize_fs+0x481/0xd80
>  __ext4_ioctl+0x1616/0x1d90
>  ext4_ioctl+0x12/0x20
>  __x64_sys_ioctl+0xf0/0x150
>  do_syscall_64+0x3b/0x90
> ==================================================================
> 
> This is because flexbg_size is too large and the size of the new_group_data
> array to be allocated exceeds MAX_ORDER. Currently, the minimum value of
> MAX_ORDER is 8, the minimum value of PAGE_SIZE is 4096, the corresponding
> maximum number of groups that can be allocated is:
> 
>  (PAGE_SIZE << MAX_ORDER) / sizeof(struct ext4_new_group_data) ≈ 21845
> 
> And the value that is down-aligned to the power of 2 is 16384. Therefore,
> this value is defined as MAX_RESIZE_BG, and the number of groups added
> each time does not exceed this value during resizing, and is added multiple
> times to complete the online resizing. The difference is that the metadata
> in a flex_bg may be more dispersed.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/resize.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 0a57b199883c..e168a9f59600 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -218,10 +218,17 @@ struct ext4_new_flex_group_data {
>  						   in the flex group */
>  	__u16 *bg_flags;			/* block group flags of groups
>  						   in @groups */
> +	ext4_group_t resize_bg;			/* number of allocated
> +						   new_group_data */
>  	ext4_group_t count;			/* number of groups in @groups
>  						 */
>  };
>  
> +/*
> + * Avoiding memory allocation failures due to too many groups added each time.
> + */
> +#define MAX_RESIZE_BG				16384
> +
>  /*
>   * alloc_flex_gd() allocates a ext4_new_flex_group_data with size of
>   * @flexbg_size.
> @@ -236,14 +243,18 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
>  	if (flex_gd == NULL)
>  		goto out3;
>  
> -	flex_gd->count = flexbg_size;
> -	flex_gd->groups = kmalloc_array(flexbg_size,
> +	if (unlikely(flexbg_size > MAX_RESIZE_BG))
> +		flex_gd->resize_bg = MAX_RESIZE_BG;
> +	else
> +		flex_gd->resize_bg = flexbg_size;
> +
> +	flex_gd->groups = kmalloc_array(flex_gd->resize_bg,
>  					sizeof(struct ext4_new_group_data),
>  					GFP_NOFS);
>  	if (flex_gd->groups == NULL)
>  		goto out2;
>  
> -	flex_gd->bg_flags = kmalloc_array(flexbg_size, sizeof(__u16),
> +	flex_gd->bg_flags = kmalloc_array(flex_gd->resize_bg, sizeof(__u16),
>  					  GFP_NOFS);
>  	if (flex_gd->bg_flags == NULL)
>  		goto out1;
> @@ -1602,8 +1613,7 @@ static int ext4_flex_group_add(struct super_block *sb,
>  
>  static int ext4_setup_next_flex_gd(struct super_block *sb,
>  				    struct ext4_new_flex_group_data *flex_gd,
> -				    ext4_fsblk_t n_blocks_count,
> -				    unsigned int flexbg_size)
> +				    ext4_fsblk_t n_blocks_count)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	struct ext4_super_block *es = sbi->s_es;
> @@ -1627,7 +1637,7 @@ static int ext4_setup_next_flex_gd(struct super_block *sb,
>  	BUG_ON(last);
>  	ext4_get_group_no_and_offset(sb, n_blocks_count - 1, &n_group, &last);
>  
> -	last_group = group | (flexbg_size - 1);
> +	last_group = group | (flex_gd->resize_bg - 1);
>  	if (last_group > n_group)
>  		last_group = n_group;
>  
> @@ -2130,8 +2140,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
>  	/* Add flex groups. Note that a regular group is a
>  	 * flex group with 1 group.
>  	 */
> -	while (ext4_setup_next_flex_gd(sb, flex_gd, n_blocks_count,
> -					      flexbg_size)) {
> +	while (ext4_setup_next_flex_gd(sb, flex_gd, n_blocks_count)) {
>  		if (time_is_before_jiffies(last_update_time + HZ * 10)) {
>  			if (last_update_time)
>  				ext4_msg(sb, KERN_INFO,
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()

[Baokun Li]

When a large flex_bg file system is resized, the number of groups to be
added may be small, and a large amount of memory that will not be used will
be allocated. Therefore, resize_bg can be set to the size after the number
of new_group_data to be used is aligned upwards to the power of 2. This
does not affect the disk layout after online resize and saves some memory.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/resize.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index e168a9f59600..4a7430524014 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -235,8 +235,10 @@ struct ext4_new_flex_group_data {
  *
  * Returns NULL on failure otherwise address of the allocated structure.
  */
-static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
+static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size,
+				ext4_group_t o_group, ext4_group_t n_group)
 {
+	ext4_group_t last_group;
 	struct ext4_new_flex_group_data *flex_gd;
 
 	flex_gd = kmalloc(sizeof(*flex_gd), GFP_NOFS);
@@ -248,6 +250,14 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
 	else
 		flex_gd->resize_bg = flexbg_size;
 
+	/* Avoid allocating new groups that will not be used. */
+	last_group = o_group | (flex_gd->resize_bg - 1);
+	if (n_group <= last_group)
+		flex_gd->resize_bg = 1 << fls(n_group - o_group + 1);
+	else if (n_group - last_group < flex_gd->resize_bg)
+		flex_gd->resize_bg = 1 << max(fls(last_group - o_group + 1),
+					      fls(n_group - last_group));
+
 	flex_gd->groups = kmalloc_array(flex_gd->resize_bg,
 					sizeof(struct ext4_new_group_data),
 					GFP_NOFS);
@@ -2131,7 +2141,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 	if (err)
 		goto out;
 
-	flex_gd = alloc_flex_gd(flexbg_size);
+	flex_gd = alloc_flex_gd(flexbg_size, o_group, n_group);
 	if (flex_gd == NULL) {
 		err = -ENOMEM;
 		goto out;
-- 
2.31.1

Re: [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()

[Jan Kara]

On Wed 18-10-23 19:42:21, Baokun Li wrote:
> When a large flex_bg file system is resized, the number of groups to be
> added may be small, and a large amount of memory that will not be used will
> be allocated. Therefore, resize_bg can be set to the size after the number
> of new_group_data to be used is aligned upwards to the power of 2. This
> does not affect the disk layout after online resize and saves some memory.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good, just one small comment below. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>


> @@ -248,6 +250,14 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
>  	else
>  		flex_gd->resize_bg = flexbg_size;
>  
> +	/* Avoid allocating new groups that will not be used. */

Perhaps make the comment more understandable like:
	/* Avoid allocating large 'groups' array if not needed */

									Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()

[Baokun Li]

On 2023/10/19 19:57, Jan Kara wrote:
> On Wed 18-10-23 19:42:21, Baokun Li wrote:
>> When a large flex_bg file system is resized, the number of groups to be
>> added may be small, and a large amount of memory that will not be used will
>> be allocated. Therefore, resize_bg can be set to the size after the number
>> of new_group_data to be used is aligned upwards to the power of 2. This
>> does not affect the disk layout after online resize and saves some memory.
>>
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> Looks good, just one small comment below. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
>
>> @@ -248,6 +250,14 @@ static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
>>   	else
>>   		flex_gd->resize_bg = flexbg_size;
>>   
>> +	/* Avoid allocating new groups that will not be used. */
> Perhaps make the comment more understandable like:
> 	/* Avoid allocating large 'groups' array if not needed */
>
> 									Honza
Thank you very much for your careful review!
I'll change the comment to this more understandable one in the next version.

Cheers!
-- 
With Best Regards,
Baokun Li
.