Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Mon, 9 May 2022 11:57:11 +0530
From:   Ojaswin Mujoo <ojaswin@linux.ibm.com>
To:     linux-ext4@vger.kernel.org
Cc:     "Theodore Ts'o" <tytso@mit.edu>,
        Ritesh Harjani <riteshh@linux.ibm.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ext4: Fix journal_ioprio mount option handling
Message-ID: <Yni0P6wS3r32XAZN@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com>
References: <20220418083545.45778-1-ojaswin@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220418083545.45778-1-ojaswin@linux.ibm.com>
Precedence: bulk

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@linux.ibm.com>
> ---
>  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
>