| Message ID | 20231018114221.441526-5-libaokun1@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: fix WARN_ON in alloc_flex_gd() | expand |
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
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!
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;
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(-)