On Fri, 11 Jul 2014, Theodore Ts'o wrote:
> Date: Fri, 11 Jul 2014 17:52:36 -0400
> From: Theodore Ts'o <tytso@xxxxxxx>
> To: Ext4 Developers List <linux-ext4@xxxxxxxxxxxxxxx>
> Cc: Theodore Ts'o <tytso@xxxxxxx>
> Subject: [PATCH] ext4: revert commit which was causing fs corruption after
> journal replays
>
> Commit 007649375f6af2 ("ext4: initialize multi-block allocator before
> checking block descriptors") causes the block group descriptor's count
> of the number of free blocks to become inconsistent with the number of
> free blocks in the allocation bitmap. This is a harmless form of fs
> corruption, but it causes the kernel to potentially remount the file
> system read-only, or to panic, depending on the file systems's error
> behavior.
>
> Thanks to Eric Whitney for his tireless work to reproduce and to find
> the guilty commit.
>
> Fixes: 007649375f6af2 ("ext4: initialize multi-block allocator before checking block descriptors"
Reviewed-by: Lukas Czerner <lczerner@xxxxxxxxxx>
>
> Cc: stable@xxxxxxxxxxxxxxx # 3.15
> Reported-by: David Jander <david@xxxxxxxxxxx>
> Reported-by: Matteo Croce <technoboy85@xxxxxxxxx>
> Tested-by: Eric Whitney <enwlinux@xxxxxxxxx>
> Suggested-by: Eric Whitney <enwlinux@xxxxxxxxx>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> ---
> fs/ext4/super.c | 51 ++++++++++++++++++++++++---------------------------
> 1 file changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6297c07..6df7bc6 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3879,38 +3879,19 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> goto failed_mount2;
> }
> }
> -
> - /*
> - * set up enough so that it can read an inode,
> - * and create new inode for buddy allocator
> - */
> - sbi->s_gdb_count = db_count;
> - if (!test_opt(sb, NOLOAD) &&
> - EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
> - sb->s_op = &ext4_sops;
> - else
> - sb->s_op = &ext4_nojournal_sops;
> -
> - ext4_ext_init(sb);
> - err = ext4_mb_init(sb);
> - if (err) {
> - ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
> - err);
> - goto failed_mount2;
> - }
> -
> if (!ext4_check_descriptors(sb, &first_not_zeroed)) {
> ext4_msg(sb, KERN_ERR, "group descriptors corrupted!");
> - goto failed_mount2a;
> + goto failed_mount2;
> }
> if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
> if (!ext4_fill_flex_info(sb)) {
> ext4_msg(sb, KERN_ERR,
> "unable to initialize "
> "flex_bg meta info!");
> - goto failed_mount2a;
> + goto failed_mount2;
> }
>
> + sbi->s_gdb_count = db_count;
> get_random_bytes(&sbi->s_next_generation, sizeof(u32));
> spin_lock_init(&sbi->s_next_gen_lock);
>
> @@ -3945,6 +3926,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> sbi->s_stripe = ext4_get_stripe_size(sbi);
> sbi->s_extent_max_zeroout_kb = 32;
>
> + /*
> + * set up enough so that it can read an inode
> + */
> + if (!test_opt(sb, NOLOAD) &&
> + EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
> + sb->s_op = &ext4_sops;
> + else
> + sb->s_op = &ext4_nojournal_sops;
> sb->s_export_op = &ext4_export_ops;
> sb->s_xattr = ext4_xattr_handlers;
> #ifdef CONFIG_QUOTA
> @@ -4134,13 +4123,21 @@ no_journal:
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
> "reserved pool", ext4_calculate_resv_clusters(sb));
> - goto failed_mount5;
> + goto failed_mount4a;
> }
>
> err = ext4_setup_system_zone(sb);
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to initialize system "
> "zone (%d)", err);
> + goto failed_mount4a;
> + }
> +
> + ext4_ext_init(sb);
> + err = ext4_mb_init(sb);
> + if (err) {
> + ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
> + err);
> goto failed_mount5;
> }
>
> @@ -4217,8 +4214,11 @@ failed_mount8:
> failed_mount7:
> ext4_unregister_li_request(sb);
> failed_mount6:
> - ext4_release_system_zone(sb);
> + ext4_mb_release(sb);
> failed_mount5:
> + ext4_ext_release(sb);
> + ext4_release_system_zone(sb);
> +failed_mount4a:
> dput(sb->s_root);
> sb->s_root = NULL;
> failed_mount4:
> @@ -4242,14 +4242,11 @@ failed_mount3:
> percpu_counter_destroy(&sbi->s_extent_cache_cnt);
> if (sbi->s_mmp_tsk)
> kthread_stop(sbi->s_mmp_tsk);
> -failed_mount2a:
> - ext4_mb_release(sb);
> failed_mount2:
> for (i = 0; i < db_count; i++)
> brelse(sbi->s_group_desc[i]);
> ext4_kvfree(sbi->s_group_desc);
> failed_mount:
> - ext4_ext_release(sb);
> if (sbi->s_chksum_driver)
> crypto_free_shash(sbi->s_chksum_driver);
> if (sbi->s_proc) {
>
--
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
