Re: [PATCH v2 28/34] ubifs: don't set SB_RDONLY after errors

[Zhihao Cheng <chengzhihao1@huawei.com>]

在 2026/10/30 0:07, Christian Brauner 写道:
> ubifs_ro_mode() sets c->ro_error and SB_RDONLY from whatever context hit
> the error, e.g. a failing write, and without holding s_umount. UBIFS
> can't be frozen but that still races with everyone else changing
> sb->s_flags. And c->ro_error alone already makes every write fail with
> EROFS.
> 
> So only set SB_RDONLY while the filesystem is still being mounted and
> call sb_set_emergency_ro() after that. The xattr setters check
> c->ro_error as well because ecryptfs calls them without write access on
> the lower mount and they change the inode in memory first.
> 
> So far a remount to read-only after an error was refused and a remount
> to read-write went through and cleared SB_RDONLY again because
> c->ro_mount was never set. Now going read-only works and its writes fail
> with EROFS before any I/O, and going read-write is refused by the VFS.
> fsync() fails with EROFS instead of taking ubifs_fsync()'s read-only
> shortcut.
> 
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>   fs/ubifs/io.c    | 9 ++++++++-
>   fs/ubifs/super.c | 5 +----
>   fs/ubifs/xattr.c | 6 ++++++
>   3 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 6c6d68242779..7a59aa1ea5ce 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -72,7 +72,14 @@ void ubifs_ro_mode(struct ubifs_info *c, int err)
>   	if (!c->ro_error) {
>   		c->ro_error = 1;
>   		c->no_chk_data_crc = 0;
> -		c->vfs_sb->s_flags |= SB_RDONLY;
> +		/*
> +		 * Before SB_BORN the mounter holds s_umount, later see
> +		 * sb_set_emergency_ro().
> +		 */
> +		if (!(c->vfs_sb->s_flags & SB_BORN))
> +			c->vfs_sb->s_flags |= SB_RDONLY;
> +		else
> +			sb_set_emergency_ro(c->vfs_sb);
>   		ubifs_warn(c, "switched to read-only mode, error %d", err);
>   		dump_stack();
>   	}
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 38972786817e..ef39da4e780b 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1977,10 +1977,7 @@ static int ubifs_reconfigure(struct fs_context *fc)
>   		if (err)
>   			return err;
>   	} else if (!c->ro_mount && (fc->sb_flags & SB_RDONLY)) {
> -		if (c->ro_error) {
> -			ubifs_msg(c, "cannot re-mount R/O due to prior errors");
> -			return -EROFS;
> -		}
> +		/* After an error ubifs_remount_ro() can't write but still works. */

Hi, Brauner
If we remove the c->ro_error check, ubifs_remount_ro->ubifs_wbuf_sync 
will return EROFS and trigger ubifs_ro_mode() again, I think we can 
retain the pre-check placement to trigger more error printing.
BTW, a remount to read-write after an error is refused too.
>   		ubifs_remount_ro(c);
>   	}
>   
> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
> index 3b0e8a270f59..572f6f3e52ad 100644
> --- a/fs/ubifs/xattr.c
> +++ b/fs/ubifs/xattr.c
> @@ -260,6 +260,9 @@ int ubifs_xattr_set(struct inode *host, const char *name, const void *value,
>   	if (check_lock)
>   		ubifs_assert(c, inode_is_locked(host));
>   
> +	if (c->ro_error)
> +		return -EROFS;
> +
>   	if (size > UBIFS_MAX_INO_DATA)
>   		return -ERANGE;
>   
> @@ -564,6 +567,9 @@ static int ubifs_xattr_remove(struct inode *host, const char *name)
>   
>   	ubifs_assert(c, inode_is_locked(host));
>   
> +	if (c->ro_error)
> +		return -EROFS;
> +
>   	if (fname_len(&nm) > UBIFS_MAX_NLEN)
>   		return -ENAMETOOLONG;
>   
> 
The sb_rdonly won't affect the sync_fs_one_sb() after an error, and 
sb_emergency_ro check in not added into sync_fs_one_sb, should we add 
the c->ro_error check in ubifs_sync_fs() to prevent the failure 
ubifs_wbuf_sync executing?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/