Re: [PATCH] ext4: Fix journal_ioprio mount option handling

[Ojaswin Mujoo]

Hello,

Please let me know if there are any suggestions or reviews on this patch.

Thank you in advance,
Ojaswin

On Mon, Apr 18, 2022 at 02:05:45PM +0530, Ojaswin Mujoo wrote:
> In __ext4_super() we always overwrote the user specified journal_ioprio
> value with a default value, expecting  parse_apply_sb_mount_options() to
> later correctly set ctx->journal_ioprio to the user specified value.
> However, if parse_apply_sb_mount_options() returned early because of
> empty sbi->es_s->s_mount_opts, the correct journal_ioprio value was
> never set.
> 
> This patch fixes __ext4_super() to only use the default value if the
> user has not specified any value for journal_ioprio.
> 
> Similarly, the remount behavior was to either use journal_ioprio
> value specified during initial mount, or use the default value
> irrespective of the journal_ioprio value specified during remount.
> This patch modifies this to first check if a new value for ioprio
> has been passed during remount and apply it. Incase, no new value is
> passed, use the value specified during initial mount.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> ---
>  fs/ext4/super.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c5a9ffbf7f4f..bfd767c51203 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4427,7 +4427,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  	int silent = fc->sb_flags & SB_SILENT;
>  
>  	/* Set defaults for the variables that will be set during parsing */
> -	ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
> +	if (!(ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO))
> +		ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
>  
>  	sbi->s_inode_readahead_blks = EXT4_DEF_INODE_READAHEAD_BLKS;
>  	sbi->s_sectors_written_start =
> @@ -6289,7 +6290,6 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
>  	char *to_free[EXT4_MAXQUOTAS];
>  #endif
>  
> -	ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
>  
>  	/* Store the original options */
>  	old_sb_flags = sb->s_flags;
> @@ -6315,9 +6315,14 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
>  		} else
>  			old_opts.s_qf_names[i] = NULL;
>  #endif
> -	if (sbi->s_journal && sbi->s_journal->j_task->io_context)
> -		ctx->journal_ioprio =
> -			sbi->s_journal->j_task->io_context->ioprio;
> +	if (!(ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO)) {
> +		if (sbi->s_journal && sbi->s_journal->j_task->io_context)
> +			ctx->journal_ioprio =
> +				sbi->s_journal->j_task->io_context->ioprio;
> +		else
> +			ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
> +
> +	}
>  
>  	ext4_apply_options(fc, sb);
>  
> -- 
> 2.27.0
>