[PATCH 0/4][e2fsprogs] Enable FLEX_BG support

[PATCH 0/4][e2fsprogs] Enable FLEX_BG support

[Jose R. Santos]

The following series of patches add support creating and checking
filesystems with the FLEX_BG feature.  This feature currently groups
meta-data from a series of groups at the beginning of a flex group in
order to improve performance during heavy meta-data operations.

Some light testing on meta-data filesystem and fsck times already show
some improvements.

This is still very experimental code I meant mostly as a prototype, but
comments are welcome.


-JRS

[PATCH 1/4][e2fsprogs] Reserve the INCOMPAT feature number for FLEX_BG.

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Reserve the INCOMPAT feature number for FLEX_BG.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 lib/ext2fs/ext2_fs.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index a316665..2394857 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -640,6 +640,7 @@ struct ext2_super_block {
 #define EXT3_FEATURE_INCOMPAT_EXTENTS		0x0040
 #define EXT4_FEATURE_INCOMPAT_64BIT		0x0080
 #define EXT4_FEATURE_INCOMPAT_MMP		0x0100
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200
 
 
 #define EXT2_FEATURE_COMPAT_SUPP	0

[PATCH 2/4][e2fsprogs] Allow FLEX_BG to be use as a feature option at mke2fs time.

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Allow FLEX_BG to be use as a feature option at mke2fs time.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 lib/e2p/feature.c   |    2 ++
 lib/ext2fs/ext2fs.h |    6 ++++--
 misc/mke2fs.c       |    3 ++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/e2p/feature.c b/lib/e2p/feature.c
index fe7e65a..4bf5630 100644
--- a/lib/e2p/feature.c
+++ b/lib/e2p/feature.c
@@ -67,6 +67,8 @@ static struct feature feature_list[] = {
 			"extent" },
 	{	E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_64BIT,
 			"64bit" },
+	{       E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_FLEX_BG,
+                        "flex_bg"},
 	{	0, 0, 0 },
 };
 
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 20c63c0..d1cda2f 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -441,12 +441,14 @@ typedef struct ext2_icount *ext2_icount_t;
 					 EXT2_FEATURE_INCOMPAT_COMPRESSION|\
 					 EXT3_FEATURE_INCOMPAT_JOURNAL_DEV|\
 					 EXT2_FEATURE_INCOMPAT_META_BG|\
-					 EXT3_FEATURE_INCOMPAT_RECOVER)
+					 EXT3_FEATURE_INCOMPAT_RECOVER|\
+					 EXT4_FEATURE_INCOMPAT_FLEX_BG)
 #else
 #define EXT2_LIB_FEATURE_INCOMPAT_SUPP	(EXT2_FEATURE_INCOMPAT_FILETYPE|\
 					 EXT3_FEATURE_INCOMPAT_JOURNAL_DEV|\
 					 EXT2_FEATURE_INCOMPAT_META_BG|\
-					 EXT3_FEATURE_INCOMPAT_RECOVER)
+					 EXT3_FEATURE_INCOMPAT_RECOVER|\
+					 EXT4_FEATURE_INCOMPAT_FLEX_BG)
 #endif
 #define EXT2_LIB_FEATURE_RO_COMPAT_SUPP	(EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER|\
 					 EXT2_FEATURE_RO_COMPAT_LARGE_FILE)
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 0c6d4f3..0af92e2 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -873,7 +873,8 @@ static __u32 ok_features[3] = {
 		EXT2_FEATURE_COMPAT_LAZY_BG,	/* Compat */
 	EXT2_FEATURE_INCOMPAT_FILETYPE|		/* Incompat */
 		EXT3_FEATURE_INCOMPAT_JOURNAL_DEV|
-		EXT2_FEATURE_INCOMPAT_META_BG,
+		EXT2_FEATURE_INCOMPAT_META_BG|
+		EXT4_FEATURE_INCOMPAT_FLEX_BG,
 	EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER	/* R/O compat */
 };
 

[PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Relax group descriptor checking.

In order for tools such as dump2efs, e2fsck and debugfs to open a ext4
filesystem with FLEX_BG feature enable, some descriptor checking needs
to be relaxed.  This patch changes the group desciptor checking so
that bitmaps and inode tables can be located anywhere in the
partitions block range.

Eventually, a more thorough check would restrict bitmaps and inode
tables to be located at the beginning of a flex block group range.
Since the super block does not currently know about the number of
groups per flex group, this will do for now.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 e2fsck/super.c          |   10 ++++++++--
 lib/ext2fs/check_desc.c |   10 ++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index 00a131c..8e58e5c 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -578,8 +578,14 @@ void check_super_block(e2fsck_t ctx)
 	for (i = 0, gd=fs->group_desc; i < fs->group_desc_count; i++, gd++) {
 		pctx.group = i;
 
-		first_block = ext2fs_group_first_block(fs, i);
-		last_block = ext2fs_group_last_block(fs, i);
+		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super,
+			EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
+			first_block = fs->super->s_first_data_block;
+			last_block = fs->super->s_blocks_count;
+		} else {
+			first_block = ext2fs_group_first_block(fs, i);
+			last_block = ext2fs_group_last_block(fs, i);
+		}
 
 		if ((gd->bg_block_bitmap < first_block) ||
 		    (gd->bg_block_bitmap > last_block)) {
diff --git a/lib/ext2fs/check_desc.c b/lib/ext2fs/check_desc.c
index 146f9e5..bb65c06 100644
--- a/lib/ext2fs/check_desc.c
+++ b/lib/ext2fs/check_desc.c
@@ -38,8 +38,14 @@ errcode_t ext2fs_check_desc(ext2_filsys fs)
 	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
 
 	for (i = 0; i < fs->group_desc_count; i++) {
-		first_block = ext2fs_group_first_block(fs, i);
-		last_block = ext2fs_group_last_block(fs, i);
+		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super, EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
+			first_block = fs->super->s_first_data_block;
+			last_block = fs->super->s_blocks_count;
+		}
+		else {
+			first_block = ext2fs_group_first_block(fs, i);
+			last_block = ext2fs_group_last_block(fs, i);
+		}
 
 		/*
 		 * Check to make sure block bitmap for group is

[PATCH 4/4][e2fsprogs] New bitmap and inode table allocation for FLEX_BG

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

New bitmap and inode table allocation for FLEX_BG

Change the way we allocate bitmaps and inode tables if the FLEX_BG
feature is used at mke2fs time.  The block and inode bitmaps are
allocated as a one contiguous set for each flex block group.  Due to
the size of the inode tables, the inode table for each block group is
allocate individually but packed close together at the beginning of a
flex group.  For now, this allow for the inode table to be packed
close to the inode bitmaps in cases where we try to allocate a large
group of inode tables right after the bitmaps and fail.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 lib/ext2fs/alloc_tables.c |  138 ++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 134 insertions(+), 4 deletions(-)

diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c
index 4ad2ba9..75252fa 100644
--- a/lib/ext2fs/alloc_tables.c
+++ b/lib/ext2fs/alloc_tables.c
@@ -27,6 +27,130 @@
 #include "ext2_fs.h"
 #include "ext2fs.h"
 
+#define ALLOC_BLOCK_BITMAPS	1
+#define ALLOC_INODE_BITMAPS	2
+#define ALLOC_INODE_TABLES	3
+
+#define GROUPS_PER_FLEXBG	64
+#define FIRST_METADATA_GROUP	3
+
+errcode_t ext2fs_allocate_contiguous(ext2_filsys fs, dgrp_t group,
+				     int type, blk_t start_blk, blk_t last_blk, 
+				     int count, ext2fs_block_bitmap bmap)
+{
+	errcode_t	retval;
+	blk_t		new_blk, blk;
+	int		i, j;
+
+	if (!bmap)
+		bmap = fs->block_map;
+
+	switch (type) {
+	case ALLOC_BLOCK_BITMAPS:
+		retval = ext2fs_get_free_blocks(fs, start_blk, last_blk,
+						1 * count, bmap, &new_blk);
+		if (retval)
+			return retval;
+		for (i=0, blk=new_blk; i < count; i++, blk++) {
+			ext2fs_mark_block_bitmap(bmap, blk);
+			fs->group_desc[group+i].bg_block_bitmap = blk;
+		}
+		break;
+
+	case ALLOC_INODE_BITMAPS:
+		retval = ext2fs_get_free_blocks(fs, start_blk, last_blk,
+						1 * count, bmap, &new_blk);
+		if (retval)
+			return retval;
+		for (i=0, blk=new_blk; i < count; i++, blk++) {
+			ext2fs_mark_block_bitmap(bmap, blk);
+			fs->group_desc[group+i].bg_inode_bitmap = blk;
+		}
+		break;
+
+	case ALLOC_INODE_TABLES:
+		for (i=0, blk=new_blk; i < count; i++, blk++) {
+			retval = ext2fs_get_free_blocks(fs, start_blk, last_blk,
+							fs->inode_blocks_per_group,
+							bmap, &new_blk);
+			if (retval)
+				return retval;
+			for (j=0, blk = new_blk;
+			     j < fs->inode_blocks_per_group; j++, blk++)
+				ext2fs_mark_block_bitmap(bmap, blk);
+			fs->group_desc[group+i].bg_inode_table = new_blk;
+		}
+		break;
+
+	}
+	return 0;
+}
+
+
+
+errcode_t ext2fs_allocate_flex_groups(ext2_filsys fs)
+{
+	errcode_t	retval;
+	blk_t		start, last, j, blocks;
+	dgrp_t		i, k;
+	int		gpm;
+
+	gpm = GROUPS_PER_FLEXBG;
+	blocks = 0;
+
+	for (i = 0; i < fs->group_desc_count; i=i+gpm) {
+		if (i == 0 )
+			start = ext2fs_group_first_block(fs,
+							 FIRST_METADATA_GROUP);
+		else
+			start = ext2fs_group_first_block(fs, i);
+
+		if (i+gpm-1 > fs->group_desc_count) {
+			last = ext2fs_group_last_block(fs, fs->group_desc_count);
+			gpm = fs->group_desc_count - i;
+		}
+		else
+			last = ext2fs_group_last_block(fs, i+gpm-1);
+
+		retval = ext2fs_allocate_contiguous(fs, i, ALLOC_BLOCK_BITMAPS,
+						    start, last, gpm,
+						    fs->block_map);
+		if (retval)
+			return retval;
+		retval = ext2fs_allocate_contiguous(fs, i, ALLOC_INODE_BITMAPS,
+						    start, last, gpm,
+						    fs->block_map);
+		if (retval)
+			return retval;
+		retval = ext2fs_allocate_contiguous(fs, i, ALLOC_INODE_TABLES,
+						    start, last, gpm,
+						    fs->block_map);
+		if (retval)
+			return retval;
+
+		/*
+		 * The content of bg_free_blocks_count is previously
+		 * assigned with out knowledge of the new allocation
+		 * scheme.  Need to update the number of free blocks
+		 * per group descriptor or fsck will complain.
+		 */
+
+		for (k=i; k<i+gpm; k++){
+			if (k > fs->group_desc_count)
+				break;
+			start = ext2fs_group_first_block(fs, k);
+			last = ext2fs_group_last_block(fs, k);
+			for (j=start; j<=last; j++) {
+				if( !ext2fs_fast_test_block_bitmap(fs->block_map, j))
+					blocks++;
+			}
+				fs->group_desc[k].bg_free_blocks_count = blocks;
+			blocks = 0;
+		}
+	}
+	return 0;
+}
+
 errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
 				      ext2fs_block_bitmap bmap)
 {
@@ -107,10 +231,16 @@ errcode_t ext2fs_allocate_tables(ext2_filsys fs)
 	errcode_t	retval;
 	dgrp_t		i;
 
-	for (i = 0; i < fs->group_desc_count; i++) {
-		retval = ext2fs_allocate_group_table(fs, i, fs->block_map);
-		if (retval)
-			return retval;
+	if (EXT2_HAS_INCOMPAT_FEATURE (fs->super, 
+				       EXT4_FEATURE_INCOMPAT_FLEX_BG)) 
+		ext2fs_allocate_flex_groups(fs);
+	
+	else {
+		for (i = 0; i < fs->group_desc_count; i++) {
+			retval = ext2fs_allocate_group_table(fs, i, fs->block_map);
+			if (retval)
+				return retval;
+		}
 	}
 	return 0;
 }

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Aneesh Kumar K.V]

Jose R. Santos wrote:
> From: Jose R. Santos <jrs@us.ibm.com>
> 
>
>  e2fsck/super.c          |   10 ++++++++--
>  lib/ext2fs/check_desc.c |   10 ++++++++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/e2fsck/super.c b/e2fsck/super.c
> index 00a131c..8e58e5c 100644
> --- a/e2fsck/super.c
> +++ b/e2fsck/super.c
> @@ -578,8 +578,14 @@ void check_super_block(e2fsck_t ctx)
>  	for (i = 0, gd=fs->group_desc; i < fs->group_desc_count; i++, gd++) {
>  		pctx.group = i;
> 
> -		first_block = ext2fs_group_first_block(fs, i);
> -		last_block = ext2fs_group_last_block(fs, i);
> +		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super,
> +			EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> +			first_block = fs->super->s_first_data_block;
> +			last_block = fs->super->s_blocks_count;


I guess this should be fs->super->s_blocks_count - 1 ;





> +		} else {
> +			first_block = ext2fs_group_first_block(fs, i);
> +			last_block = ext2fs_group_last_block(fs, i);
> +		}
> 
>  		if ((gd->bg_block_bitmap < first_block) ||
>  		    (gd->bg_block_bitmap > last_block)) {
> diff --git a/lib/ext2fs/check_desc.c b/lib/ext2fs/check_desc.c
> index 146f9e5..bb65c06 100644
> --- a/lib/ext2fs/check_desc.c
> +++ b/lib/ext2fs/check_desc.c
> @@ -38,8 +38,14 @@ errcode_t ext2fs_check_desc(ext2_filsys fs)
>  	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
> 
>  	for (i = 0; i < fs->group_desc_count; i++) {
> -		first_block = ext2fs_group_first_block(fs, i);
> -		last_block = ext2fs_group_last_block(fs, i);
> +		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super, EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> +			first_block = fs->super->s_first_data_block;
> +			last_block = fs->super->s_blocks_count;
> +		

I guess this should be fs->super->s_blocks_count - 1 ;

-aneesh

Re: [PATCH 4/4][e2fsprogs] New bitmap and inode table allocation for FLEX_BG

[Aneesh Kumar K.V]

Jose R. Santos wrote:
> From: Jose R. Santos <jrs@us.ibm.com>
> 
> Ne
> +	case ALLOC_INODE_TABLES:
> +		for (i=0, blk=new_blk; i < count; i++, blk++) {

I guess you can drop the blk update in the for() loop above. 

> +			retval = ext2fs_get_free_blocks(fs, start_blk, last_blk,
> +							fs->inode_blocks_per_group,
> +							bmap, &new_blk);
> +			if (retval)
> +				return retval;
> +			for (j=0, blk = new_blk;
> +			     j < fs->inode_blocks_per_group; j++, blk++)
> +				ext2fs_mark_block_bitmap(bmap, blk);
> +			fs->group_desc[group+i].bg_inode_table = new_blk;
> +		}
> +		break;
> +
> +	}
> +	return 0;
> +}
> +
> +
> +
> +errcode_t ext2fs_allocate_flex_groups(ext2_filsys fs)
> +{
> +	errcode_t	retval;
> +	blk_t		start, last, j, blocks;
> +	dgrp_t		i, k;
> +	int		gpm;
> +
> +	gpm = GROUPS_PER_FLEXBG;
> +	blocks = 0;
> +
> +	for (i = 0; i < fs->group_desc_count; i=i+gpm) {
> +		if (i == 0 )
> +			start = ext2fs_group_first_block(fs,
> +							 FIRST_METADATA_GROUP);
> +		else
> +			start = ext2fs_group_first_block(fs, i);
> +
> +		if (i+gpm-1 > fs->group_desc_count) {
		

		if (i+gpm >= fs->group_desc_count) 


> +			last = ext2fs_group_last_block(fs, fs->group_desc_count);
> +			gpm = fs->group_desc_count - i;
> +		}


-aneesh

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Jose R. Santos]

On Fri, 03 Aug 2007 10:54:04 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Jose R. Santos wrote:
> > From: Jose R. Santos <jrs@us.ibm.com>
> > 
> >
> >  e2fsck/super.c          |   10 ++++++++--
> >  lib/ext2fs/check_desc.c |   10 ++++++++--
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/e2fsck/super.c b/e2fsck/super.c
> > index 00a131c..8e58e5c 100644
> > --- a/e2fsck/super.c
> > +++ b/e2fsck/super.c
> > @@ -578,8 +578,14 @@ void check_super_block(e2fsck_t ctx)
> >  	for (i = 0, gd=fs->group_desc; i < fs->group_desc_count; i++, gd++) {
> >  		pctx.group = i;
> > 
> > -		first_block = ext2fs_group_first_block(fs, i);
> > -		last_block = ext2fs_group_last_block(fs, i);
> > +		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super,
> > +			EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> > +			first_block = fs->super->s_first_data_block;
> > +			last_block = fs->super->s_blocks_count;
> 
> 
> I guess this should be fs->super->s_blocks_count - 1 ;

Updated.

> > +		} else {
> > +			first_block = ext2fs_group_first_block(fs, i);
> > +			last_block = ext2fs_group_last_block(fs, i);
> > +		}
> > 
> >  		if ((gd->bg_block_bitmap < first_block) ||
> >  		    (gd->bg_block_bitmap > last_block)) {
> > diff --git a/lib/ext2fs/check_desc.c b/lib/ext2fs/check_desc.c
> > index 146f9e5..bb65c06 100644
> > --- a/lib/ext2fs/check_desc.c
> > +++ b/lib/ext2fs/check_desc.c
> > @@ -38,8 +38,14 @@ errcode_t ext2fs_check_desc(ext2_filsys fs)
> >  	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
> > 
> >  	for (i = 0; i < fs->group_desc_count; i++) {
> > -		first_block = ext2fs_group_first_block(fs, i);
> > -		last_block = ext2fs_group_last_block(fs, i);
> > +		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super, EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> > +			first_block = fs->super->s_first_data_block;
> > +			last_block = fs->super->s_blocks_count;
> > +		
> 
> I guess this should be fs->super->s_blocks_count - 1 ;

Updated.

Thanks
 
> -aneesh

-JRS

Re: [PATCH 4/4][e2fsprogs] New bitmap and inode table allocation for FLEX_BG

[Jose R. Santos]

On Fri, 03 Aug 2007 12:01:20 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:

> 
> 
> Jose R. Santos wrote:
> > From: Jose R. Santos <jrs@us.ibm.com>
> > 
> > Ne
> > +	case ALLOC_INODE_TABLES:
> > +		for (i=0, blk=new_blk; i < count; i++, blk++) {
> 
> I guess you can drop the blk update in the for() loop above. 

This was from the remains of the very first attempt at inode table
allocation.  Thanks for catching.

> 
> > +			retval = ext2fs_get_free_blocks(fs, start_blk, last_blk,
> > +							fs->inode_blocks_per_group,
> > +							bmap, &new_blk);
> > +			if (retval)
> > +				return retval;
> > +			for (j=0, blk = new_blk;
> > +			     j < fs->inode_blocks_per_group; j++, blk++)
> > +				ext2fs_mark_block_bitmap(bmap, blk);
> > +			fs->group_desc[group+i].bg_inode_table = new_blk;
> > +		}
> > +		break;
> > +
> > +	}
> > +	return 0;
> > +}
> > +
> > +
> > +
> > +errcode_t ext2fs_allocate_flex_groups(ext2_filsys fs)
> > +{
> > +	errcode_t	retval;
> > +	blk_t		start, last, j, blocks;
> > +	dgrp_t		i, k;
> > +	int		gpm;
> > +
> > +	gpm = GROUPS_PER_FLEXBG;
> > +	blocks = 0;
> > +
> > +	for (i = 0; i < fs->group_desc_count; i=i+gpm) {
> > +		if (i == 0 )
> > +			start = ext2fs_group_first_block(fs,
> > +							 FIRST_METADATA_GROUP);
> > +		else
> > +			start = ext2fs_group_first_block(fs, i);
> > +
> > +		if (i+gpm-1 > fs->group_desc_count) {
> 		
> 
> 		if (i+gpm >= fs->group_desc_count) 

Update.

> 
> > +			last = ext2fs_group_last_block(fs, fs->group_desc_count);
> > +			gpm = fs->group_desc_count - i;
> > +		}
> 
> 
> -aneesh

Thanks

-JRS

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Andreas Dilger]

On Aug 02, 2007  23:00 -0500, Jose R. Santos wrote:
> Eventually, a more thorough check would restrict bitmaps and inode
> tables to be located at the beginning of a flex block group range.
> Since the super block does not currently know about the number of
> groups per flex group, this will do for now.
 
As with regular block groups, it would probably be better to limit the
bitmaps and inode tables to anywhere inside the flexbg instead of the
start.  That would allow, for example, INCOMPAT_FLEXBG to be enabled
on an existing filesystem and the metadata could be moved together as
space becomes available.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Jose R. Santos]

On Fri, 3 Aug 2007 12:22:17 -0600
Andreas Dilger <adilger@clusterfs.com> wrote:
> On Aug 02, 2007  23:00 -0500, Jose R. Santos wrote:
> > Eventually, a more thorough check would restrict bitmaps and inode
> > tables to be located at the beginning of a flex block group range.
> > Since the super block does not currently know about the number of
> > groups per flex group, this will do for now.
> 
> As with regular block groups, it would probably be better to limit the
> bitmaps and inode tables to anywhere inside the flexbg instead of the
> start.  That would allow, for example, INCOMPAT_FLEXBG to be enabled
> on an existing filesystem and the metadata could be moved together as
> space becomes available.

This is something that would be simple to do if the ratio of block
groups per flex group known to the filesystem itself.  This implies
adding another field to the super block as reliable way to obtain this
information.  The only thing keeping me from doing so is the uncertainty
of backwards compatibility when changing the super block structure.

I agree though that one of the requirements for this feature is more
robust checking of the location of the bitmaps and inode tables within
the flex group.  Checking of the descriptor in flexbg become a little
more complicated than in regular block groups because:

1. The block and inode bitmaps should be allocated a one big chunk for
each flex group.

2. The block and inode bitmaps should be located in the first block
group and the inode tables with in the first few groups of a flex group.

3. If the full range of bitmaps in not allocated contiguously, this
means that bad blocks caused us to move a particular bitmap out and
thus the bad block list should be checked to ensure that this was the
case.

If the above conditions are not met, this could point to possible
corruption in the block descriptors.

> Cheers, Andreas
> --
> Andreas Dilger
> Principal Software Engineer
> Cluster File Systems, Inc.
> 

-JRS

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Theodore Tso]

On Mon, Nov 05, 2007 at 08:53:52AM -0600, Jose R. Santos wrote:
> Yes, the patch is slightly off and I did send an email pointing to
> exactly this, but you had already committed the patch by the time the
> email was sent.  This patch was submitted to the mailing list before
> the final kernel changes made it into the queue.

Feel free to resend patches series that are only in the pu series.
(Don't bother if it's just to fix merge changes, since I can do that
fairly easily; if I need help I'll people know.)  But if you want to
fix substantive issues, send me a new set of patches and I'll replace
them; the whole point of the "pu" branch is that it will rewind as
patches get refined and "cooked".

              	       	       	      	 - Ted

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Jose R. Santos]

On Sat, 3 Nov 2007 19:36:09 -0400
Theodore Tso <tytso@mit.edu> wrote:

> On Mon, Aug 13, 2007 at 11:33:03PM -0500, Jose R. Santos wrote:
> > From: Jose R. Santos <jrs@us.ibm.com>
> > 
> > Relax group descriptor checking.
> 
> This patch should really be before patch #2 in the series (add the
> ability to handle the new feature before adding the ability to add in
> mke2fs).  (Actually, I would have split up #2 into one patch which
> added the libe2p handling for the feature, then added the change to
> e2fsck, and then added the mke2fs changes, but that's just me
> quibbling.)

Sound reasonable.  I'll take care of patch ordering better next time.

> > @@ -578,8 +580,16 @@ void check_super_block(e2fsck_t ctx)
> >  	for (i = 0, gd=fs->group_desc; i < fs->group_desc_count; i++, gd++) {
> >  		pctx.group = i;
> >  
> > -		first_block = ext2fs_group_first_block(fs, i);
> > -		last_block = ext2fs_group_last_block(fs, i);
> > +		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super,
> > +					       EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> > +			meta_bg_size = (fs->blocksize / sizeof (struct ext2_group_desc));
> > +			start_group = (i / meta_bg_size) * meta_bg_size;
> > +			first_block = ext2fs_group_first_block(fs, start_group);
> > +			last_block = ext2fs_group_first_block(fs, start_group + meta_bg_size);
> 
> This patch requires that the metadata be in located in the metablock
> group descriptor, instead of anywhere in the filesystem, which is what
> we ultimately ended up checking into the kernel.  Being more flexible
> is good (even if that's not the layout we use by default).  I'll fix
> this up using git rebase --interactive and republish the patch in the
> next branch.

Yes, the patch is slightly off and I did send an email pointing to
exactly this, but you had already committed the patch by the time the
email was sent.  This patch was submitted to the mailing list before
the final kernel changes made it into the queue.

> 						- Ted

-JRS

Re: [PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Theodore Tso]

On Mon, Aug 13, 2007 at 11:33:03PM -0500, Jose R. Santos wrote:
> From: Jose R. Santos <jrs@us.ibm.com>
> 
> Relax group descriptor checking.

This patch should really be before patch #2 in the series (add the
ability to handle the new feature before adding the ability to add in
mke2fs).  (Actually, I would have split up #2 into one patch which
added the libe2p handling for the feature, then added the change to
e2fsck, and then added the mke2fs changes, but that's just me
quibbling.)

> @@ -578,8 +580,16 @@ void check_super_block(e2fsck_t ctx)
>  	for (i = 0, gd=fs->group_desc; i < fs->group_desc_count; i++, gd++) {
>  		pctx.group = i;
>  
> -		first_block = ext2fs_group_first_block(fs, i);
> -		last_block = ext2fs_group_last_block(fs, i);
> +		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super,
> +					       EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> +			meta_bg_size = (fs->blocksize / sizeof (struct ext2_group_desc));
> +			start_group = (i / meta_bg_size) * meta_bg_size;
> +			first_block = ext2fs_group_first_block(fs, start_group);
> +			last_block = ext2fs_group_first_block(fs, start_group + meta_bg_size);

This patch requires that the metadata be in located in the metablock
group descriptor, instead of anywhere in the filesystem, which is what
we ultimately ended up checking into the kernel.  Being more flexible
is good (even if that's not the layout we use by default).  I'll fix
this up using git rebase --interactive and republish the patch in the
next branch.

						- Ted

[PATCH 3/4][e2fsprogs] Relax group descriptor checking.

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Relax group descriptor checking.

In order for tools such as dump2efs, e2fsck and debugfs to open a ext4
filesystem with FLEX_BG feature enable, some descriptor checking needs
to be relaxed.  This patch changes the group desciptor checking so
that bitmaps and inode tables can be located anywhere in the
partitions block range.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 e2fsck/super.c          |   14 ++++++++++++--
 lib/ext2fs/check_desc.c |   15 +++++++++++++--
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index 00a131c..ed28732 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -463,6 +463,8 @@ void check_super_block(e2fsck_t ctx)
 	int	inodes_per_block;
 	int	ipg_max;
 	int	inode_size;
+	dgrp_t	start_group;
+	int 	meta_bg_size;
 	dgrp_t	i;
 	blk_t	should_be;
 	struct problem_context	pctx;
@@ -578,8 +580,16 @@ void check_super_block(e2fsck_t ctx)
 	for (i = 0, gd=fs->group_desc; i < fs->group_desc_count; i++, gd++) {
 		pctx.group = i;
 
-		first_block = ext2fs_group_first_block(fs, i);
-		last_block = ext2fs_group_last_block(fs, i);
+		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super,
+					       EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
+			meta_bg_size = (fs->blocksize / sizeof (struct ext2_group_desc));
+			start_group = (i / meta_bg_size) * meta_bg_size;
+			first_block = ext2fs_group_first_block(fs, start_group);
+			last_block = ext2fs_group_first_block(fs, start_group + meta_bg_size);
+		} else {
+			first_block = ext2fs_group_first_block(fs, i);
+			last_block = ext2fs_group_last_block(fs, i);
+		}
 
 		if ((gd->bg_block_bitmap < first_block) ||
 		    (gd->bg_block_bitmap > last_block)) {
diff --git a/lib/ext2fs/check_desc.c b/lib/ext2fs/check_desc.c
index 146f9e5..dbbcfb3 100644
--- a/lib/ext2fs/check_desc.c
+++ b/lib/ext2fs/check_desc.c
@@ -34,12 +34,23 @@ errcode_t ext2fs_check_desc(ext2_filsys fs)
 	dgrp_t i;
 	blk_t first_block = fs->super->s_first_data_block;
 	blk_t last_block;
+	dgrp_t start_group;
+	int meta_bg_size;
 
 	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
 
 	for (i = 0; i < fs->group_desc_count; i++) {
-		first_block = ext2fs_group_first_block(fs, i);
-		last_block = ext2fs_group_last_block(fs, i);
+		if (EXT2_HAS_INCOMPAT_FEATURE (fs->super, 
+					       EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
+			meta_bg_size = (fs->blocksize / sizeof (struct ext2_group_desc));
+			start_group = (i / meta_bg_size) * meta_bg_size;
+			first_block = ext2fs_group_first_block(fs, start_group);
+			last_block = ext2fs_group_first_block(fs, start_group + meta_bg_size);
+		}
+		else {
+			first_block = ext2fs_group_first_block(fs, i);
+			last_block = ext2fs_group_last_block(fs, i);
+		}
 
 		/*
 		 * Check to make sure block bitmap for group is