On Sat, 12 Jul 2014, Theodore Ts'o wrote:
> Date: Sat, 12 Jul 2014 12:10:15 -0400
> From: Theodore Ts'o <tytso@xxxxxxx>
> To: Ext4 Developers List <linux-ext4@xxxxxxxxxxxxxxx>
> Cc: Theodore Ts'o <tytso@xxxxxxx>
> Subject: [PATCH -v2] ext4: rearrange initialization to fix EXT4FS_DEBUG
>
> The EXT4FS_DEBUG is a *very* developer specific #ifdef designed for
> ext4 developers only. (You have to modify fs/ext4/ext4.h to enable
> it.)
>
> Rearrange how we initialize data structures to avoid calling
> ext4_count_free_clusters() until the multiblock allocator has been
> initialized.
>
> This also allows us to only call ext4_count_free_clusters() once, and
> simplifies the code somewhat.
>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> ---
>
> [ This will teach me not to try to push out a patch in a hurry before
> running to catch an airplane... and again, I don't plan to include
> this until the next merge window. -- Ted ]
>
> fs/ext4/super.c | 87 +++++++++++++++++++++++++--------------------------------
> 1 file changed, 38 insertions(+), 49 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6df7bc6..30767cd 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2142,10 +2142,6 @@ static int ext4_check_descriptors(struct super_block *sb,
> }
> if (NULL != first_not_zeroed)
> *first_not_zeroed = grp;
> -
> - ext4_free_blocks_count_set(sbi->s_es,
> - EXT4_C2B(sbi, ext4_count_free_clusters(sb)));
> - sbi->s_es->s_free_inodes_count =cpu_to_le32(ext4_count_free_inodes(sb));
> return 1;
> }
>
> @@ -3883,13 +3879,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> ext4_msg(sb, KERN_ERR, "group descriptors corrupted!");
> 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_mount2;
> - }
>
> sbi->s_gdb_count = db_count;
> get_random_bytes(&sbi->s_next_generation, sizeof(u32));
> @@ -3902,23 +3891,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> /* Register extent status tree shrinker */
> ext4_es_register_shrinker(sbi);
>
> - err = percpu_counter_init(&sbi->s_freeclusters_counter,
> - ext4_count_free_clusters(sb));
> - if (!err) {
> - err = percpu_counter_init(&sbi->s_freeinodes_counter,
> - ext4_count_free_inodes(sb));
> - }
> - if (!err) {
> - err = percpu_counter_init(&sbi->s_dirs_counter,
> - ext4_count_dirs(sb));
> - }
> - if (!err) {
> - err = percpu_counter_init(&sbi->s_dirtyclusters_counter, 0);
> - }
> - if (!err) {
> - err = percpu_counter_init(&sbi->s_extent_cache_cnt, 0);
> - }
> - if (err) {
> + if ((err = percpu_counter_init(&sbi->s_extent_cache_cnt, 0)) != 0) {
> ext4_msg(sb, KERN_ERR, "insufficient memory");
> goto failed_mount3;
> }
> @@ -4022,18 +3995,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>
> sbi->s_journal->j_commit_callback = ext4_journal_commit_callback;
>
> - /*
> - * The journal may have updated the bg summary counts, so we
> - * need to update the global counters.
> - */
> - percpu_counter_set(&sbi->s_freeclusters_counter,
> - ext4_count_free_clusters(sb));
> - percpu_counter_set(&sbi->s_freeinodes_counter,
> - ext4_count_free_inodes(sb));
> - percpu_counter_set(&sbi->s_dirs_counter,
> - ext4_count_dirs(sb));
> - percpu_counter_set(&sbi->s_dirtyclusters_counter, 0);
> -
> no_journal:
> if (ext4_mballoc_ready) {
> sbi->s_mb_cache = ext4_xattr_create_cache(sb->s_id);
> @@ -4141,6 +4102,32 @@ no_journal:
> goto failed_mount5;
> }
>
> + block = ext4_count_free_clusters(sb);
> + ext4_free_blocks_count_set(sbi->s_es, block);
This does not seem right. block is in cluster units so we need to
convert.
ext4_free_blocks_count_set(sbi->s_es, EXT4_C2B(sbi, blocks));
Otherwise it looks good to me.
Thanks!
-Lukas
> + err = percpu_counter_init(&sbi->s_freeclusters_counter, block);
> + if (!err) {
> + unsigned long freei = ext4_count_free_inodes(sb);
> + sbi->s_es->s_free_inodes_count = cpu_to_le32(freei);
> + err = percpu_counter_init(&sbi->s_freeinodes_counter, freei);
> + }
> + if (!err)
> + err = percpu_counter_init(&sbi->s_dirs_counter,
> + ext4_count_dirs(sb));
> + if (!err)
> + err = percpu_counter_init(&sbi->s_dirtyclusters_counter, 0);
> + if (err) {
> + ext4_msg(sb, KERN_ERR, "insufficient memory");
> + goto failed_mount6;
> + }
> +
> + 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_mount6;
> + }
> +
> err = ext4_register_li_request(sb, first_not_zeroed);
> if (err)
> goto failed_mount6;
> @@ -4215,6 +4202,12 @@ failed_mount7:
> ext4_unregister_li_request(sb);
> failed_mount6:
> ext4_mb_release(sb);
> + if (sbi->s_flex_groups)
> + ext4_kvfree(sbi->s_flex_groups);
> + percpu_counter_destroy(&sbi->s_freeclusters_counter);
> + percpu_counter_destroy(&sbi->s_freeinodes_counter);
> + percpu_counter_destroy(&sbi->s_dirs_counter);
> + percpu_counter_destroy(&sbi->s_dirtyclusters_counter);
> failed_mount5:
> ext4_ext_release(sb);
> ext4_release_system_zone(sb);
> @@ -4233,12 +4226,6 @@ failed_mount_wq:
> failed_mount3:
> ext4_es_unregister_shrinker(sbi);
> del_timer_sync(&sbi->s_err_report);
> - if (sbi->s_flex_groups)
> - ext4_kvfree(sbi->s_flex_groups);
> - percpu_counter_destroy(&sbi->s_freeclusters_counter);
> - percpu_counter_destroy(&sbi->s_freeinodes_counter);
> - percpu_counter_destroy(&sbi->s_dirs_counter);
> - percpu_counter_destroy(&sbi->s_dirtyclusters_counter);
> percpu_counter_destroy(&sbi->s_extent_cache_cnt);
> if (sbi->s_mmp_tsk)
> kthread_stop(sbi->s_mmp_tsk);
> @@ -4556,11 +4543,13 @@ static int ext4_commit_super(struct super_block *sb, int sync)
> else
> es->s_kbytes_written =
> cpu_to_le64(EXT4_SB(sb)->s_kbytes_written);
> - ext4_free_blocks_count_set(es,
> + if (EXT4_SB(sb)->s_freeclusters_counter.counters)
> + ext4_free_blocks_count_set(es,
> EXT4_C2B(EXT4_SB(sb), percpu_counter_sum_positive(
> &EXT4_SB(sb)->s_freeclusters_counter)));
> - es->s_free_inodes_count =
> - cpu_to_le32(percpu_counter_sum_positive(
> + if (EXT4_SB(sb)->s_freeinodes_counter.counters)
> + es->s_free_inodes_count =
> + cpu_to_le32(percpu_counter_sum_positive(
> &EXT4_SB(sb)->s_freeinodes_counter));
> BUFFER_TRACE(sbh, "marking dirty");
> ext4_superblock_csum_set(sb);
>
--
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
