On Mon, 15 Oct 2012, Tao Ma wrote:
> Date: Mon, 15 Oct 2012 21:01:33 +0800
> From: Tao Ma <tm@xxxxxx>
> To: linux-ext4@xxxxxxxxxxxxxxx
> Cc: Theodore Ts'o <tytso@xxxxxxx>
> Subject: [PATCH] ext4: Checksum the block bitmap properly with bigalloc
> enabled.
>
> From: Tao Ma <boyu.mt@xxxxxxxxxx>
>
> In mke2fs, we only checksum the whole bitmap block and it is right.
> While in the kernel, we use EXT4_BLOCKS_PER_GROUP to indicate the
> size of the checksumed bitmap which is wrong when we enable bigalloc.
> The right size should be EXT4_CLUSTERS_PER_GROUP and this patch fixes
> it.
>
> Also as every caller of ext4_block_bitmap_csum_set and
> ext4_block_bitmap_csum_verify pass in EXT4_BLOCKS_PER_GROUP(sb)/8,
> we'd better removes this parameter and sets it in the function itself.
Looks good to me.
Reviewed-by: Lukas Czerner <lczerner@xxxxxxxxxx>
Thanks!
-Lukas
>
> Cc: "Theodore Ts'o" <tytso@xxxxxxx>
> Signed-off-by: Tao Ma <boyu.mt@xxxxxxxxxx>
> ---
> fs/ext4/balloc.c | 8 +++-----
> fs/ext4/bitmap.c | 6 ++++--
> fs/ext4/ext4.h | 4 ++--
> fs/ext4/ialloc.c | 4 +---
> fs/ext4/mballoc.c | 9 +++------
> fs/ext4/resize.c | 3 +--
> 6 files changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 1b50890..cf18217 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -174,8 +174,7 @@ void ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
> ext4_free_inodes_set(sb, gdp, 0);
> ext4_itable_unused_set(sb, gdp, 0);
> memset(bh->b_data, 0xff, sb->s_blocksize);
> - ext4_block_bitmap_csum_set(sb, block_group, gdp, bh,
> - EXT4_BLOCKS_PER_GROUP(sb) / 8);
> + ext4_block_bitmap_csum_set(sb, block_group, gdp, bh);
> return;
> }
> memset(bh->b_data, 0, sb->s_blocksize);
> @@ -212,8 +211,7 @@ void ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
> */
> ext4_mark_bitmap_end(num_clusters_in_group(sb, block_group),
> sb->s_blocksize * 8, bh->b_data);
> - ext4_block_bitmap_csum_set(sb, block_group, gdp, bh,
> - EXT4_BLOCKS_PER_GROUP(sb) / 8);
> + ext4_block_bitmap_csum_set(sb, block_group, gdp, bh);
> ext4_group_desc_csum_set(sb, block_group, gdp);
> }
>
> @@ -350,7 +348,7 @@ void ext4_validate_block_bitmap(struct super_block *sb,
> return;
> }
> if (unlikely(!ext4_block_bitmap_csum_verify(sb, block_group,
> - desc, bh, EXT4_BLOCKS_PER_GROUP(sb) / 8))) {
> + desc, bh))) {
> ext4_unlock_group(sb, block_group);
> ext4_error(sb, "bg %u: bad block bitmap checksum", block_group);
> return;
> diff --git a/fs/ext4/bitmap.c b/fs/ext4/bitmap.c
> index 5c2d181..3285aa5 100644
> --- a/fs/ext4/bitmap.c
> +++ b/fs/ext4/bitmap.c
> @@ -58,11 +58,12 @@ void ext4_inode_bitmap_csum_set(struct super_block *sb, ext4_group_t group,
>
> int ext4_block_bitmap_csum_verify(struct super_block *sb, ext4_group_t group,
> struct ext4_group_desc *gdp,
> - struct buffer_head *bh, int sz)
> + struct buffer_head *bh)
> {
> __u32 hi;
> __u32 provided, calculated;
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> + int sz = EXT4_CLUSTERS_PER_GROUP(sb) / 8;
>
> if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
> @@ -84,8 +85,9 @@ int ext4_block_bitmap_csum_verify(struct super_block *sb, ext4_group_t group,
>
> void ext4_block_bitmap_csum_set(struct super_block *sb, ext4_group_t group,
> struct ext4_group_desc *gdp,
> - struct buffer_head *bh, int sz)
> + struct buffer_head *bh)
> {
> + int sz = EXT4_CLUSTERS_PER_GROUP(sb) / 8;
> __u32 csum;
> struct ext4_sb_info *sbi = EXT4_SB(sb);
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 3ab2539..d6024d5 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1882,10 +1882,10 @@ int ext4_inode_bitmap_csum_verify(struct super_block *sb, ext4_group_t group,
> struct buffer_head *bh, int sz);
> void ext4_block_bitmap_csum_set(struct super_block *sb, ext4_group_t group,
> struct ext4_group_desc *gdp,
> - struct buffer_head *bh, int sz);
> + struct buffer_head *bh);
> int ext4_block_bitmap_csum_verify(struct super_block *sb, ext4_group_t group,
> struct ext4_group_desc *gdp,
> - struct buffer_head *bh, int sz);
> + struct buffer_head *bh);
>
> /* balloc.c */
> extern void ext4_validate_block_bitmap(struct super_block *sb,
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index fa36372..4facdd2 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -762,9 +762,7 @@ got:
> ext4_free_group_clusters_set(sb, gdp,
> ext4_free_clusters_after_init(sb, group, gdp));
> ext4_block_bitmap_csum_set(sb, group, gdp,
> - block_bitmap_bh,
> - EXT4_BLOCKS_PER_GROUP(sb) /
> - 8);
> + block_bitmap_bh);
> ext4_group_desc_csum_set(sb, group, gdp);
> }
> ext4_unlock_group(sb, group);
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index f8b27bf..5804273 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2805,8 +2805,7 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
> }
> len = ext4_free_group_clusters(sb, gdp) - ac->ac_b_ex.fe_len;
> ext4_free_group_clusters_set(sb, gdp, len);
> - ext4_block_bitmap_csum_set(sb, ac->ac_b_ex.fe_group, gdp, bitmap_bh,
> - EXT4_BLOCKS_PER_GROUP(sb) / 8);
> + ext4_block_bitmap_csum_set(sb, ac->ac_b_ex.fe_group, gdp, bitmap_bh);
> ext4_group_desc_csum_set(sb, ac->ac_b_ex.fe_group, gdp);
>
> ext4_unlock_group(sb, ac->ac_b_ex.fe_group);
> @@ -4666,8 +4665,7 @@ do_more:
>
> ret = ext4_free_group_clusters(sb, gdp) + count_clusters;
> ext4_free_group_clusters_set(sb, gdp, ret);
> - ext4_block_bitmap_csum_set(sb, block_group, gdp, bitmap_bh,
> - EXT4_BLOCKS_PER_GROUP(sb) / 8);
> + ext4_block_bitmap_csum_set(sb, block_group, gdp, bitmap_bh);
> ext4_group_desc_csum_set(sb, block_group, gdp);
> ext4_unlock_group(sb, block_group);
> percpu_counter_add(&sbi->s_freeclusters_counter, count_clusters);
> @@ -4811,8 +4809,7 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
> mb_free_blocks(NULL, &e4b, bit, count);
> blk_free_count = blocks_freed + ext4_free_group_clusters(sb, desc);
> ext4_free_group_clusters_set(sb, desc, blk_free_count);
> - ext4_block_bitmap_csum_set(sb, block_group, desc, bitmap_bh,
> - EXT4_BLOCKS_PER_GROUP(sb) / 8);
> + ext4_block_bitmap_csum_set(sb, block_group, desc, bitmap_bh);
> ext4_group_desc_csum_set(sb, block_group, desc);
> ext4_unlock_group(sb, block_group);
> percpu_counter_add(&sbi->s_freeclusters_counter,
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 7a75e10..47bf06a 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1212,8 +1212,7 @@ static int ext4_set_bitmap_checksums(struct super_block *sb,
> bh = ext4_get_bitmap(sb, group_data->block_bitmap);
> if (!bh)
> return -EIO;
> - ext4_block_bitmap_csum_set(sb, group, gdp, bh,
> - EXT4_BLOCKS_PER_GROUP(sb) / 8);
> + ext4_block_bitmap_csum_set(sb, group, gdp, bh);
> brelse(bh);
>
> return 0;
>
--
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
