Re: [PATCH v2 19/34] ocfs2: keep the current mount options on remount

[Joseph Qi <joseph.qi@linux.alibaba.com>]

On 10/30/26 12:07 AM, Christian Brauner wrote:
> ocfs2_init_fs_context() doesn't inherit the current mount options for a
> reconfigure. But ocfs2_check_set_options() insists on exactly one
> heartbeat mode and ocfs2_reconfigure() refuses to change the data mode.
> So every remount without heartbeat= fails with EINVAL, including
> emergency remount (sysrq u) which passes no options at all, and that
> WARNs for every ocfs2 mounted read-write. A remount that does pass
> heartbeat= silently resets every option it doesn't repeat, e.g. noacl,
> errors=, barrier= and preferred_slot=.
> 
> So start a reconfigure from the current options like exfat and gfs2 do.
> Asking for a different data or heartbeat mode still fails with EINVAL.
> 
> The next patch stops ocfs2 from setting SB_RDONLY after errors. An
> errored ocfs2 then stays read-write and emergency remount would run into
> this as well.
> 
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/ocfs2/super.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 0af2245ae303..67283244426e 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -1222,6 +1222,19 @@ static int ocfs2_init_fs_context(struct fs_context *fc)
>  	mopt->resv_level = OCFS2_DEFAULT_RESV_LEVEL;
>  	mopt->dir_resv_level = -1;
>  
> +	/*
> +	 * Start from the current options so a remount only changes what it
> +	 * specifies, emergency remount passes none at all.
> +	 */
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> +		struct ocfs2_super *osb = OCFS2_SB(fc->root->d_sb);
> +
> +		mopt->mount_opt = osb->s_mount_opt;
> +		mopt->atime_quantum = osb->s_atime_quantum;
> +		mopt->slot = osb->preferred_slot;
> +		mopt->user_stack = ocfs2_userspace_stack(osb);
> +	}

ocfs2_reconfigure() decides the transition from fc->sb_flags alone without
sb_flags_mask test, unlike reconfigure_super(). And fspick() passes
fs_context_for_reconfigure(target.dentry, 0, 0). So on a read-only mount:

  fspick(AT_FDCWD, "/mnt", 0)        sb_flags 0, sb_flags_mask 0
  fsconfig(fd, FSCONFIG_SET_STRING, "atime_quantum", "30", 0)
  fsconfig(fd, FSCONFIG_CMD_RECONFIGURE)
    reconfigure_super()
      ocfs2_reconfigure()
        (bool)(0 & SB_RDONLY) != sb_rdonly(sb)  -> 0 != 1, enters
        fc->sb_flags & SB_RDONLY                -> 0, takes the rw branch
        sb->s_flags &= ~SB_RDONLY
        osb->osb_flags &= ~OCFS2_OSB_SOFT_RO
        ocfs2_enable_quotas(osb)                -> writes the quota files

The final WRITE_ONCE(sb->s_flags, ...) in reconfigure_super() then
leaves it cleared, because SB_RDONLY is not in sb_flags_mask. The same
falls out of a legacy remount whose flags omit MS_RDONLY.

So it seems we should test sb_flags_mask as well?

Thanks,
Joseph

> +
>  	fc->fs_private = mopt;
>  	fc->ops = &ocfs2_context_ops;
>  
>