On Aug 22, 2007 22:41 -0700, Avantika Mathur wrote:
> From: Andreas Dilger <adilger@xxxxxxxxxxxxx>
> Signed-off-by: Avantika Mathur <mathur@xxxxxxxxxx>
Please change this to a Signed-off-by: for me.
> +unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
> + int block_group, struct ext4_group_desc *gdp)
> +{
> + /* Set bits for block and inode bitmaps, and inode table */
> + ext4_set_bit(le32_to_cpu(gdp->bg_block_bitmap) - start,
> + bh->b_data);
> + ext4_set_bit(le32_to_cpu(gdp->bg_inode_bitmap) - start,
> + bh->b_data);
These need to be wrapped as ext4_{block,inode}_bitmap(sb, gdp).
One minor thing I never got around to implementing here is to handle the
last group is marked BLOGK_UNINIT. This shouldn't happen in real life,
because e2fsprogs will never create such a filesystem, but using the
mark_bitmap_end() code (which should be moved to a common location, it
is already duplicated from resize.c) as in ext4_init_inode_bitmap() is
pretty trivial.
That could be done as a separate patch to avoid delaying this one as
it does not affect any normal operation.
> @@ -116,10 +186,22 @@ read_block_bitmap(struct super_block *sb
> - bh = sb_bread(sb, ext4_block_bitmap(sb, desc));
> + if (desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
> + bh = sb_getblk(sb, le32_to_cpu(desc->bg_block_bitmap));
Needs to stay as ext4_block_bitmap(sb, desc) as it was before.
> + } else {
> + bh = sb_bread(sb, le32_to_cpu(desc->bg_block_bitmap));
> + }
As above.
> +unsigned ext4_init_inode_bitmap(struct super_block *sb,
> + struct buffer_head *bh, int block_group,
> + struct ext4_group_desc *gdp)
> +{
> + /* If checksum is bad mark all blocks and inodes use to prevent
> + * allocation, essentially implementing a per-group read-only flag. */
s/use/used/
> - bh = sb_bread(sb, ext4_inode_bitmap(sb, desc));
> + if (desc->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
> + bh = sb_getblk(sb, le32_to_cpu(desc->bg_inode_bitmap));
Needs to stay as ext4_inode_bitmap(sb, desc)
> + } else {
> + bh = sb_bread(sb, le32_to_cpu(desc->bg_inode_bitmap));
> + }
As above.
> +__le16 ext4_group_desc_csum(struct ext4_sb_info *sbi, __u32 block_group,
> + struct ext4_group_desc *gdp)
> +{
> + crc = crc16(crc, (__u8 *)gdp + offset,
> + le16_to_cpu(sbi->s_es->s_desc_size)
> + - offset);
Please put '-' operator at end of the previous line.
> +int ext4_group_desc_csum_verify(struct ext4_sb_info *sbi, __u32 block_group,
> + struct ext4_group_desc *gdp)
> +{
> + if ((sbi->s_es->s_feature_ro_compat &
> + cpu_to_le32(EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) &&
> + (gdp->bg_checksum != ext4_group_desc_csum(sbi, block_group, gdp)))
This line should be indented to the left one space, since it is a different
condition than "(sbi->s_es ...)".
> struct ext4_group_desc
> {
> __u16 bg_flags;
Can you please add a comment for this /* EXT4_BG_flags (INODE_UNINIT, etc) */
> - __u32 bg_reserved[3];
> + __u32 bg_reserved[2];
Likewise, for the above /* Likely block/inode bitmap checksum */
> + __le16 bg_itable_unused; /* Unused inodes count */
> + __le16 bg_checksum; /* crc16(sb_uuid+group+desc) */
The above are using spaces instead of tabs for alignment of the comments?
> +#define EXT4_BG_INODE_UNINIT 0x0001 /* Inode table/bitmap not in use */
> +#define EXT4_BG_BLOCK_UNINIT 0x0002 /* Block bitmap not in use */
> +#define EXT4_BG_INODE_ZEROED 0x0004 /* On-disk itable initialized to zero */
Likewise, the above have spaces instead of tabs for alignment.
> @@ -734,6 +740,7 @@ static inline int ext4_valid_inum(struct
> #define EXT4_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
> +#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010
> #define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
> #define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE 0x0040
>
> @@ -755,6 +762,7 @@ static inline int ext4_valid_inum(struct
> EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \
> EXT4_FEATURE_RO_COMPAT_DIR_NLINK | \
> EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE | \
> + EXT4_FEATURE_RO_COMPAT_GDT_CSUM| \
This should probably be put before DIR_NLINK to keep numerical order.
I think this is ready to be added to the patch queue. Ted, if you could
merge in the uninit groups patches to e2fsprogs that would be much
appreciated (which incidentally fixes the LAZY_BG-with-RESIZE_INODE support
and is needed for Jose's FLEX_BG too).
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
-
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
