ext4: fix online resize with mballoc

PATCH New Frédéric Bohé 2 participants 3 messages Jul 1, 2008 – Jul 17, 2008 1 message not in the archive ext4 file system

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Frédéric Bohé

author

2008-07-01 09:16 UTC #1 lore raw

PATCH v2 1/1 ext4: fix online resize with mballoc

From: Frederic Bohe <[email protected]>

Update group infos when updating a group's descriptor.
Add group infos when adding a group's descriptor.
Refresh cache pages used by mb_alloc when changes occur.
This will probably need modifications when META_BG resizing will be allowed.

Signed-off-by Frederic Bohe <[email protected]>

---
This patch apply on top of 2.6.26-rc6 + ext4-patch-queue-37b3e39765d8521ba2252bfec81bb91504fa35c8

It fixes oops when a filesystem is resized online while being mounted with mballoc. Two oops have been identified:
The first one occurs when unmounting the resized filesystem.
The second one happens when trying to write to a group added during the online resize.

This patch has been tested with:
- small (100MB) and large (5TB) filesystems.
- 1K blocks and 4K blocks filesystems.
- inode size=256 up to 1024.

Tests consist in :
- online resizing, filling all blocks of the fs, unmounting, fs check
- filling all blocks of the fs, online resizing, filling newly added groups, unmounting, fs check
- Concurrent file copy/online resize, unmounting, fs check.

Non regression tests :
- offline resizing.
- online resizing without mballoc.

 ext4.h    |    4 +
 mballoc.c |  236 +++++++++++++++++++++++++++++++++++++++++++++++---------------
 resize.c  |   52 +++++++++++++
 3 files changed, 236 insertions(+), 56 deletions(-)

fs/ext4/ext4.h +4 -0

Index: linux-2.6.26-rc6.patch_resize/fs/ext4/ext4.h
===================================================================
@@ -1115,6 +1115,10 @@ extern int __init init_ext4_mballoc(void
 extern void exit_ext4_mballoc(void);
 extern void ext4_mb_free_blocks(handle_t *, struct inode *,
 		unsigned long, unsigned long, int, unsigned long *);
+extern int ext4_mb_add_more_groupinfo(struct super_block *sb,
+		ext4_group_t i, struct ext4_group_desc *desc);
+extern void ext4_mb_update_group_info(struct ext4_group_info *grp,
+		ext4_grpblk_t add);
 
 
 /* inode.c */

fs/ext4/mballoc.c +179 ~~-55~~

262 lines not on this page · raw message

fs/ext4/resize.c +51 -1

76 lines not on this page · raw message

Frédéric Bohé

author

2008-07-17 09:33 UTC #2 lore raw

PATCH 1/1 Fix inode table allocation with flexbg

In reply to a message not in the archive

From: Frederic Bohe <[email protected]>

Disordered inode tables may appear when inode_blocks_per_group is lesser or equal to the number of groups in a flex group.

Signed-off-by Frederic Bohe <[email protected]>

---
This bug can be reproduced with:
mkfs.ext4 -t ext4dev -G512 70G

In that case, you can see with dump2fs that inode tables for groups 510 and 511 are placed just after group 51's inode table instead of being placed after group 509's inode table.

 alloc_tables.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

lib/ext2fs/alloc_tables.c +7 -9

Index: e2fsprogs/lib/ext2fs/alloc_tables.c
===================================================================
@@ -34,9 +34,10 @@
  * tables can be allocated continously and in order.
  */
 static blk_t flexbg_offset(ext2_filsys fs, dgrp_t group, blk_t start_blk,
-			   ext2fs_block_bitmap bmap, int offset, int size)
+			   ext2fs_block_bitmap bmap, int offset, int size,
+			   int elem_size)
 {
-	int		flexbg, flexbg_size, elem_size;
+	int		flexbg, flexbg_size;
 	blk_t		last_blk, first_free = 0;
 	dgrp_t	       	last_grp;

@@ -54,10 +55,6 @@ static blk_t flexbg_offset(ext2_filsys f
 	 * search is still valid.
 	 */
 	if (start_blk && group % flexbg_size) {
-		if (size > flexbg_size)
-			elem_size = fs->inode_blocks_per_group;
-		else
-			elem_size = 1;
 		if (ext2fs_test_block_bitmap_range(bmap, start_blk + elem_size,
 						   size))
 			return start_blk + elem_size;

@@ -126,7 +123,7 @@ errcode_t ext2fs_allocate_group_table(ex
 		if (group && fs->group_desc[group-1].bg_block_bitmap)
 			prev_block = fs->group_desc[group-1].bg_block_bitmap;
 		start_blk = flexbg_offset(fs, group, prev_block, bmap,
-						 0, rem_grps);
+						 0, rem_grps, 1);
 		last_blk = ext2fs_group_last_block(fs, last_grp);
 	}

@@ -154,7 +151,7 @@ errcode_t ext2fs_allocate_group_table(ex
 		if (group && fs->group_desc[group-1].bg_inode_bitmap)
 			prev_block = fs->group_desc[group-1].bg_inode_bitmap;
 		start_blk = flexbg_offset(fs, group, prev_block, bmap,
-						 flexbg_size, rem_grps);
+						 flexbg_size, rem_grps, 1);
 		last_blk = ext2fs_group_last_block(fs, last_grp);
 	}

@@ -187,7 +184,8 @@ errcode_t ext2fs_allocate_group_table(ex
 		group_blk = flexbg_offset(fs, group, prev_block, bmap,
 						 flexbg_size * 2,
 						 fs->inode_blocks_per_group *
-						 rem_grps);
+						 rem_grps,
+						 fs->inode_blocks_per_group);
 		last_blk = ext2fs_group_last_block(fs, last_grp);
 	}

Jose R. Santos

Acked-by on v1 1/1

2008-07-17 13:48 UTC #3 lore raw

Re: [PATCH][e2fsprogs] Fix inode table allocation with flexbg

Quoted 5 lines · On Thu, 17 Jul 2008 13:34:41 +0200 Frédéric Bohé <[email protected]> wrote:

From: Frederic Bohe <[email protected]>
Disordered inode tables may appear when inode_blocks_per_group is lesser or equal to the number of groups in a flex group.

Acked-by Jose R. Santos <[email protected]> with some comments bellow

Quoted 17 lines

Signed-off-by Frederic Bohe <[email protected]>
---
This bug can be reproduced with:
mkfs.ext4 -t ext4dev -G512 70G
In that case, you can see with dump2fs that inode tables for groups 510 and 511 are placed just after group 51's inode table instead of being placed after group 509's inode table.
 alloc_tables.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
Index: e2fsprogs/lib/ext2fs/alloc_tables.c
===================================================================

lib/ext2fs/alloc_tables.c line 38

Quoted 6 lines

lib/ext2fs/alloc_tables.c +2 -1

@@ -34,9 +34,10 @@
  * tables can be allocated continously and in order.
  */
 static blk_t flexbg_offset(ext2_filsys fs, dgrp_t group, blk_t start_blk,
-			   ext2fs_block_bitmap bmap, int offset, int size)
+			   ext2fs_block_bitmap bmap, int offset, int size,
+			   int elem_size)

While there is nothing technically wrong with the patch I think it exposes an issue with the original flexbg_offset function. The routine right now requires to many arguments and you need to do some calculations within some of the arguments which make this look ugly and error prone if someone decides to modify this code later on.

I've been thinking about rewriting this function so it looks something like this:

static blk64_t flexbg_offset(ext2_filsys fs, ext2fs_block_bitmap bmap,
dgrp_t group, unsigned int type)

and do all the offset, start_blk, size, elem_size calculations inside the routine. I need to make a long term fix for this issue since I think 7 args for this routine is just to much.

-JRS
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html