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.
Signed-off-by: Harshad Shirwadkar <[email protected]>
---
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
On Apr 1, 2021, at 11:45, Harshad Shirwadkar <[email protected]> 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 <[email protected]>
> ---
> 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
>
Thanks for taking a look!
On Thu, Apr 1, 2021 at 10:16 PM Andreas Dilger <[email protected]> wrote:
>
> On Apr 1, 2021, at 11:45, Harshad Shirwadkar <[email protected]> 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.
How so? This patch doesn't get rid of the prefetch_block_bitmaps mount
option, it just marks it as "removed". So, you can still pass
"prefetch_block_bitmaps" mount option and get the prefetching
behavior, the only difference is that you'll get an additional kernel
message saying that "ignoring removed mount option
prefetch_block_bitmaps", which I thought is good since looking at that
message, users will eventually remove that mount option from their
mount arguments.
Thanks,
Harshad
>
> Cheers, Andreas
>
> > Signed-off-by: Harshad Shirwadkar <[email protected]>
> > ---
> > 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
> >
On Apr 2, 2021, at 10:46, harshad shirwadkar <[email protected]> wrote:
>
> Thanks for taking a look!
>
>> On Thu, Apr 1, 2021 at 10:16 PM Andreas Dilger <[email protected]> wrote:
>>
>>> On Apr 1, 2021, at 11:45, Harshad Shirwadkar <[email protected]> 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.
> How so? This patch doesn't get rid of the prefetch_block_bitmaps mount
> option, it just marks it as "removed". So, you can still pass
> "prefetch_block_bitmaps" mount option and get the prefetching
> behavior, the only difference is that you'll get an additional kernel
> message saying that "ignoring removed mount option
> prefetch_block_bitmaps", which I thought is good since looking at that
> message, users will eventually remove that mount option from their
> mount arguments.
Sorry, I guess I was reading too quickly, and wasn't familiar with the
Opt_removed mechanism. It seems OK as-is, you can add:
Reviewed-by: Andreas Dilger <[email protected]>
Cheers, Andreas
>>> Signed-off-by: Harshad Shirwadkar <[email protected]>
>>> ---
>>> 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
>>>