On Wed, Jun 03, 2009 at 12:16:08PM -0600, Andreas Dilger wrote:
> On Jun 02, 2009 08:07 -0400, Theodore Ts'o wrote:
> > + 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)))) {
>
> I was just thinking it might make sense to wrap this check into a helper
> like the following. We check the validity of blocks in at least half a
> dozen different places. The elaborate ext4_blocktype is to allow for
> future expansion of this checking mechanism to allow it to check for
> blocks overlapping with e.g. the inode table and such, and possibly for
> using with the jbd2 buffer checksum mechanism at some later date.
We do have a helper function that is waiting to be merged in the patch
queue. See the patch "add-check-block-validity-to-ext4_get_blocks_wrap".
It doesn't have the blocktype extension, since to keep things fast and
simple, I have a single red-black tree for any blocks that shouldn't
be used for file blocks allows for a *much* more compact
representation in the red-black tree, thanks to flex_bg putting the
block and inode bitmaps and inode tables back-to-back with each other.
If I were to add blocktype information to the red-black tree that
ext4_data_block_valid() could check against, the red-black tree would
at least triple in size.
The nice thing about this patch (which will be merged for 2.6.31) is
that it's a runtime mount option. So if we have a customer that runs
into problems, we don't have to ship them a custom debugging kernel;
we just tell them to mount the filesystem with block_validity, and we
can start debugging the problem right away.
- Ted
--
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
