Re: s_first_meta_bg treatment incompatibility between kernel and e2fsprogs

[Theodore Tso <tytso@mit.edu>]

On Sun, Nov 15, 2009 at 11:28:13AM +0100, Damien Guibouret wrote:
> I've open a kernel bug since:  
> http://bugzilla.kernel.org/show_bug.cgi?id=14601
> with a proposed patch (little different from yours but it is matter of  
> taste :)

For future references, patches are less likely to slip through the
cracks if they are sent to the linux-ext4 mailing list as opposed to
having a BZ bug opened.  (Yeah, I know, that's unusual).  The reason
for that is that patches are tracked via patchwork, here:

	http://patchwork.ozlabs.org/project/linux-ext4

Basically, anything that looks like a patch which is sent to
linux-ext4 gets snagged by patchwork, and it's a good place to look
for stuff that hasn't yet been merged.  In some cases there are good
reasons why a patch has been kept out, and in other cases patches have
been merged or definitely rejected and I don't get to getting that
status updated in patchwork, but overall I've found it to work very
well.

As far as the matter of taste issue is concerned, I think we already
have too many static functions with a single caller, and it actually
makes the code harder to understand.  So adding yet another simple
static function I think is a bad thing, not a good thing.

> And I think there is some other places where kernel should be fixed when  
> it uses s_gdb_count (but here my knowledge of the sources are not deep  
> enough to be sure on what shall be performed).

I've looked through the other areas, and the one place where I see a
problem is in the block validity checks in ext4_iget() for the
extended attribute block and in block_validity.c.  The former can and
should be fixed to use the latter.

Here's the fix that I plan to be using.   Comments, anyone?

							- Ted

ext4: fix block validity checks so they work correctly with meta_bg

The block validity checks used by ext4_data_block_valid() wasn't
correctly written to check file systems with the meta_bg feature.  Fix
this.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/block_validity.c |    2 +-
 fs/ext4/inode.c          |    5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
index 50784ef..dc79b75 100644
--- a/fs/ext4/block_validity.c
+++ b/fs/ext4/block_validity.c
@@ -160,7 +160,7 @@ int ext4_setup_system_zone(struct super_block *sb)
 		if (ext4_bg_has_super(sb, i) &&
 		    ((i < 5) || ((i % flex_size) == 0)))
 			add_system_zone(sbi, ext4_group_first_block_no(sb, i),
-					sbi->s_gdb_count + 1);
+					ext4_bg_num_gdb(sb, i) + 1);
 		gdp = ext4_get_group_desc(sb, i, NULL);
 		ret = add_system_zone(sbi, ext4_block_bitmap(sb, gdp), 1);
 		if (ret)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b5cdb88..c62ca93 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4886,10 +4886,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 
 	ret = 0;
 	if (ei->i_file_acl &&
-	    ((ei->i_file_acl <
-	      (le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block) +
-	       EXT4_SB(sb)->s_gdb_count)) ||
-	     (ei->i_file_acl >= ext4_blocks_count(EXT4_SB(sb)->s_es)))) {
+	    !ext4_data_block_valid(EXT4_SB(sb), ei->i_file_acl, 1)) {
 		ext4_error(sb, __func__,
 			   "bad extended attribute block %llu in inode #%lu",
 			   ei->i_file_acl, inode->i_ino);