Re: ext3: bogus i_mode errors with 2.6.18.1

[Andreas Dilger <adilger@clusterfs.com>]

On Oct 30, 2006  10:55 +0100, Andre Noll wrote:
> Note that the patch does not address the EXT3_FEATURE_INCOMPAT_META_BG
> case yet. I'll have to look at this in more detail.

See more below...  Basically, the patch looks good enough to submit for
inclusion.  Can you CC it to Andrew on the next iteration.

> +static inline int block_needs_fix(ext3_fsblk_t block,
> +		unsigned long num,
> +		struct ext3_group_desc *gdp,
> +		struct super_block *sb)
> +{
> +	if (in_range(le32_to_cpu(gdp->bg_block_bitmap), block, num))
> +		return 1;
> +	if (in_range(le32_to_cpu(gdp->bg_inode_bitmap), block, num))
> +		return 1;
> +	if (in_range(block, le32_to_cpu(gdp->bg_inode_table),
> +			EXT3_SB(sb)->s_itb_per_group))
> +		return 1;
> +	if (in_range(block + num - 1, le32_to_cpu(gdp->bg_inode_table),
> +			EXT3_SB(sb)->s_itb_per_group))
> +		return 1;

This _should_ probably also include a check for the backup group descriptors
and superblock, but unfortunately the helpers ext3_bg_has_super() and
ext3_bg_num_gdb() are fairly CPU intensive and not something you want to
check for each block that is allocated/freed.  In the future (mballoc) when
we have per-group data structs we can just store a flag per group whether
it has a backup or not instead of recomputing the powers of 3, 5, 7 each time.
Maybe a comment for now to indicate that needs to be fixed in the future?


Also a minor nit - this is actually checking "num" blocks, and it might be
useful in the future so it would be clearer to call it something like
ext3_blocks_are_metadata().

> +static void fix_group(int group, struct super_block *sb)
> +{
> +	int i;
> +	ext3_fsblk_t bit;
> +	unsigned long gdblocks;
> +	struct buffer_head *gdp_bh;
> +	struct ext3_group_desc *gdp = ext3_get_group_desc(sb, group, &gdp_bh);
> +
> +	if (ext3_bg_has_super(sb, group))
> +		ext3_set_bit(0, gdp_bh->b_data);
> +	gdblocks = ext3_bg_num_gdb(sb, group);
> +	for (i = 0, bit = 1; i < gdblocks; i++, bit++)
> +		/* actually a bit more complex for INCOMPAT_META_BG fs */
> +		ext3_set_bit(i, gdp_bh->b_data);

Actually, in newer kernels ext3_bg_num_gdb() includes INCOMPAT_META_BG
support, so that may be enough.


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