Re: [PATCH v6 7/7] ext4: make prefetch_block_bitmaps default

From: Andreas Dilger <adilger@dilger.ca>
To: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu
Subject: Re: [PATCH v6 7/7] ext4: make prefetch_block_bitmaps default
Date: Thu, 1 Apr 2021 23:16:35 -0600	[thread overview]
Message-ID: <5815C46F-D210-4545-9610-136F68E93B66@dilger.ca> (raw)
In-Reply-To: <20210401172129.189766-8-harshadshirwadkar@gmail.com>

On Apr 1, 2021, at 11:45, Harshad Shirwadkar <harshadshirwadkar@gmail.com> wrote:
> 
> Block bitmap prefetching is needed for these allocator optimization
> data structures to get populated and provide better group scanning
> order. So, turn it on bu default. prefetch_block_bitmaps mount option
> is now marked as removed and a new option no_prefetch_block_bitmaps is
> added to disable block bitmap prefetching.

This makes it more difficult to change between an old kernel and a new one
using this option. It would be better to keep prefetch_block_bitmaps to turn
the option on (not harmful if it is already on), and no_* turn it off. 

Cheers, Andreas

> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> ---
> fs/ext4/ext4.h  |  2 +-
> fs/ext4/super.c | 15 ++++++++-------
> 2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 9a5afe9d2310..20c757f711e7 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1227,7 +1227,7 @@ struct ext4_inode_info {
> #define EXT4_MOUNT_JOURNAL_CHECKSUM    0x800000 /* Journal checksums */
> #define EXT4_MOUNT_JOURNAL_ASYNC_COMMIT    0x1000000 /* Journal Async Commit */
> #define EXT4_MOUNT_WARN_ON_ERROR    0x2000000 /* Trigger WARN_ON on error */
> -#define EXT4_MOUNT_PREFETCH_BLOCK_BITMAPS 0x4000000
> +#define EXT4_MOUNT_NO_PREFETCH_BLOCK_BITMAPS 0x4000000
> #define EXT4_MOUNT_DELALLOC        0x8000000 /* Delalloc support */
> #define EXT4_MOUNT_DATA_ERR_ABORT    0x10000000 /* Abort on file data write */
> #define EXT4_MOUNT_BLOCK_VALIDITY    0x20000000 /* Block validity checking */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6116640081c0..cec0fb07916b 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1687,7 +1687,7 @@ enum {
>    Opt_dioread_nolock, Opt_dioread_lock,
>    Opt_discard, Opt_nodiscard, Opt_init_itable, Opt_noinit_itable,
>    Opt_max_dir_size_kb, Opt_nojournal_checksum, Opt_nombcache,
> -    Opt_prefetch_block_bitmaps, Opt_mb_optimize_scan,
> +    Opt_no_prefetch_block_bitmaps, Opt_mb_optimize_scan,
> #ifdef CONFIG_EXT4_DEBUG
>    Opt_fc_debug_max_replay, Opt_fc_debug_force
> #endif
> @@ -1787,7 +1787,8 @@ static const match_table_t tokens = {
>    {Opt_inlinecrypt, "inlinecrypt"},
>    {Opt_nombcache, "nombcache"},
>    {Opt_nombcache, "no_mbcache"},    /* for backward compatibility */
> -    {Opt_prefetch_block_bitmaps, "prefetch_block_bitmaps"},
> +    {Opt_removed, "prefetch_block_bitmaps"},
> +    {Opt_no_prefetch_block_bitmaps, "no_prefetch_block_bitmaps"},
>    {Opt_mb_optimize_scan, "mb_optimize_scan=%d"},
>    {Opt_removed, "check=none"},    /* mount option from ext2/3 */
>    {Opt_removed, "nocheck"},    /* mount option from ext2/3 */
> @@ -2009,7 +2010,7 @@ static const struct mount_opts {
>    {Opt_max_dir_size_kb, 0, MOPT_GTE0},
>    {Opt_test_dummy_encryption, 0, MOPT_STRING},
>    {Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
> -    {Opt_prefetch_block_bitmaps, EXT4_MOUNT_PREFETCH_BLOCK_BITMAPS,
> +    {Opt_no_prefetch_block_bitmaps, EXT4_MOUNT_NO_PREFETCH_BLOCK_BITMAPS,
>     MOPT_SET},
>    {Opt_mb_optimize_scan, EXT4_MOUNT2_MB_OPTIMIZE_SCAN, MOPT_GTE0},
> #ifdef CONFIG_EXT4_DEBUG
> @@ -3706,11 +3707,11 @@ static struct ext4_li_request *ext4_li_request_new(struct super_block *sb,
> 
>    elr->lr_super = sb;
>    elr->lr_first_not_zeroed = start;
> -    if (test_opt(sb, PREFETCH_BLOCK_BITMAPS))
> -        elr->lr_mode = EXT4_LI_MODE_PREFETCH_BBITMAP;
> -    else {
> +    if (test_opt(sb, NO_PREFETCH_BLOCK_BITMAPS)) {
>        elr->lr_mode = EXT4_LI_MODE_ITABLE;
>        elr->lr_next_group = start;
> +    } else {
> +        elr->lr_mode = EXT4_LI_MODE_PREFETCH_BBITMAP;
>    }
> 
>    /*
> @@ -3741,7 +3742,7 @@ int ext4_register_li_request(struct super_block *sb,
>        goto out;
>    }
> 
> -    if (!test_opt(sb, PREFETCH_BLOCK_BITMAPS) &&
> +    if (test_opt(sb, NO_PREFETCH_BLOCK_BITMAPS) &&
>        (first_not_zeroed == ngroups || sb_rdonly(sb) ||
>         !test_opt(sb, INIT_INODE_TABLE)))
>        goto out;
> -- 
> 2.31.0.291.g576ba9dcdaf-goog
> 