| Message ID | 20210401172129.189766-8-harshadshirwadkar@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | Block Allocator Improvements | expand |
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 >
Thanks for taking a look! On Thu, Apr 1, 2021 at 10:16 PM Andreas Dilger <adilger@dilger.ca> wrote: > > 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. 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 <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 > >
On Apr 2, 2021, at 10:46, harshad shirwadkar <harshadshirwadkar@gmail.com> wrote: > > Thanks for taking a look! > >> On Thu, Apr 1, 2021 at 10:16 PM Andreas Dilger <adilger@dilger.ca> wrote: >> >>> 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. > 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 <adilger@dilger.ca> 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 >>>
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;
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 <harshadshirwadkar@gmail.com> --- fs/ext4/ext4.h | 2 +- fs/ext4/super.c | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-)