diff mbox series

[7/7] ext4: don't update checksum of new initialized bitmaps

Message ID 20180220023038.19883-8-tytso@mit.edu
State Accepted, archived
Headers show
Series ext4: fix up shutdown handling | expand

Commit Message

Theodore Ts'o Feb. 20, 2018, 2:30 a.m. UTC 
When reading the inode or block allocation bitmap, if the bitmap needs
to be initialized, do not update the checksum in the block group
descriptor.  That's because we're not set up to journal those changes.
Instead, just set the verified bit on the bitmap block, so that it's
not necessary to validate the checksum.

When a block or inode allocation actually happens, at that point the
checksum will be calculated, and update of the bg descriptor block
will be properly journalled.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
---
 fs/ext4/balloc.c |  3 +--
 fs/ext4/ialloc.c | 47 +++--------------------------------------------
 2 files changed, 4 insertions(+), 46 deletions(-)

Comments

Jan Kara March 7, 2018, 9:06 a.m. UTC | #1 
On Mon 19-02-18 21:30:38, Theodore Ts'o wrote:
> When reading the inode or block allocation bitmap, if the bitmap needs
> to be initialized, do not update the checksum in the block group
> descriptor.  That's because we're not set up to journal those changes.
> Instead, just set the verified bit on the bitmap block, so that it's
> not necessary to validate the checksum.
> 
> When a block or inode allocation actually happens, at that point the
> checksum will be calculated, and update of the bg descriptor block
> will be properly journalled.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Cc: stable@vger.kernel.org
> ---
>  fs/ext4/balloc.c |  3 +--
>  fs/ext4/ialloc.c | 47 +++--------------------------------------------
>  2 files changed, 4 insertions(+), 46 deletions(-)
> 
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index f9b3e0a83526..f82c4966f4ce 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -243,8 +243,6 @@ static int ext4_init_block_bitmap(struct super_block *sb,
>  	 */
>  	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_group_desc_csum_set(sb, block_group, gdp);
>  	return 0;
>  }

Probably you should remove the bad checksum handling in
ext4_init_block_bitmap() the same way as you did in
ext4_init_inode_bitmap()? Otherwise the patch looks good.

								Honza
Theodore Ts'o March 22, 2018, 3:29 p.m. UTC | #2 
On Wed, Mar 07, 2018 at 10:06:07AM +0100, Jan Kara wrote:
> 
> Probably you should remove the bad checksum handling in
> ext4_init_block_bitmap() the same way as you did in
> ext4_init_inode_bitmap()? Otherwise the patch looks good.

There isn't an ext4_init_inode_bitmap() --- the functionality is
inlined ext4_read_inode_bitmap(), and it's already doing it that way
(it doesn't set the checksum; it just sets the verified bit).

    	    	    	      	      - Ted
diff mbox series

Patch

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index f9b3e0a83526..f82c4966f4ce 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -243,8 +243,6 @@  static int ext4_init_block_bitmap(struct super_block *sb,
 	 */
 	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_group_desc_csum_set(sb, block_group, gdp);
 	return 0;
 }
 
@@ -448,6 +446,7 @@  ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 		err = ext4_init_block_bitmap(sb, bh, block_group, desc);
 		set_bitmap_uptodate(bh);
 		set_buffer_uptodate(bh);
+		set_buffer_verified(bh);
 		ext4_unlock_group(sb, block_group);
 		unlock_buffer(bh);
 		if (err) {
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 7830d28df331..3fa93665b4a3 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -66,44 +66,6 @@  void ext4_mark_bitmap_end(int start_bit, int end_bit, char *bitmap)
 		memset(bitmap + (i >> 3), 0xff, (end_bit - i) >> 3);
 }
 
-/* Initializes an uninitialized inode bitmap */
-static int ext4_init_inode_bitmap(struct super_block *sb,
-				       struct buffer_head *bh,
-				       ext4_group_t block_group,
-				       struct ext4_group_desc *gdp)
-{
-	struct ext4_group_info *grp;
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	J_ASSERT_BH(bh, buffer_locked(bh));
-
-	/* If checksum is bad mark all blocks and inodes use to prevent
-	 * allocation, essentially implementing a per-group read-only flag. */
-	if (!ext4_group_desc_csum_verify(sb, block_group, gdp)) {
-		grp = ext4_get_group_info(sb, block_group);
-		if (!EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
-			percpu_counter_sub(&sbi->s_freeclusters_counter,
-					   grp->bb_free);
-		set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
-		if (!EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) {
-			int count;
-			count = ext4_free_inodes_count(sb, gdp);
-			percpu_counter_sub(&sbi->s_freeinodes_counter,
-					   count);
-		}
-		set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
-		return -EFSBADCRC;
-	}
-
-	memset(bh->b_data, 0, (EXT4_INODES_PER_GROUP(sb) + 7) / 8);
-	ext4_mark_bitmap_end(EXT4_INODES_PER_GROUP(sb), sb->s_blocksize * 8,
-			bh->b_data);
-	ext4_inode_bitmap_csum_set(sb, block_group, gdp, bh,
-				   EXT4_INODES_PER_GROUP(sb) / 8);
-	ext4_group_desc_csum_set(sb, block_group, gdp);
-
-	return 0;
-}
-
 void ext4_end_bitmap_read(struct buffer_head *bh, int uptodate)
 {
 	if (uptodate) {
@@ -187,17 +149,14 @@  ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 
 	ext4_lock_group(sb, block_group);
 	if (desc->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
-		err = ext4_init_inode_bitmap(sb, bh, block_group, desc);
+		memset(bh->b_data, 0, (EXT4_INODES_PER_GROUP(sb) + 7) / 8);
+		ext4_mark_bitmap_end(EXT4_INODES_PER_GROUP(sb),
+				     sb->s_blocksize * 8, bh->b_data);
 		set_bitmap_uptodate(bh);
 		set_buffer_uptodate(bh);
 		set_buffer_verified(bh);
 		ext4_unlock_group(sb, block_group);
 		unlock_buffer(bh);
-		if (err) {
-			ext4_error(sb, "Failed to init inode bitmap for group "
-				   "%u: %d", block_group, err);
-			goto out;
-		}
 		return bh;
 	}
 	ext4_unlock_group(sb, block_group);