| Message ID | c98970fe99f26718586d02e942f293300fb48ef3.1646732698.git.ojaswin@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] ext4: Make mb_optimize_scan option work with set/unset mount cmd | expand |
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 <riteshh@linux.ibm.com> > > Reported-by: Ritesh Harjani <riteshh@linux.ibm.com> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > --- > 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 >
On Tue, 8 Mar 2022 15:22:00 +0530, 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 > > [...] Applied, thanks! [1/2] ext4: Make mb_optimize_scan option work with set/unset mount cmd commit: 27b38686a3bb601db48901dbc4e2fc5d77ffa2c1 [2/2] ext4: Make mb_optimize_scan performance mount option work with extents commit: 077d0c2c78df6f7260cdd015a991327efa44d8ad Best regards,
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) {
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 <riteshh@linux.ibm.com> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> --- fs/ext4/super.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)