After moving to the new mount API, mb_optimize_scan mount option
handling was not working as expected due to the parsed value always
being overwritten by default. Refactor and fix this to the expected
behavior described below:
* mb_optimize_scan=1 - On
* mb_optimize_scan=0 - Off
* mb_optimize_scan not passed - On if no. of BGs > threshold else off
* Remounts retain previous value unless we explicitly pass the option
with a new value
Reported-by: Ritesh Harjani <[email protected]>
Signed-off-by: Ojaswin Mujoo <[email protected]>
---
fs/ext4/super.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c5021ca0a28a..cd0547fabd79 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2021,12 +2021,12 @@ static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
#define EXT4_SPEC_s_commit_interval (1 << 16)
#define EXT4_SPEC_s_fc_debug_max_replay (1 << 17)
#define EXT4_SPEC_s_sb_block (1 << 18)
+#define EXT4_SPEC_mb_optimize_scan (1 << 19)
struct ext4_fs_context {
char *s_qf_names[EXT4_MAXQUOTAS];
char *test_dummy_enc_arg;
int s_jquota_fmt; /* Format of quota to use */
- int mb_optimize_scan;
#ifdef CONFIG_EXT4_DEBUG
int s_fc_debug_max_replay;
#endif
@@ -2451,12 +2451,17 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
ctx_clear_mount_opt(ctx, m->mount_opt);
return 0;
case Opt_mb_optimize_scan:
- if (result.int_32 != 0 && result.int_32 != 1) {
+ if (result.int_32 == 1) {
+ ctx_set_mount_opt2(ctx, EXT4_MOUNT2_MB_OPTIMIZE_SCAN);
+ ctx->spec |= EXT4_SPEC_mb_optimize_scan;
+ } else if (result.int_32 == 0) {
+ ctx_clear_mount_opt2(ctx, EXT4_MOUNT2_MB_OPTIMIZE_SCAN);
+ ctx->spec |= EXT4_SPEC_mb_optimize_scan;
+ } else {
ext4_msg(NULL, KERN_WARNING,
"mb_optimize_scan should be set to 0 or 1.");
return -EINVAL;
}
- ctx->mb_optimize_scan = result.int_32;
return 0;
}
@@ -4369,7 +4374,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
/* Set defaults for the variables that will be set during parsing */
ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
- ctx->mb_optimize_scan = DEFAULT_MB_OPTIMIZE_SCAN;
sbi->s_inode_readahead_blks = EXT4_DEF_INODE_READAHEAD_BLKS;
sbi->s_sectors_written_start =
@@ -5320,12 +5324,12 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
* turned off by passing "mb_optimize_scan=0". This can also be
* turned on forcefully by passing "mb_optimize_scan=1".
*/
- if (ctx->mb_optimize_scan == 1)
- set_opt2(sb, MB_OPTIMIZE_SCAN);
- else if (ctx->mb_optimize_scan == 0)
- clear_opt2(sb, MB_OPTIMIZE_SCAN);
- else if (sbi->s_groups_count >= MB_DEFAULT_LINEAR_SCAN_THRESHOLD)
- set_opt2(sb, MB_OPTIMIZE_SCAN);
+ if (!(ctx->spec & EXT4_SPEC_mb_optimize_scan)) {
+ if (sbi->s_groups_count >= MB_DEFAULT_LINEAR_SCAN_THRESHOLD)
+ set_opt2(sb, MB_OPTIMIZE_SCAN);
+ else
+ clear_opt2(sb, MB_OPTIMIZE_SCAN);
+ }
err = ext4_mb_init(sb);
if (err) {
--
2.27.0
Currently mb_optimize_scan scan feature which improves filesystem
performance heavily (when FS is fragmented), seems to be not working
with files with extents (ext4 by default has files with extents).
This patch fixes that and makes mb_optimize_scan feature work
for files with extents.
Below are some performance numbers obtained when allocating a 10M and 100M
file with and w/o this patch on a filesytem with no 1M contiguous block.
<perf numbers>
===============
Workload: dd if=/dev/urandom of=test conv=fsync bs=1M count=10/100
Time taken
=====================================================
no. Size without-patch with-patch Diff(%)
1 10M 0m8.401s 0m5.623s 33.06%
2 100M 1m40.465s 1m14.737s 25.6%
<debug stats>
=============
w/o patch:
mballoc:
reqs: 17056
success: 11407
groups_scanned: 13643
cr0_stats:
hits: 37
groups_considered: 9472
useless_loops: 36
bad_suggestions: 0
cr1_stats:
hits: 11418
groups_considered: 908560
useless_loops: 1894
bad_suggestions: 0
cr2_stats:
hits: 1873
groups_considered: 6913
useless_loops: 21
cr3_stats:
hits: 21
groups_considered: 5040
useless_loops: 21
extents_scanned: 417364
goal_hits: 3707
2^n_hits: 37
breaks: 1873
lost: 0
buddies_generated: 239/240
buddies_time_used: 651080
preallocated: 705
discarded: 478
with patch:
mballoc:
reqs: 12768
success: 11305
groups_scanned: 12768
cr0_stats:
hits: 1
groups_considered: 18
useless_loops: 0
bad_suggestions: 0
cr1_stats:
hits: 5829
groups_considered: 50626
useless_loops: 0
bad_suggestions: 0
cr2_stats:
hits: 6938
groups_considered: 580363
useless_loops: 0
cr3_stats:
hits: 0
groups_considered: 0
useless_loops: 0
extents_scanned: 309059
goal_hits: 0
2^n_hits: 1
breaks: 1463
lost: 0
buddies_generated: 239/240
buddies_time_used: 791392
preallocated: 673
discarded: 446
Fixes: 196e402 (ext4: improve cr 0 / cr 1 group scanning)
Reported-by: Geetika Moolchandani <[email protected]>
Reported-by: Nageswara R Sastry <[email protected]>
Suggested-by: Ritesh Harjani <[email protected]>
Signed-off-by: Ojaswin Mujoo <[email protected]>
---
fs/ext4/mballoc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 67ac95c4cd9b..f9be6ab482a5 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1000,7 +1000,7 @@ static inline int should_optimize_scan(struct ext4_allocation_context *ac)
return 0;
if (ac->ac_criteria >= 2)
return 0;
- if (ext4_test_inode_flag(ac->ac_inode, EXT4_INODE_EXTENTS))
+ if (!ext4_test_inode_flag(ac->ac_inode, EXT4_INODE_EXTENTS))
return 0;
return 1;
}
--
2.27.0
cc' Lukas too
On 22/03/08 03:22PM, Ojaswin Mujoo wrote:
> After moving to the new mount API, mb_optimize_scan mount option
> handling was not working as expected due to the parsed value always
> being overwritten by default. Refactor and fix this to the expected
> behavior described below:
>
> * mb_optimize_scan=1 - On
> * mb_optimize_scan=0 - Off
> * mb_optimize_scan not passed - On if no. of BGs > threshold else off
> * Remounts retain previous value unless we explicitly pass the option
> with a new value
So with new mount API, once we call ctx_set/clear_mount_opt2 with
EXT4_MOUNT2_MB_OPTIMIZE_SCAN, ext4_apply_options() will take care of
setting/clearing it in sbi->s_mount_**
Then with that small nit mentioned below, the patch looks good to me.
Feel free to add after addressing it.
Reviewed-by: Ritesh Harjani <[email protected]>
>
> Reported-by: Ritesh Harjani <[email protected]>
> Signed-off-by: Ojaswin Mujoo <[email protected]>
> ---
> fs/ext4/super.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c5021ca0a28a..cd0547fabd79 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2021,12 +2021,12 @@ static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
> #define EXT4_SPEC_s_commit_interval (1 << 16)
> #define EXT4_SPEC_s_fc_debug_max_replay (1 << 17)
> #define EXT4_SPEC_s_sb_block (1 << 18)
> +#define EXT4_SPEC_mb_optimize_scan (1 << 19)
>
> struct ext4_fs_context {
> char *s_qf_names[EXT4_MAXQUOTAS];
> char *test_dummy_enc_arg;
> int s_jquota_fmt; /* Format of quota to use */
> - int mb_optimize_scan;
> #ifdef CONFIG_EXT4_DEBUG
> int s_fc_debug_max_replay;
> #endif
> @@ -2451,12 +2451,17 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
> ctx_clear_mount_opt(ctx, m->mount_opt);
> return 0;
> case Opt_mb_optimize_scan:
> - if (result.int_32 != 0 && result.int_32 != 1) {
> + if (result.int_32 == 1) {
> + ctx_set_mount_opt2(ctx, EXT4_MOUNT2_MB_OPTIMIZE_SCAN);
> + ctx->spec |= EXT4_SPEC_mb_optimize_scan;
> + } else if (result.int_32 == 0) {
> + ctx_clear_mount_opt2(ctx, EXT4_MOUNT2_MB_OPTIMIZE_SCAN);
> + ctx->spec |= EXT4_SPEC_mb_optimize_scan;
> + } else {
> ext4_msg(NULL, KERN_WARNING,
> "mb_optimize_scan should be set to 0 or 1.");
> return -EINVAL;
> }
> - ctx->mb_optimize_scan = result.int_32;
> return 0;
> }
>
> @@ -4369,7 +4374,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>
> /* Set defaults for the variables that will be set during parsing */
> ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
> - ctx->mb_optimize_scan = DEFAULT_MB_OPTIMIZE_SCAN;
So if we are not using this DEFAULT_MB_OPTIMIZE_SCAN macro anywhere else, then
we should just kill it's definition too in the same patch.
>
> sbi->s_inode_readahead_blks = EXT4_DEF_INODE_READAHEAD_BLKS;
> sbi->s_sectors_written_start =
> @@ -5320,12 +5324,12 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> * turned off by passing "mb_optimize_scan=0". This can also be
> * turned on forcefully by passing "mb_optimize_scan=1".
> */
> - if (ctx->mb_optimize_scan == 1)
> - set_opt2(sb, MB_OPTIMIZE_SCAN);
> - else if (ctx->mb_optimize_scan == 0)
> - clear_opt2(sb, MB_OPTIMIZE_SCAN);
> - else if (sbi->s_groups_count >= MB_DEFAULT_LINEAR_SCAN_THRESHOLD)
> - set_opt2(sb, MB_OPTIMIZE_SCAN);
> + if (!(ctx->spec & EXT4_SPEC_mb_optimize_scan)) {
> + if (sbi->s_groups_count >= MB_DEFAULT_LINEAR_SCAN_THRESHOLD)
> + set_opt2(sb, MB_OPTIMIZE_SCAN);
> + else
> + clear_opt2(sb, MB_OPTIMIZE_SCAN);
> + }
>
> err = ext4_mb_init(sb);
> if (err) {
> --
> 2.27.0
>