Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: linux-fsdevel@vger.kernel.org, Jan Kara <jack@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	ecryptfs@vger.kernel.org,  linux-ext4@vger.kernel.org,
	jfs-discussion@lists.sourceforge.net,
	 linux-nilfs@vger.kernel.org, exfat@lists.linux.dev,
	linux-mm@kvack.org,  ntfs@lists.linux.dev,
	ocfs2-devel@lists.linux.dev,  linux-btrfs@vger.kernel.org,
	gfs2@lists.linux.dev,  linux-efi@vger.kernel.org,
	linux-mtd@lists.infradead.org,  linux-xfs@vger.kernel.org,
	 "Christian Brauner (Amutable)" <brauner@kernel.org>
Subject: [PATCH v2 20/34] ocfs2: don't set SB_RDONLY after filesystem errors
Date: Thu, 29 Oct 2026 17:07:11 +0100	[thread overview]
Message-ID: <20261029-work-fs-sb_rdonly-v2-20-2f44f64f0b66@kernel.org> (raw)
In-Reply-To: <20261029-work-fs-sb_rdonly-v2-0-2f44f64f0b66@kernel.org>

With errors=remount-ro, the default, ocfs2_handle_error() sets SB_RDONLY
without holding s_umount. If that happens while the filesystem is frozen
through bdev_freeze() thaw_super() leaves the s_writers rwsems locked.
Writers hang and so does umount because ocfs2_shutdown_local_alloc()
needs sb_start_intwrite().

The soft read-only state doesn't stop modifications on its own. Every
read-only mount sets it and those still need transactions for recovery,
the local alloc and the truncate log. Everything else was stopped by the
VFS because of SB_RDONLY. So call sb_set_emergency_ro() instead which
keeps it that way, and keep setting SOFT_RO for the existing
ocfs2_emergency_state() checks.

The xattr and ACL setters get a check of their own because ecryptfs
calls them without write access on the lower mount. It sits in the xattr
handlers and not in ocfs2_xattr_set() because chmod and setfacl store
the ACL through it after the new mode was already committed.

Quotas aren't enabled at mount in the error state because that writes
the quota files. ocfs2_sync_fs() doesn't flush anything anymore and a
remount to read-only doesn't suspend quotas. Q_GETQUOTA, Q_GETNEXTQUOTA
and Q_SYNC can still start transactions. I've left that alone.

Writes through file descriptors opened before the error fail with EROFS
and umount completes instead of hanging.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/ocfs2/acl.c   |  3 +++
 fs/ocfs2/super.c | 18 +++++++++++++-----
 fs/ocfs2/xattr.c |  8 ++++++++
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c
index 801a2f56ad08..8fb151a55044 100644
--- a/fs/ocfs2/acl.c
+++ b/fs/ocfs2/acl.c
@@ -268,6 +268,9 @@ int ocfs2_iop_set_acl(const struct mnt_idmap *idmap, struct dentry *dentry,
 	struct ocfs2_lock_holder oh;
 	struct inode *inode = d_inode(dentry);
 
+	if (unlikely(ocfs2_emergency_state(OCFS2_SB(inode->i_sb))))
+		return -EROFS;
+
 	had_lock = ocfs2_inode_lock_tracker(inode, &bh, 1, &oh);
 	if (had_lock < 0)
 		return had_lock;
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 67283244426e..6541294641da 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -403,6 +403,10 @@ static int ocfs2_sync_fs(struct super_block *sb, int wait)
 	if (ocfs2_is_hard_readonly(osb))
 		return -EROFS;
 
+	/* Nothing to flush or commit after an error. */
+	if (ocfs2_is_soft_readonly(osb))
+		return 0;
+
 	if (wait) {
 		status = ocfs2_flush_truncate_log(osb);
 		if (status < 0)
@@ -647,8 +651,8 @@ static int ocfs2_reconfigure(struct fs_context *fc)
 
 	/* We're going to/from readonly mode. */
 	if ((bool)(fc->sb_flags & SB_RDONLY) != sb_rdonly(sb)) {
-		/* Disable quota accounting before remounting RO */
-		if (fc->sb_flags & SB_RDONLY) {
+		/* Disable quota accounting before remounting RO, not after an error. */
+		if ((fc->sb_flags & SB_RDONLY) && !sb_emergency_ro(sb)) {
 			ret = ocfs2_susp_quotas(osb, 0);
 			if (ret < 0)
 				goto out;
@@ -1137,7 +1141,7 @@ static int ocfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	/* Now we can initialize quotas because we can afford to wait
 	 * for cluster locks recovery now. That also means that truncation
 	 * log recovery can happen but that waits for proper quota setup */
-	if (!sb_rdonly(sb)) {
+	if (!sb_rdonly(sb) && !sb_emergency_ro(sb)) {
 		status = ocfs2_enable_quotas(osb);
 		if (status < 0) {
 			/* We have to err-out specially here because
@@ -2519,11 +2523,15 @@ static int ocfs2_handle_error(struct super_block *sb)
 		rv = -EIO;
 	} else { /* default option */
 		rv = -EROFS;
-		if (sb_rdonly(sb) && ocfs2_emergency_state(osb))
+		if (ocfs2_emergency_state(osb))
 			return rv;
 
 		pr_crit("OCFS2: File system is now read-only.\n");
-		sb->s_flags |= SB_RDONLY;
+		/*
+		 * SB_RDONLY needs s_umount, see sb_set_emergency_ro(). The
+		 * soft read-only flag makes ocfs2_emergency_state() refuse.
+		 */
+		sb_set_emergency_ro(sb);
 		ocfs2_set_ro_flag(osb, 0);
 	}
 
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index b2bd06083a14..a7d0ec8cdc20 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -7781,6 +7781,9 @@ static int ocfs2_xattr_security_set(const struct xattr_handler *handler,
 				    const char *name, const void *value,
 				    size_t size, int flags)
 {
+	if (unlikely(ocfs2_emergency_state(OCFS2_SB(inode->i_sb))))
+		return -EROFS;
+
 	return ocfs2_xattr_set(inode, OCFS2_XATTR_INDEX_SECURITY,
 			       name, value, size, flags);
 }
@@ -7929,6 +7932,9 @@ static int ocfs2_xattr_trusted_set(const struct xattr_handler *handler,
 				   const char *name, const void *value,
 				   size_t size, int flags)
 {
+	if (unlikely(ocfs2_emergency_state(OCFS2_SB(inode->i_sb))))
+		return -EROFS;
+
 	return ocfs2_xattr_set(inode, OCFS2_XATTR_INDEX_TRUSTED,
 			       name, value, size, flags);
 }
@@ -7964,6 +7970,8 @@ static int ocfs2_xattr_user_set(const struct xattr_handler *handler,
 
 	if (osb->s_mount_opt & OCFS2_MOUNT_NOUSERXATTR)
 		return -EOPNOTSUPP;
+	if (unlikely(ocfs2_emergency_state(osb)))
+		return -EROFS;
 
 	return ocfs2_xattr_set(inode, OCFS2_XATTR_INDEX_USER,
 			       name, value, size, flags);

-- 
2.53.0


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

  parent reply	other threads:[~2026-10-29 16:08 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 16:06 [PATCH v2 00/34] fs: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-29 16:06 ` [PATCH v2 01/34] ecryptfs: don't leak the inode when a lookup can't open the lower file Christian Brauner
2026-10-29 16:06 ` [PATCH v2 02/34] ecryptfs: fall back to a read-only " Christian Brauner
2026-10-29 16:06 ` [PATCH v2 03/34] fs: add a generic emergency read-only state Christian Brauner
2026-10-29 16:06 ` [PATCH v2 04/34] ext2: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-29 16:06 ` [PATCH v2 05/34] jfs: " Christian Brauner
2026-10-29 16:06 ` [PATCH v2 06/34] nilfs2: " Christian Brauner
2026-10-29 16:06 ` [PATCH v2 07/34] fat: " Christian Brauner
2026-10-29 16:06 ` [PATCH v2 08/34] exfat: keep the current mount options on remount Christian Brauner
2026-10-29 16:07 ` [PATCH v2 09/34] exfat: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-29 16:07 ` [PATCH v2 10/34] exfat: use the generic emergency read-only state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 11/34] hfs: don't set SB_RDONLY after MDB write errors Christian Brauner
2026-10-29 16:07 ` [PATCH v2 12/34] hfs: use the generic emergency read-only state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 13/34] hfsplus: don't set SB_RDONLY when creating the attributes tree Christian Brauner
2026-10-29 16:07 ` [PATCH v2 14/34] affs: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-29 16:07 ` [PATCH v2 15/34] ufs: " Christian Brauner
2026-10-29 16:07 ` [PATCH v2 16/34] hpfs: " Christian Brauner
2026-10-29 16:07 ` [PATCH v2 17/34] ntfs: " Christian Brauner
2026-10-29 16:07 ` [PATCH v2 18/34] ntfs: use the generic emergency read-only state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 19/34] ocfs2: keep the current mount options on remount Christian Brauner
2026-10-30  2:58   ` Joseph Qi
2026-10-29 16:07 ` Christian Brauner [this message]
2026-10-30  3:01   ` [PATCH v2 20/34] ocfs2: don't set SB_RDONLY after filesystem errors Joseph Qi
2026-10-29 16:07 ` [PATCH v2 21/34] zonefs: don't set SB_RDONLY after I/O errors Christian Brauner
2026-10-29 16:07 ` [PATCH v2 22/34] zonefs: use the generic emergency read-only state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 23/34] btrfs: check BTRFS_FS_STATE_RO instead of SB_RDONLY at runtime Christian Brauner
2026-10-29 16:07 ` [PATCH v2 24/34] btrfs: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-29 16:07 ` [PATCH v2 25/34] gfs2: wait for a pending withdraw before remounting read-only Christian Brauner
2026-10-29 16:07 ` [PATCH v2 26/34] gfs2: don't set SB_RDONLY on withdraw Christian Brauner
2026-10-29 16:07 ` [PATCH v2 27/34] efivarfs: stop using SB_RDONLY for read-only efivar ops Christian Brauner
2026-10-29 16:07 ` [PATCH v2 28/34] ubifs: don't set SB_RDONLY after errors Christian Brauner
2026-10-30  3:42   ` Zhihao Cheng
2026-10-29 16:07 ` [PATCH v2 29/34] selftests/filesystems: test filesystem errors while frozen Christian Brauner
2026-10-29 16:07 ` [PATCH v2 30/34] ext4: allow remounting read-only in the emergency state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 31/34] ext4: use the generic emergency read-only state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 32/34] selftests/filesystems: test ext4 remounts in the emergency state Christian Brauner
2026-10-29 16:07 ` [PATCH v2 33/34] xfs: allow remounting read-only after a shutdown Christian Brauner
2026-10-29 16:07 ` [PATCH v2 34/34] selftests/filesystems: test xfs remounts " Christian Brauner

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=20261029-work-fs-sb_rdonly-v2-20-2f44f64f0b66@kernel.org \
    --to=brauner@kernel.org \
    --cc=ecryptfs@vger.kernel.org \
    --cc=exfat@lists.linux.dev \
    --cc=gfs2@lists.linux.dev \
    --cc=jack@suse.com \
    --cc=jfs-discussion@lists.sourceforge.net \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-nilfs@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=ntfs@lists.linux.dev \
    --cc=ocfs2-devel@lists.linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox