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) --- 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