* [PATCH] ext4: Checksum the block bitmap properly with bigalloc enabled.
@ 2012-10-15 13:01 Tao Ma
2012-10-16 7:05 ` Lukáš Czerner
2012-10-22 4:33 ` Theodore Ts'o
0 siblings, 2 replies; 3+ messages in thread
From: Tao Ma @ 2012-10-15 13:01 UTC (permalink / raw)
To: linux-ext4; +Cc: Theodore Ts'o
From: Tao Ma <boyu.mt@taobao.com>
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.
Cc: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
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;
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ext4: Checksum the block bitmap properly with bigalloc enabled.
2012-10-15 13:01 [PATCH] ext4: Checksum the block bitmap properly with bigalloc enabled Tao Ma
@ 2012-10-16 7:05 ` Lukáš Czerner
2012-10-22 4:33 ` Theodore Ts'o
1 sibling, 0 replies; 3+ messages in thread
From: Lukáš Czerner @ 2012-10-16 7:05 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4, Theodore Ts'o
On Mon, 15 Oct 2012, Tao Ma wrote:
> Date: Mon, 15 Oct 2012 21:01:33 +0800
> From: Tao Ma <tm@tao.ma>
> To: linux-ext4@vger.kernel.org
> Cc: Theodore Ts'o <tytso@mit.edu>
> Subject: [PATCH] ext4: Checksum the block bitmap properly with bigalloc
> enabled.
>
> From: Tao Ma <boyu.mt@taobao.com>
>
> 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@redhat.com>
Thanks!
-Lukas
>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
> ---
> 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;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ext4: Checksum the block bitmap properly with bigalloc enabled.
2012-10-15 13:01 [PATCH] ext4: Checksum the block bitmap properly with bigalloc enabled Tao Ma
2012-10-16 7:05 ` Lukáš Czerner
@ 2012-10-22 4:33 ` Theodore Ts'o
1 sibling, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2012-10-22 4:33 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4
On Mon, Oct 15, 2012 at 09:01:33PM +0800, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> 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.
>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
Thanks, applied!
- Ted
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-22 4:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-15 13:01 [PATCH] ext4: Checksum the block bitmap properly with bigalloc enabled Tao Ma
2012-10-16 7:05 ` Lukáš Czerner
2012-10-22 4:33 ` Theodore Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).