public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] ocfs2: Refactor read-only checks to use ocfs2_emergency_state
@ 2025-12-02  2:54 Ahmet Eray Karadag
  2025-12-02  2:54 ` [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr Ahmet Eray Karadag
  2025-12-02  2:54 ` [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state Ahmet Eray Karadag
  0 siblings, 2 replies; 9+ messages in thread
From: Ahmet Eray Karadag @ 2025-12-02  2:54 UTC (permalink / raw)
  To: mark, jlbec, joseph.qi
  Cc: ocfs2-devel, linux-kernel, david.hunter.linux, skhan,
	Ahmet Eray Karadag

Hi all,

Following the fix for the `make_bad_inode` validation failure (syzbot ID:
b93b65ee321c97861072), this separate series introduces a new helper
function, `ocfs2_emergency_state()`, to improve and centralize
read-only and error state checking.

This is modeled after the `ext4_emergency_state()` pattern, providing
a single, unified location for checking all filesystem-level emergency
conditions. This makes the code cleaner and ensures that any future
checks (e.g., for fatal error states) can be added in one place.

This series is structured as follows:

1.  The first patch introduces the `ocfs2_emergency_state()` helper
    (currently checking for -EROFS) and applies it to `ocfs2_setattr`
    to provide a "fail-fast" mechanism, as suggested by Albin
    Babu Varghese.
2.  The second patch completes the refactoring by converting all
    remaining read-only checks throughout OCFS2 to use this new helper.

Previous-link: https://lore.kernel.org/all/cover.1763337347.git.eraykrdg1@gmail.com/

Ahmet Eray Karadag (2):
  ocfs2: Add ocfs2_emergency_state helper and apply to setattr
  ocfs2: Convert remaining read-only checks to ocfs2_emergency_state

 fs/ocfs2/buffer_head_io.c |  4 ++--
 fs/ocfs2/file.c           | 23 ++++++++++++++++-------
 fs/ocfs2/inode.c          |  3 +--
 fs/ocfs2/move_extents.c   |  5 +++--
 fs/ocfs2/ocfs2.h          | 18 ++++++++++++++++++
 fs/ocfs2/resize.c         |  8 +++++---
 fs/ocfs2/super.c          |  2 +-
 7 files changed, 46 insertions(+), 17 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr
  2025-12-02  2:54 [PATCH v3 0/2] ocfs2: Refactor read-only checks to use ocfs2_emergency_state Ahmet Eray Karadag
@ 2025-12-02  2:54 ` Ahmet Eray Karadag
  2025-12-02  3:16   ` Heming Zhao
  2025-12-02  7:31   ` Joseph Qi
  2025-12-02  2:54 ` [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state Ahmet Eray Karadag
  1 sibling, 2 replies; 9+ messages in thread
From: Ahmet Eray Karadag @ 2025-12-02  2:54 UTC (permalink / raw)
  To: mark, jlbec, joseph.qi
  Cc: ocfs2-devel, linux-kernel, david.hunter.linux, skhan,
	Ahmet Eray Karadag, Heming Zhao, Albin Babu Varghese

To centralize error checking, follow the pattern of other filesystems
like ext4 (which uses `ext4_emergency_state()`), and prepare for
future enhancements, this patch introduces a new helper function:
`ocfs2_emergency_state()`.

The purpose of this helper is to provide a single, unified location
for checking all filesystem-level emergency conditions. In this
initial implementation, the function only checks for the existing
hard and soft read-only modes, returning -EROFS if either is set.

This provides a foundation where future checks (e.g., for fatal error
states returning -EIO, or shutdown states) can be easily added in
one place.

This patch also adds this new check to the beginning of
`ocfs2_setattr()`. This ensures that operations like `ftruncate`
(which triggered the original BUG) fail-fast with -EROFS when the
filesystem is already in a read-only state.

Suggested-by: Heming Zhao <heming.zhao@suse.com>
Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
---
v2:
 - Introducing new function `ocfs2_is_readonly` to lower the cost
 - Using unlikely for status check
---
v3:
 - Fix compilation error (missing semicolon).
---
 fs/ocfs2/file.c  |  6 ++++++
 fs/ocfs2/ocfs2.h | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 21d797ccccd0..253b4f300127 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1137,6 +1137,12 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 					from_kgid(&init_user_ns, attr->ia_gid) : 0);
 
 	/* ensuring we don't even attempt to truncate a symlink */
+	status = ocfs2_emergency_state(osb);
+	if (unlikely(status)) {
+		mlog_errno(status);
+		goto bail;
+	}
+
 	if (S_ISLNK(inode->i_mode))
 		attr->ia_valid &= ~ATTR_SIZE;
 
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index 6aaa94c554c1..91a861babf74 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -680,6 +680,24 @@ static inline int ocfs2_is_soft_readonly(struct ocfs2_super *osb)
 	return ret;
 }
 
+static inline int ocfs2_is_readonly(struct ocfs2_super *osb)
+{
+	int ret;
+	spin_lock(&osb->osb_lock);
+	ret = osb->osb_flags & (OCFS2_OSB_SOFT_RO | OCFS2_OSB_HARD_RO);
+	spin_unlock(&osb->osb_lock);
+
+	return ret;
+}
+
+static inline int ocfs2_emergency_state(struct ocfs2_super *osb)
+{
+	if (ocfs2_is_readonly(osb)) {
+		return -EROFS;
+	}
+	return 0;
+}
+
 static inline int ocfs2_clusterinfo_valid(struct ocfs2_super *osb)
 {
 	return (osb->s_feature_incompat &
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state
  2025-12-02  2:54 [PATCH v3 0/2] ocfs2: Refactor read-only checks to use ocfs2_emergency_state Ahmet Eray Karadag
  2025-12-02  2:54 ` [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr Ahmet Eray Karadag
@ 2025-12-02  2:54 ` Ahmet Eray Karadag
  2025-12-02  3:16   ` Heming Zhao
  2025-12-02  7:39   ` Joseph Qi
  1 sibling, 2 replies; 9+ messages in thread
From: Ahmet Eray Karadag @ 2025-12-02  2:54 UTC (permalink / raw)
  To: mark, jlbec, joseph.qi
  Cc: ocfs2-devel, linux-kernel, david.hunter.linux, skhan,
	Ahmet Eray Karadag, Albin Babu Varghese

To centralize error checking, follow the pattern of other filesystems
like ext4 (which uses `ext4_emergency_state()`), and prepare for
future enhancements, this patch introduces a new helper function:
`ocfs2_emergency_state()`.

The purpose of this helper is to provide a single, unified location
for checking all filesystem-level emergency conditions. In this
initial implementation, the function only checks for the existing
hard and soft read-only modes, returning -EROFS if either is set.

This provides a foundation where future checks (e.g., for fatal error
states returning -EIO, or shutdown states) can be easily added in
one place.

This patch also adds this new check to the beginning of
`ocfs2_setattr()`. This ensures that operations like `ftruncate`
(which triggered the original BUG) fail-fast with -EROFS when the
filesystem is already in a read-only state.

Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
---
v2:
 - Use `unlikely()` for status check
---
 fs/ocfs2/buffer_head_io.c |  4 ++--
 fs/ocfs2/file.c           | 17 ++++++++++-------
 fs/ocfs2/inode.c          |  3 +--
 fs/ocfs2/move_extents.c   |  5 +++--
 fs/ocfs2/resize.c         |  8 +++++---
 fs/ocfs2/super.c          |  2 +-
 6 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index 8f714406528d..61a0f522c673 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -434,8 +434,8 @@ int ocfs2_write_super_or_backup(struct ocfs2_super *osb,
 	BUG_ON(buffer_jbd(bh));
 	ocfs2_check_super_or_backup(osb->sb, bh->b_blocknr);
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb)) {
-		ret = -EROFS;
+	ret = ocfs2_emergency_state(osb);
+	if (unlikely(ret)) {
 		mlog_errno(ret);
 		goto out;
 	}
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 253b4f300127..540b35ec02e2 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -179,8 +179,9 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
 			      file->f_path.dentry->d_name.name,
 			      (unsigned long long)datasync);
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
-		return -EROFS;
+	ret = ocfs2_emergency_state(osb);
+	if (unlikely(ret))
+		return ret;
 
 	err = file_write_and_wait_range(file, start, end);
 	if (err)
@@ -209,7 +210,7 @@ int ocfs2_should_update_atime(struct inode *inode,
 	struct timespec64 now;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
+	if (unlikely(ocfs2_emergency_state(osb)))
 		return 0;
 
 	if ((inode->i_flags & S_NOATIME) ||
@@ -1949,8 +1950,9 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
 	handle_t *handle;
 	unsigned long long max_off = inode->i_sb->s_maxbytes;
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
-		return -EROFS;
+	ret = ocfs2_emergency_state(osb);
+	if (unlikely(ret))
+		return ret;
 
 	inode_lock(inode);
 
@@ -2713,8 +2715,9 @@ static loff_t ocfs2_remap_file_range(struct file *file_in, loff_t pos_in,
 		return -EINVAL;
 	if (!ocfs2_refcount_tree(osb))
 		return -EOPNOTSUPP;
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
-		return -EROFS;
+	ret = ocfs2_emergency_state(osb);
+	if (unlikely(ret))
+		return ret;
 
 	/* Lock both files against IO */
 	ret = ocfs2_reflink_inodes_lock(inode_in, &in_bh, inode_out, &out_bh);
diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index fcc89856ab95..b7dad049cfa3 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1586,8 +1586,7 @@ static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
 	trace_ocfs2_filecheck_repair_inode_block(
 		(unsigned long long)bh->b_blocknr);
 
-	if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) ||
-	    ocfs2_is_soft_readonly(OCFS2_SB(sb))) {
+	if (unlikely(ocfs2_emergency_state(OCFS2_SB(sb)))) {
 		mlog(ML_ERROR,
 		     "Filecheck: cannot repair dinode #%llu "
 		     "on readonly filesystem\n",
diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
index 86f2631e6360..9d2a2d054aa1 100644
--- a/fs/ocfs2/move_extents.c
+++ b/fs/ocfs2/move_extents.c
@@ -898,8 +898,9 @@ static int ocfs2_move_extents(struct ocfs2_move_extents_context *context)
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
-		return -EROFS;
+	status = ocfs2_emergency_state(osb);
+	if (unlikely(status))
+		return status;
 
 	inode_lock(inode);
 
diff --git a/fs/ocfs2/resize.c b/fs/ocfs2/resize.c
index b0733c08ed13..ae30ae67e220 100644
--- a/fs/ocfs2/resize.c
+++ b/fs/ocfs2/resize.c
@@ -276,8 +276,9 @@ int ocfs2_group_extend(struct inode * inode, int new_clusters)
 	u32 first_new_cluster;
 	u64 lgd_blkno;
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
-		return -EROFS;
+	ret = ocfs2_emergency_state(osb);
+	if (unlikely(ret))
+		return ret;
 
 	if (new_clusters < 0)
 		return -EINVAL;
@@ -466,7 +467,8 @@ int ocfs2_group_add(struct inode *inode, struct ocfs2_new_group_input *input)
 	u16 cl_bpc;
 	u64 bg_ptr;
 
-	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
+	ret = ocfs2_emergency_state(osb);
+	if (unlikely(ret))
 		return -EROFS;
 
 	main_bm_inode = ocfs2_get_system_file_inode(osb,
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 53daa4482406..c6019d260efc 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2487,7 +2487,7 @@ static int ocfs2_handle_error(struct super_block *sb)
 		rv = -EIO;
 	} else { /* default option */
 		rv = -EROFS;
-		if (sb_rdonly(sb) && (ocfs2_is_soft_readonly(osb) || ocfs2_is_hard_readonly(osb)))
+		if (sb_rdonly(sb) && ocfs2_emergency_state(osb))
 			return rv;
 
 		pr_crit("OCFS2: File system is now read-only.\n");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr
  2025-12-02  2:54 ` [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr Ahmet Eray Karadag
@ 2025-12-02  3:16   ` Heming Zhao
  2025-12-02  7:31   ` Joseph Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Heming Zhao @ 2025-12-02  3:16 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, Albin Babu Varghese

On Tue, Dec 02, 2025 at 05:54:57AM +0300, Ahmet Eray Karadag wrote:
> To centralize error checking, follow the pattern of other filesystems
> like ext4 (which uses `ext4_emergency_state()`), and prepare for
> future enhancements, this patch introduces a new helper function:
> `ocfs2_emergency_state()`.
> 
> The purpose of this helper is to provide a single, unified location
> for checking all filesystem-level emergency conditions. In this
> initial implementation, the function only checks for the existing
> hard and soft read-only modes, returning -EROFS if either is set.
> 
> This provides a foundation where future checks (e.g., for fatal error
> states returning -EIO, or shutdown states) can be easily added in
> one place.
> 
> This patch also adds this new check to the beginning of
> `ocfs2_setattr()`. This ensures that operations like `ftruncate`
> (which triggered the original BUG) fail-fast with -EROFS when the
> filesystem is already in a read-only state.
> 
> Suggested-by: Heming Zhao <heming.zhao@suse.com>
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>

Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
> v2:
>  - Introducing new function `ocfs2_is_readonly` to lower the cost
>  - Using unlikely for status check
> ---
> v3:
>  - Fix compilation error (missing semicolon).
> ---
>  fs/ocfs2/file.c  |  6 ++++++
>  fs/ocfs2/ocfs2.h | 18 ++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 21d797ccccd0..253b4f300127 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1137,6 +1137,12 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  					from_kgid(&init_user_ns, attr->ia_gid) : 0);
>  
>  	/* ensuring we don't even attempt to truncate a symlink */
> +	status = ocfs2_emergency_state(osb);
> +	if (unlikely(status)) {
> +		mlog_errno(status);
> +		goto bail;
> +	}
> +
>  	if (S_ISLNK(inode->i_mode))
>  		attr->ia_valid &= ~ATTR_SIZE;
>  
> diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
> index 6aaa94c554c1..91a861babf74 100644
> --- a/fs/ocfs2/ocfs2.h
> +++ b/fs/ocfs2/ocfs2.h
> @@ -680,6 +680,24 @@ static inline int ocfs2_is_soft_readonly(struct ocfs2_super *osb)
>  	return ret;
>  }
>  
> +static inline int ocfs2_is_readonly(struct ocfs2_super *osb)
> +{
> +	int ret;
> +	spin_lock(&osb->osb_lock);
> +	ret = osb->osb_flags & (OCFS2_OSB_SOFT_RO | OCFS2_OSB_HARD_RO);
> +	spin_unlock(&osb->osb_lock);
> +
> +	return ret;
> +}
> +
> +static inline int ocfs2_emergency_state(struct ocfs2_super *osb)
> +{
> +	if (ocfs2_is_readonly(osb)) {
> +		return -EROFS;
> +	}
> +	return 0;
> +}
> +
>  static inline int ocfs2_clusterinfo_valid(struct ocfs2_super *osb)
>  {
>  	return (osb->s_feature_incompat &
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state
  2025-12-02  2:54 ` [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state Ahmet Eray Karadag
@ 2025-12-02  3:16   ` Heming Zhao
  2025-12-02  7:39   ` Joseph Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Heming Zhao @ 2025-12-02  3:16 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, Albin Babu Varghese

On Tue, Dec 02, 2025 at 05:54:58AM +0300, Ahmet Eray Karadag wrote:
> To centralize error checking, follow the pattern of other filesystems
> like ext4 (which uses `ext4_emergency_state()`), and prepare for
> future enhancements, this patch introduces a new helper function:
> `ocfs2_emergency_state()`.
> 
> The purpose of this helper is to provide a single, unified location
> for checking all filesystem-level emergency conditions. In this
> initial implementation, the function only checks for the existing
> hard and soft read-only modes, returning -EROFS if either is set.
> 
> This provides a foundation where future checks (e.g., for fatal error
> states returning -EIO, or shutdown states) can be easily added in
> one place.
> 
> This patch also adds this new check to the beginning of
> `ocfs2_setattr()`. This ensures that operations like `ftruncate`
> (which triggered the original BUG) fail-fast with -EROFS when the
> filesystem is already in a read-only state.
> 
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>

Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
> v2:
>  - Use `unlikely()` for status check
> ---
>  fs/ocfs2/buffer_head_io.c |  4 ++--
>  fs/ocfs2/file.c           | 17 ++++++++++-------
>  fs/ocfs2/inode.c          |  3 +--
>  fs/ocfs2/move_extents.c   |  5 +++--
>  fs/ocfs2/resize.c         |  8 +++++---
>  fs/ocfs2/super.c          |  2 +-
>  6 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index 8f714406528d..61a0f522c673 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -434,8 +434,8 @@ int ocfs2_write_super_or_backup(struct ocfs2_super *osb,
>  	BUG_ON(buffer_jbd(bh));
>  	ocfs2_check_super_or_backup(osb->sb, bh->b_blocknr);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb)) {
> -		ret = -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret)) {
>  		mlog_errno(ret);
>  		goto out;
>  	}
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 253b4f300127..540b35ec02e2 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -179,8 +179,9 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
>  			      file->f_path.dentry->d_name.name,
>  			      (unsigned long long)datasync);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	err = file_write_and_wait_range(file, start, end);
>  	if (err)
> @@ -209,7 +210,7 @@ int ocfs2_should_update_atime(struct inode *inode,
>  	struct timespec64 now;
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return 0;
>  
>  	if ((inode->i_flags & S_NOATIME) ||
> @@ -1949,8 +1950,9 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>  	handle_t *handle;
>  	unsigned long long max_off = inode->i_sb->s_maxbytes;
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	inode_lock(inode);
>  
> @@ -2713,8 +2715,9 @@ static loff_t ocfs2_remap_file_range(struct file *file_in, loff_t pos_in,
>  		return -EINVAL;
>  	if (!ocfs2_refcount_tree(osb))
>  		return -EOPNOTSUPP;
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	/* Lock both files against IO */
>  	ret = ocfs2_reflink_inodes_lock(inode_in, &in_bh, inode_out, &out_bh);
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index fcc89856ab95..b7dad049cfa3 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1586,8 +1586,7 @@ static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
>  	trace_ocfs2_filecheck_repair_inode_block(
>  		(unsigned long long)bh->b_blocknr);
>  
> -	if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) ||
> -	    ocfs2_is_soft_readonly(OCFS2_SB(sb))) {
> +	if (unlikely(ocfs2_emergency_state(OCFS2_SB(sb)))) {
>  		mlog(ML_ERROR,
>  		     "Filecheck: cannot repair dinode #%llu "
>  		     "on readonly filesystem\n",
> diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
> index 86f2631e6360..9d2a2d054aa1 100644
> --- a/fs/ocfs2/move_extents.c
> +++ b/fs/ocfs2/move_extents.c
> @@ -898,8 +898,9 @@ static int ocfs2_move_extents(struct ocfs2_move_extents_context *context)
>  	struct buffer_head *di_bh = NULL;
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	status = ocfs2_emergency_state(osb);
> +	if (unlikely(status))
> +		return status;
>  
>  	inode_lock(inode);
>  
> diff --git a/fs/ocfs2/resize.c b/fs/ocfs2/resize.c
> index b0733c08ed13..ae30ae67e220 100644
> --- a/fs/ocfs2/resize.c
> +++ b/fs/ocfs2/resize.c
> @@ -276,8 +276,9 @@ int ocfs2_group_extend(struct inode * inode, int new_clusters)
>  	u32 first_new_cluster;
>  	u64 lgd_blkno;
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	if (new_clusters < 0)
>  		return -EINVAL;
> @@ -466,7 +467,8 @@ int ocfs2_group_add(struct inode *inode, struct ocfs2_new_group_input *input)
>  	u16 cl_bpc;
>  	u64 bg_ptr;
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
>  		return -EROFS;
>  
>  	main_bm_inode = ocfs2_get_system_file_inode(osb,
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 53daa4482406..c6019d260efc 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -2487,7 +2487,7 @@ static int ocfs2_handle_error(struct super_block *sb)
>  		rv = -EIO;
>  	} else { /* default option */
>  		rv = -EROFS;
> -		if (sb_rdonly(sb) && (ocfs2_is_soft_readonly(osb) || ocfs2_is_hard_readonly(osb)))
> +		if (sb_rdonly(sb) && ocfs2_emergency_state(osb))
>  			return rv;
>  
>  		pr_crit("OCFS2: File system is now read-only.\n");
> -- 
> 2.43.0
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr
  2025-12-02  2:54 ` [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr Ahmet Eray Karadag
  2025-12-02  3:16   ` Heming Zhao
@ 2025-12-02  7:31   ` Joseph Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Joseph Qi @ 2025-12-02  7:31 UTC (permalink / raw)
  To: Ahmet Eray Karadag, mark, jlbec
  Cc: ocfs2-devel, linux-kernel, david.hunter.linux, skhan, Heming Zhao,
	Albin Babu Varghese



On 2025/12/2 10:54, Ahmet Eray Karadag wrote:
> To centralize error checking, follow the pattern of other filesystems
> like ext4 (which uses `ext4_emergency_state()`), and prepare for
> future enhancements, this patch introduces a new helper function:
> `ocfs2_emergency_state()`.
> 
> The purpose of this helper is to provide a single, unified location
> for checking all filesystem-level emergency conditions. In this
> initial implementation, the function only checks for the existing
> hard and soft read-only modes, returning -EROFS if either is set.
> 
> This provides a foundation where future checks (e.g., for fatal error
> states returning -EIO, or shutdown states) can be easily added in
> one place.
> 
> This patch also adds this new check to the beginning of
> `ocfs2_setattr()`. This ensures that operations like `ftruncate`
> (which triggered the original BUG) fail-fast with -EROFS when the
> filesystem is already in a read-only state.
> 
> Suggested-by: Heming Zhao <heming.zhao@suse.com>
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> ---
> v2:
>  - Introducing new function `ocfs2_is_readonly` to lower the cost
>  - Using unlikely for status check
> ---
> v3:
>  - Fix compilation error (missing semicolon).
> ---
>  fs/ocfs2/file.c  |  6 ++++++
>  fs/ocfs2/ocfs2.h | 18 ++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 21d797ccccd0..253b4f300127 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1137,6 +1137,12 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  					from_kgid(&init_user_ns, attr->ia_gid) : 0);
>  
>  	/* ensuring we don't even attempt to truncate a symlink */

The above comments is for "if (S_ISLNK(inode->i_mode))".
So it should be move down just before symlink check.

> +	status = ocfs2_emergency_state(osb);
> +	if (unlikely(status)) {
> +		mlog_errno(status);
> +		goto bail;
> +	}
> +
>  	if (S_ISLNK(inode->i_mode))
>  		attr->ia_valid &= ~ATTR_SIZE;
>  
> diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
> index 6aaa94c554c1..91a861babf74 100644
> --- a/fs/ocfs2/ocfs2.h
> +++ b/fs/ocfs2/ocfs2.h
> @@ -680,6 +680,24 @@ static inline int ocfs2_is_soft_readonly(struct ocfs2_super *osb)
>  	return ret;
>  }
>  
> +static inline int ocfs2_is_readonly(struct ocfs2_super *osb)
> +{
> +	int ret;
> +	spin_lock(&osb->osb_lock);
> +	ret = osb->osb_flags & (OCFS2_OSB_SOFT_RO | OCFS2_OSB_HARD_RO);
> +	spin_unlock(&osb->osb_lock);
> +
> +	return ret;
> +}
> +
> +static inline int ocfs2_emergency_state(struct ocfs2_super *osb)
> +{
> +	if (ocfs2_is_readonly(osb)) {

The '{' can be eliminated.

Joseph

> +		return -EROFS;
> +	}
> +	return 0;
> +}
> +
>  static inline int ocfs2_clusterinfo_valid(struct ocfs2_super *osb)
>  {
>  	return (osb->s_feature_incompat &


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state
  2025-12-02  2:54 ` [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state Ahmet Eray Karadag
  2025-12-02  3:16   ` Heming Zhao
@ 2025-12-02  7:39   ` Joseph Qi
  2025-12-03  0:46     ` Ahmet Eray Karadag
  1 sibling, 1 reply; 9+ messages in thread
From: Joseph Qi @ 2025-12-02  7:39 UTC (permalink / raw)
  To: Ahmet Eray Karadag, mark, jlbec, Heming Zhao
  Cc: ocfs2-devel, linux-kernel, david.hunter.linux, skhan,
	Albin Babu Varghese



On 2025/12/2 10:54, Ahmet Eray Karadag wrote:
> To centralize error checking, follow the pattern of other filesystems
> like ext4 (which uses `ext4_emergency_state()`), and prepare for
> future enhancements, this patch introduces a new helper function:
> `ocfs2_emergency_state()`.
> 
> The purpose of this helper is to provide a single, unified location
> for checking all filesystem-level emergency conditions. In this
> initial implementation, the function only checks for the existing
> hard and soft read-only modes, returning -EROFS if either is set.
> 
> This provides a foundation where future checks (e.g., for fatal error
> states returning -EIO, or shutdown states) can be easily added in
> one place.
> 
> This patch also adds this new check to the beginning of
> `ocfs2_setattr()`. This ensures that operations like `ftruncate`
> (which triggered the original BUG) fail-fast with -EROFS when the
> filesystem is already in a read-only state.
> 

The above commit log is the same with patch 1 and doesn't reflect
the following changes.

> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> ---
> v2:
>  - Use `unlikely()` for status check
> ---
>  fs/ocfs2/buffer_head_io.c |  4 ++--
>  fs/ocfs2/file.c           | 17 ++++++++++-------
>  fs/ocfs2/inode.c          |  3 +--
>  fs/ocfs2/move_extents.c   |  5 +++--
>  fs/ocfs2/resize.c         |  8 +++++---
>  fs/ocfs2/super.c          |  2 +-
>  6 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index 8f714406528d..61a0f522c673 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -434,8 +434,8 @@ int ocfs2_write_super_or_backup(struct ocfs2_super *osb,
>  	BUG_ON(buffer_jbd(bh));
>  	ocfs2_check_super_or_backup(osb->sb, bh->b_blocknr);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb)) {
> -		ret = -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret)) {

I'd like use the following style:
if (ocfs2_emergency_state(osb)) {
	ret = -EROFS;
	...
}

So that ocfs2_emergency_state() can be expended with other cases(properly
other return code), without touching these code flows.

Joseph

>  		mlog_errno(ret);
>  		goto out;
>  	}
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 253b4f300127..540b35ec02e2 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -179,8 +179,9 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
>  			      file->f_path.dentry->d_name.name,
>  			      (unsigned long long)datasync);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	err = file_write_and_wait_range(file, start, end);
>  	if (err)
> @@ -209,7 +210,7 @@ int ocfs2_should_update_atime(struct inode *inode,
>  	struct timespec64 now;
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return 0;
>  
>  	if ((inode->i_flags & S_NOATIME) ||
> @@ -1949,8 +1950,9 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>  	handle_t *handle;
>  	unsigned long long max_off = inode->i_sb->s_maxbytes;
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	inode_lock(inode);
>  
> @@ -2713,8 +2715,9 @@ static loff_t ocfs2_remap_file_range(struct file *file_in, loff_t pos_in,
>  		return -EINVAL;
>  	if (!ocfs2_refcount_tree(osb))
>  		return -EOPNOTSUPP;
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	/* Lock both files against IO */
>  	ret = ocfs2_reflink_inodes_lock(inode_in, &in_bh, inode_out, &out_bh);
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index fcc89856ab95..b7dad049cfa3 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1586,8 +1586,7 @@ static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
>  	trace_ocfs2_filecheck_repair_inode_block(
>  		(unsigned long long)bh->b_blocknr);
>  
> -	if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) ||
> -	    ocfs2_is_soft_readonly(OCFS2_SB(sb))) {
> +	if (unlikely(ocfs2_emergency_state(OCFS2_SB(sb)))) {
>  		mlog(ML_ERROR,
>  		     "Filecheck: cannot repair dinode #%llu "
>  		     "on readonly filesystem\n",
> diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
> index 86f2631e6360..9d2a2d054aa1 100644
> --- a/fs/ocfs2/move_extents.c
> +++ b/fs/ocfs2/move_extents.c
> @@ -898,8 +898,9 @@ static int ocfs2_move_extents(struct ocfs2_move_extents_context *context)
>  	struct buffer_head *di_bh = NULL;
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	status = ocfs2_emergency_state(osb);
> +	if (unlikely(status))
> +		return status;
>  
>  	inode_lock(inode);
>  
> diff --git a/fs/ocfs2/resize.c b/fs/ocfs2/resize.c
> index b0733c08ed13..ae30ae67e220 100644
> --- a/fs/ocfs2/resize.c
> +++ b/fs/ocfs2/resize.c
> @@ -276,8 +276,9 @@ int ocfs2_group_extend(struct inode * inode, int new_clusters)
>  	u32 first_new_cluster;
>  	u64 lgd_blkno;
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> -		return -EROFS;
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
> +		return ret;
>  
>  	if (new_clusters < 0)
>  		return -EINVAL;
> @@ -466,7 +467,8 @@ int ocfs2_group_add(struct inode *inode, struct ocfs2_new_group_input *input)
>  	u16 cl_bpc;
>  	u64 bg_ptr;
>  
> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> +	ret = ocfs2_emergency_state(osb);
> +	if (unlikely(ret))
>  		return -EROFS;
>  
>  	main_bm_inode = ocfs2_get_system_file_inode(osb,
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 53daa4482406..c6019d260efc 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -2487,7 +2487,7 @@ static int ocfs2_handle_error(struct super_block *sb)
>  		rv = -EIO;
>  	} else { /* default option */
>  		rv = -EROFS;
> -		if (sb_rdonly(sb) && (ocfs2_is_soft_readonly(osb) || ocfs2_is_hard_readonly(osb)))
> +		if (sb_rdonly(sb) && ocfs2_emergency_state(osb))
>  			return rv;
>  
>  		pr_crit("OCFS2: File system is now read-only.\n");


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state
  2025-12-02  7:39   ` Joseph Qi
@ 2025-12-03  0:46     ` Ahmet Eray Karadag
  2025-12-03  1:00       ` Joseph Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Ahmet Eray Karadag @ 2025-12-03  0:46 UTC (permalink / raw)
  To: Joseph Qi
  Cc: mark, jlbec, Heming Zhao, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, Albin Babu Varghese

On Tue, Dec 02, 2025 at 03:39:30PM +0800, Joseph Qi wrote:
> 
> 
> On 2025/12/2 10:54, Ahmet Eray Karadag wrote:
> > To centralize error checking, follow the pattern of other filesystems
> > like ext4 (which uses `ext4_emergency_state()`), and prepare for
> > future enhancements, this patch introduces a new helper function:
> > `ocfs2_emergency_state()`.
> > 
> > The purpose of this helper is to provide a single, unified location
> > for checking all filesystem-level emergency conditions. In this
> > initial implementation, the function only checks for the existing
> > hard and soft read-only modes, returning -EROFS if either is set.
> > 
> > This provides a foundation where future checks (e.g., for fatal error
> > states returning -EIO, or shutdown states) can be easily added in
> > one place.
> > 
> > This patch also adds this new check to the beginning of
> > `ocfs2_setattr()`. This ensures that operations like `ftruncate`
> > (which triggered the original BUG) fail-fast with -EROFS when the
> > filesystem is already in a read-only state.
> > 
> 
> The above commit log is the same with patch 1 and doesn't reflect
> the following changes.
> 
> > Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> > ---
> > v2:
> >  - Use `unlikely()` for status check
> > ---
> >  fs/ocfs2/buffer_head_io.c |  4 ++--
> >  fs/ocfs2/file.c           | 17 ++++++++++-------
> >  fs/ocfs2/inode.c          |  3 +--
> >  fs/ocfs2/move_extents.c   |  5 +++--
> >  fs/ocfs2/resize.c         |  8 +++++---
> >  fs/ocfs2/super.c          |  2 +-
> >  6 files changed, 22 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> > index 8f714406528d..61a0f522c673 100644
> > --- a/fs/ocfs2/buffer_head_io.c
> > +++ b/fs/ocfs2/buffer_head_io.c
> > @@ -434,8 +434,8 @@ int ocfs2_write_super_or_backup(struct ocfs2_super *osb,
> >  	BUG_ON(buffer_jbd(bh));
> >  	ocfs2_check_super_or_backup(osb->sb, bh->b_blocknr);
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb)) {
> > -		ret = -EROFS;
> > +	ret = ocfs2_emergency_state(osb);
> > +	if (unlikely(ret)) {
> 
> I'd like use the following style:
> if (ocfs2_emergency_state(osb)) {
> 	ret = -EROFS;
> 	...
> }
> 
> So that ocfs2_emergency_state() can be expended with other cases(properly
> other return code), without touching these code flows.

In the future, there might be other return codes added to ocfs2_emergency_state().
Correct me if I'm wrong, but with your proposed style, we would only return
-EROFS in any case. Shouldn't we consider future improvements?

Thanks,
Ahmet Eray
> 
> Joseph
> 
> >  		mlog_errno(ret);
> >  		goto out;
> >  	}
> > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> > index 253b4f300127..540b35ec02e2 100644
> > --- a/fs/ocfs2/file.c
> > +++ b/fs/ocfs2/file.c
> > @@ -179,8 +179,9 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
> >  			      file->f_path.dentry->d_name.name,
> >  			      (unsigned long long)datasync);
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > -		return -EROFS;
> > +	ret = ocfs2_emergency_state(osb);
> > +	if (unlikely(ret))
> > +		return ret;
> >  
> >  	err = file_write_and_wait_range(file, start, end);
> >  	if (err)
> > @@ -209,7 +210,7 @@ int ocfs2_should_update_atime(struct inode *inode,
> >  	struct timespec64 now;
> >  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > +	if (unlikely(ocfs2_emergency_state(osb)))
> >  		return 0;
> >  
> >  	if ((inode->i_flags & S_NOATIME) ||
> > @@ -1949,8 +1950,9 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
> >  	handle_t *handle;
> >  	unsigned long long max_off = inode->i_sb->s_maxbytes;
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > -		return -EROFS;
> > +	ret = ocfs2_emergency_state(osb);
> > +	if (unlikely(ret))
> > +		return ret;
> >  
> >  	inode_lock(inode);
> >  
> > @@ -2713,8 +2715,9 @@ static loff_t ocfs2_remap_file_range(struct file *file_in, loff_t pos_in,
> >  		return -EINVAL;
> >  	if (!ocfs2_refcount_tree(osb))
> >  		return -EOPNOTSUPP;
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > -		return -EROFS;
> > +	ret = ocfs2_emergency_state(osb);
> > +	if (unlikely(ret))
> > +		return ret;
> >  
> >  	/* Lock both files against IO */
> >  	ret = ocfs2_reflink_inodes_lock(inode_in, &in_bh, inode_out, &out_bh);
> > diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> > index fcc89856ab95..b7dad049cfa3 100644
> > --- a/fs/ocfs2/inode.c
> > +++ b/fs/ocfs2/inode.c
> > @@ -1586,8 +1586,7 @@ static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
> >  	trace_ocfs2_filecheck_repair_inode_block(
> >  		(unsigned long long)bh->b_blocknr);
> >  
> > -	if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) ||
> > -	    ocfs2_is_soft_readonly(OCFS2_SB(sb))) {
> > +	if (unlikely(ocfs2_emergency_state(OCFS2_SB(sb)))) {
> >  		mlog(ML_ERROR,
> >  		     "Filecheck: cannot repair dinode #%llu "
> >  		     "on readonly filesystem\n",
> > diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
> > index 86f2631e6360..9d2a2d054aa1 100644
> > --- a/fs/ocfs2/move_extents.c
> > +++ b/fs/ocfs2/move_extents.c
> > @@ -898,8 +898,9 @@ static int ocfs2_move_extents(struct ocfs2_move_extents_context *context)
> >  	struct buffer_head *di_bh = NULL;
> >  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > -		return -EROFS;
> > +	status = ocfs2_emergency_state(osb);
> > +	if (unlikely(status))
> > +		return status;
> >  
> >  	inode_lock(inode);
> >  
> > diff --git a/fs/ocfs2/resize.c b/fs/ocfs2/resize.c
> > index b0733c08ed13..ae30ae67e220 100644
> > --- a/fs/ocfs2/resize.c
> > +++ b/fs/ocfs2/resize.c
> > @@ -276,8 +276,9 @@ int ocfs2_group_extend(struct inode * inode, int new_clusters)
> >  	u32 first_new_cluster;
> >  	u64 lgd_blkno;
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > -		return -EROFS;
> > +	ret = ocfs2_emergency_state(osb);
> > +	if (unlikely(ret))
> > +		return ret;
> >  
> >  	if (new_clusters < 0)
> >  		return -EINVAL;
> > @@ -466,7 +467,8 @@ int ocfs2_group_add(struct inode *inode, struct ocfs2_new_group_input *input)
> >  	u16 cl_bpc;
> >  	u64 bg_ptr;
> >  
> > -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
> > +	ret = ocfs2_emergency_state(osb);
> > +	if (unlikely(ret))
> >  		return -EROFS;
> >  
> >  	main_bm_inode = ocfs2_get_system_file_inode(osb,
> > diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> > index 53daa4482406..c6019d260efc 100644
> > --- a/fs/ocfs2/super.c
> > +++ b/fs/ocfs2/super.c
> > @@ -2487,7 +2487,7 @@ static int ocfs2_handle_error(struct super_block *sb)
> >  		rv = -EIO;
> >  	} else { /* default option */
> >  		rv = -EROFS;
> > -		if (sb_rdonly(sb) && (ocfs2_is_soft_readonly(osb) || ocfs2_is_hard_readonly(osb)))
> > +		if (sb_rdonly(sb) && ocfs2_emergency_state(osb))
> >  			return rv;
> >  
> >  		pr_crit("OCFS2: File system is now read-only.\n");
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state
  2025-12-03  0:46     ` Ahmet Eray Karadag
@ 2025-12-03  1:00       ` Joseph Qi
  0 siblings, 0 replies; 9+ messages in thread
From: Joseph Qi @ 2025-12-03  1:00 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: mark, jlbec, Heming Zhao, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, Albin Babu Varghese



On 2025/12/3 08:46, Ahmet Eray Karadag wrote:
> On Tue, Dec 02, 2025 at 03:39:30PM +0800, Joseph Qi wrote:
>>
>>
>> On 2025/12/2 10:54, Ahmet Eray Karadag wrote:
>>> To centralize error checking, follow the pattern of other filesystems
>>> like ext4 (which uses `ext4_emergency_state()`), and prepare for
>>> future enhancements, this patch introduces a new helper function:
>>> `ocfs2_emergency_state()`.
>>>
>>> The purpose of this helper is to provide a single, unified location
>>> for checking all filesystem-level emergency conditions. In this
>>> initial implementation, the function only checks for the existing
>>> hard and soft read-only modes, returning -EROFS if either is set.
>>>
>>> This provides a foundation where future checks (e.g., for fatal error
>>> states returning -EIO, or shutdown states) can be easily added in
>>> one place.
>>>
>>> This patch also adds this new check to the beginning of
>>> `ocfs2_setattr()`. This ensures that operations like `ftruncate`
>>> (which triggered the original BUG) fail-fast with -EROFS when the
>>> filesystem is already in a read-only state.
>>>
>>
>> The above commit log is the same with patch 1 and doesn't reflect
>> the following changes.
>>
>>> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
>>> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
>>> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
>>> ---
>>> v2:
>>>  - Use `unlikely()` for status check
>>> ---
>>>  fs/ocfs2/buffer_head_io.c |  4 ++--
>>>  fs/ocfs2/file.c           | 17 ++++++++++-------
>>>  fs/ocfs2/inode.c          |  3 +--
>>>  fs/ocfs2/move_extents.c   |  5 +++--
>>>  fs/ocfs2/resize.c         |  8 +++++---
>>>  fs/ocfs2/super.c          |  2 +-
>>>  6 files changed, 22 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
>>> index 8f714406528d..61a0f522c673 100644
>>> --- a/fs/ocfs2/buffer_head_io.c
>>> +++ b/fs/ocfs2/buffer_head_io.c
>>> @@ -434,8 +434,8 @@ int ocfs2_write_super_or_backup(struct ocfs2_super *osb,
>>>  	BUG_ON(buffer_jbd(bh));
>>>  	ocfs2_check_super_or_backup(osb->sb, bh->b_blocknr);
>>>  
>>> -	if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb)) {
>>> -		ret = -EROFS;
>>> +	ret = ocfs2_emergency_state(osb);
>>> +	if (unlikely(ret)) {
>>
>> I'd like use the following style:
>> if (ocfs2_emergency_state(osb)) {
>> 	ret = -EROFS;
>> 	...
>> }
>>
>> So that ocfs2_emergency_state() can be expended with other cases(properly
>> other return code), without touching these code flows.
> 
> In the future, there might be other return codes added to ocfs2_emergency_state().
> Correct me if I'm wrong, but with your proposed style, we would only return
> -EROFS in any case. Shouldn't we consider future improvements?
> 
> Thanks,
> Ahmet Eray
>>
Return EROFS here is to be consistent with before.
i.e. No functional change.

Joseph


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-12-03  1:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02  2:54 [PATCH v3 0/2] ocfs2: Refactor read-only checks to use ocfs2_emergency_state Ahmet Eray Karadag
2025-12-02  2:54 ` [PATCH v3 1/2] ocfs2: Add ocfs2_emergency_state helper and apply to setattr Ahmet Eray Karadag
2025-12-02  3:16   ` Heming Zhao
2025-12-02  7:31   ` Joseph Qi
2025-12-02  2:54 ` [PATCH v3 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state Ahmet Eray Karadag
2025-12-02  3:16   ` Heming Zhao
2025-12-02  7:39   ` Joseph Qi
2025-12-03  0:46     ` Ahmet Eray Karadag
2025-12-03  1:00       ` Joseph Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox