* [PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd()
@ 2023-10-18 11:42 Baokun Li
2023-10-18 11:42 ` [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Baokun Li @ 2023-10-18 11:42 UTC (permalink / raw
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int
2023-10-18 11:42 [PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
@ 2023-10-18 11:42 ` Baokun Li
2023-10-19 11:21 ` Jan Kara
2023-10-18 11:42 ` [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd() Baokun Li
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Baokun Li @ 2023-10-18 11:42 UTC (permalink / raw
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd()
2023-10-18 11:42 [PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
2023-10-18 11:42 ` [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
@ 2023-10-18 11:42 ` Baokun Li
2023-10-19 11:24 ` Jan Kara
2023-10-18 11:42 ` [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg Baokun Li
2023-10-18 11:42 ` [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd() Baokun Li
3 siblings, 1 reply; 10+ messages in thread
From: Baokun Li @ 2023-10-18 11:42 UTC (permalink / raw
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg
2023-10-18 11:42 [PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
2023-10-18 11:42 ` [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
2023-10-18 11:42 ` [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd() Baokun Li
@ 2023-10-18 11:42 ` Baokun Li
2023-10-19 11:44 ` Jan Kara
2023-10-18 11:42 ` [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd() Baokun Li
3 siblings, 1 reply; 10+ messages in thread
From: Baokun Li @ 2023-10-18 11:42 UTC (permalink / raw
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()
2023-10-18 11:42 [PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
` (2 preceding siblings ...)
2023-10-18 11:42 ` [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg Baokun Li
@ 2023-10-18 11:42 ` Baokun Li
2023-10-19 11:57 ` Jan Kara
3 siblings, 1 reply; 10+ messages in thread
From: Baokun Li @ 2023-10-18 11:42 UTC (permalink / raw
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int
2023-10-18 11:42 ` [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
@ 2023-10-19 11:21 ` Jan Kara
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2023-10-19 11:21 UTC (permalink / raw
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd()
2023-10-18 11:42 ` [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd() Baokun Li
@ 2023-10-19 11:24 ` Jan Kara
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2023-10-19 11:24 UTC (permalink / raw
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg
2023-10-18 11:42 ` [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg Baokun Li
@ 2023-10-19 11:44 ` Jan Kara
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2023-10-19 11:44 UTC (permalink / raw
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()
2023-10-18 11:42 ` [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd() Baokun Li
@ 2023-10-19 11:57 ` Jan Kara
2023-10-19 12:03 ` Baokun Li
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kara @ 2023-10-19 11:57 UTC (permalink / raw
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()
2023-10-19 11:57 ` Jan Kara
@ 2023-10-19 12:03 ` Baokun Li
0 siblings, 0 replies; 10+ messages in thread
From: Baokun Li @ 2023-10-19 12:03 UTC (permalink / raw
To: Jan Kara
Cc: linux-ext4, tytso, adilger.kernel, ritesh.list, linux-kernel,
yi.zhang, yangerkun, yukuai3, 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
.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-19 12:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-18 11:42 [PATCH 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
2023-10-18 11:42 ` [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
2023-10-19 11:21 ` Jan Kara
2023-10-18 11:42 ` [PATCH 2/4] ext4: remove unnecessary check from alloc_flex_gd() Baokun Li
2023-10-19 11:24 ` Jan Kara
2023-10-18 11:42 ` [PATCH 3/4] ext4: avoid online resizing failures due to oversized flex bg Baokun Li
2023-10-19 11:44 ` Jan Kara
2023-10-18 11:42 ` [PATCH 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd() Baokun Li
2023-10-19 11:57 ` Jan Kara
2023-10-19 12:03 ` Baokun Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).