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@xxxxxxx> --- 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); -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
