All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Zhang Yi <yi.zhang@huaweicloud.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu,
	adilger.kernel@dilger.ca, jack@suse.cz,
	harshadshirwadkar@gmail.com, yi.zhang@huawei.com,
	yukuai3@huawei.com
Subject: Re: [PATCH] ext4: fix incorrect options show of original mount_opt and extend mount_opt2
Date: Mon, 30 Jan 2023 12:11:38 +0100	[thread overview]
Message-ID: <20230130111138.76tp6pij3yhh4brh@quack3> (raw)
In-Reply-To: <20230129034939.3702550-1-yi.zhang@huaweicloud.com>

On Sun 29-01-23 11:49:39, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Current _ext4_show_options() do not distinguish MOPT_2 flag, so it mixed
> extend sbi->s_mount_opt2 options with sbi->s_mount_opt, it could lead to
> show incorrect options, e.g. show fc_debug_force if we mount with
> errors=continue mode and miss it if we set.
> 
>   $ mkfs.ext4 /dev/pmem0
>   $ mount -o errors=remount-ro /dev/pmem0 /mnt
>   $ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
>     #empty
>   $ mount -o remount,errors=continue /mnt
>   $ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
>     fc_debug_force
>   $ mount -o remount,errors=remount-ro,fc_debug_force /mnt
>   $ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
>     #empty
> 
> Fixes: 995a3ed67fc8 ("ext4: add fast_commit feature and handling for extended mount options")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Good catch! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/ext4.h  |  1 +
>  fs/ext4/super.c | 28 +++++++++++++++++++++-------
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 140e1eb300d1..6479146140d2 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1529,6 +1529,7 @@ struct ext4_sb_info {
>  	unsigned int s_mount_opt2;
>  	unsigned long s_mount_flags;
>  	unsigned int s_def_mount_opt;
> +	unsigned int s_def_mount_opt2;
>  	ext4_fsblk_t s_sb_block;
>  	atomic64_t s_resv_clusters;
>  	kuid_t s_resuid;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 260c1b3e3ef2..e6fe416a65a3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2894,7 +2894,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	struct ext4_super_block *es = sbi->s_es;
> -	int def_errors, def_mount_opt = sbi->s_def_mount_opt;
> +	int def_errors;
>  	const struct mount_opts *m;
>  	char sep = nodefs ? '\n' : ',';
>  
> @@ -2906,15 +2906,28 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
>  
>  	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
>  		int want_set = m->flags & MOPT_SET;
> +		int opt_2 = m->flags & MOPT_2;
> +		unsigned int mount_opt, def_mount_opt;
> +
>  		if (((m->flags & (MOPT_SET|MOPT_CLEAR)) == 0) ||
>  		    m->flags & MOPT_SKIP)
>  			continue;
> -		if (!nodefs && !(m->mount_opt & (sbi->s_mount_opt ^ def_mount_opt)))
> -			continue; /* skip if same as the default */
> +
> +		if (opt_2) {
> +			mount_opt = sbi->s_mount_opt2;
> +			def_mount_opt = sbi->s_def_mount_opt2;
> +		} else {
> +			mount_opt = sbi->s_mount_opt;
> +			def_mount_opt = sbi->s_def_mount_opt;
> +		}
> +		/* skip if same as the default */
> +		if (!nodefs && !(m->mount_opt & (mount_opt ^ def_mount_opt)))
> +			continue;
> +		/* select Opt_noFoo vs Opt_Foo */
>  		if ((want_set &&
> -		     (sbi->s_mount_opt & m->mount_opt) != m->mount_opt) ||
> -		    (!want_set && (sbi->s_mount_opt & m->mount_opt)))
> -			continue; /* select Opt_noFoo vs Opt_Foo */
> +		     (mount_opt & m->mount_opt) != m->mount_opt) ||
> +		    (!want_set && (mount_opt & m->mount_opt)))
> +			continue;
>  		SEQ_OPTS_PRINT("%s", token2str(m->token));
>  	}
>  
> @@ -2942,7 +2955,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
>  	if (nodefs || sbi->s_stripe)
>  		SEQ_OPTS_PRINT("stripe=%lu", sbi->s_stripe);
>  	if (nodefs || EXT4_MOUNT_DATA_FLAGS &
> -			(sbi->s_mount_opt ^ def_mount_opt)) {
> +			(sbi->s_mount_opt ^ sbi->s_def_mount_opt)) {
>  		if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)
>  			SEQ_OPTS_PUTS("data=journal");
>  		else if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_ORDERED_DATA)
> @@ -5086,6 +5099,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  		goto failed_mount;
>  
>  	sbi->s_def_mount_opt = sbi->s_mount_opt;
> +	sbi->s_def_mount_opt2 = sbi->s_mount_opt2;
>  
>  	err = ext4_check_opt_consistency(fc, sb);
>  	if (err < 0)
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2023-01-30 11:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-29  3:49 [PATCH] ext4: fix incorrect options show of original mount_opt and extend mount_opt2 Zhang Yi
2023-01-30 11:11 ` Jan Kara [this message]
2023-02-19  5:40 ` Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230130111138.76tp6pij3yhh4brh@quack3 \
    --to=jack@suse.cz \
    --cc=adilger.kernel@dilger.ca \
    --cc=harshadshirwadkar@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=yi.zhang@huawei.com \
    --cc=yi.zhang@huaweicloud.com \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.