* [PATCH v2 1/4] ext4: unify the type of flexbg_size to unsigned int
2023-10-23 1:30 [PATCH v2 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
@ 2023-10-23 1:30 ` Baokun Li
2023-10-23 2:31 ` kernel test robot
2023-10-23 1:30 ` [PATCH v2 2/4] ext4: remove unnecessary check from alloc_flex_gd() Baokun Li
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Baokun Li @ 2023-10-23 1:30 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>
Reviewed-by: Jan Kara <jack@suse.cz>
---
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] 8+ messages in thread* [PATCH v2 2/4] ext4: remove unnecessary check from alloc_flex_gd()
2023-10-23 1:30 [PATCH v2 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
2023-10-23 1:30 ` [PATCH v2 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
@ 2023-10-23 1:30 ` Baokun Li
2023-10-23 1:30 ` [PATCH v2 3/4] ext4: avoid online resizing failures due to oversized flex bg Baokun Li
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Baokun Li @ 2023-10-23 1:30 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>
Reviewed-by: Jan Kara <jack@suse.cz>
---
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] 8+ messages in thread* [PATCH v2 3/4] ext4: avoid online resizing failures due to oversized flex bg
2023-10-23 1:30 [PATCH v2 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
2023-10-23 1:30 ` [PATCH v2 1/4] ext4: unify the type of flexbg_size to unsigned int Baokun Li
2023-10-23 1:30 ` [PATCH v2 2/4] ext4: remove unnecessary check from alloc_flex_gd() Baokun Li
@ 2023-10-23 1:30 ` Baokun Li
2023-10-23 2:43 ` kernel test robot
2023-10-23 1:30 ` [PATCH v2 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd() Baokun Li
2024-01-09 2:53 ` [PATCH v2 0/4] ext4: fix WARN_ON " Theodore Ts'o
4 siblings, 1 reply; 8+ messages in thread
From: Baokun Li @ 2023-10-23 1:30 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>
Reviewed-by: Jan Kara <jack@suse.cz>
---
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] 8+ messages in thread* [PATCH v2 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()
2023-10-23 1:30 [PATCH v2 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
` (2 preceding siblings ...)
2023-10-23 1:30 ` [PATCH v2 3/4] ext4: avoid online resizing failures due to oversized flex bg Baokun Li
@ 2023-10-23 1:30 ` Baokun Li
2024-01-09 2:53 ` [PATCH v2 0/4] ext4: fix WARN_ON " Theodore Ts'o
4 siblings, 0 replies; 8+ messages in thread
From: Baokun Li @ 2023-10-23 1:30 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>
Reviewed-by: Jan Kara <jack@suse.cz>
---
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..4d4a5a32e310 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 large 'groups' array if not needed */
+ 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] 8+ messages in thread* Re: [PATCH v2 0/4] ext4: fix WARN_ON in alloc_flex_gd()
2023-10-23 1:30 [PATCH v2 0/4] ext4: fix WARN_ON in alloc_flex_gd() Baokun Li
` (3 preceding siblings ...)
2023-10-23 1:30 ` [PATCH v2 4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd() Baokun Li
@ 2024-01-09 2:53 ` Theodore Ts'o
4 siblings, 0 replies; 8+ messages in thread
From: Theodore Ts'o @ 2024-01-09 2:53 UTC (permalink / raw)
To: linux-ext4, Baokun Li
Cc: Theodore Ts'o, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3
On Mon, 23 Oct 2023 09:30:53 +0800, Baokun Li wrote:
> Changes since v1:
> * Modified the comments in patch 4.
>
> 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()
>
> [...]
Applied, thanks!
[1/4] ext4: unify the type of flexbg_size to unsigned int
(no commit info)
[2/4] ext4: remove unnecessary check from alloc_flex_gd()
(no commit info)
[3/4] ext4: avoid online resizing failures due to oversized flex bg
(no commit info)
[4/4] ext4: reduce unnecessary memory allocation in alloc_flex_gd()
(no commit info)
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 8+ messages in thread