Re: [PATCH v4 2/2] ocfs2: Convert remaining read-only checks to ocfs2_emergency_state

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

On 2025/12/3 11:34, Ahmet Eray Karadag wrote:
> Now that the centralized `ocfs2_emergency_state()` helper is available,
> refactor remaining filesystem-wide checks for `ocfs2_is_soft_readonly`
> and `ocfs2_is_hard_readonly` to use this new function.
> 
> To ensure strict consistency with the previous behavior and guarantee
> no functional changes, the call sites continue to explicitly return
> -EROFS when the emergency state is detected. This standardizes the
> check logic while preserving the existing error handling flow.
> 
> 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>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> v2:
>  - Use `unlikely()` for status check
> ---
> v4:
>  - Ensure consistency with the previous behavior as requested
> ---
>  fs/ocfs2/buffer_head_io.c | 2 +-
>  fs/ocfs2/file.c           | 8 ++++----
>  fs/ocfs2/inode.c          | 3 +--
>  fs/ocfs2/move_extents.c   | 2 +-
>  fs/ocfs2/resize.c         | 4 ++--
>  fs/ocfs2/super.c          | 2 +-
>  6 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index 8f714406528d..701d27d908d4 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -434,7 +434,7 @@ 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)) {
> +	if (unlikely(ocfs2_emergency_state(osb))) {
>  		ret = -EROFS;
>  		mlog_errno(ret);
>  		goto out;
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index ddca292944d7..732c61599159 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -179,7 +179,7 @@ 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))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return -EROFS;
>  
>  	err = file_write_and_wait_range(file, start, end);
> @@ -209,7 +209,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,7 +1949,7 @@ 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))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return -EROFS;
>  
>  	inode_lock(inode);
> @@ -2713,7 +2713,7 @@ 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))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return -EROFS;
>  
>  	/* Lock both files against IO */
> 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..71025d7b2d4b 100644
> --- a/fs/ocfs2/move_extents.c
> +++ b/fs/ocfs2/move_extents.c
> @@ -898,7 +898,7 @@ 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))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return -EROFS;
>  
>  	inode_lock(inode);
> diff --git a/fs/ocfs2/resize.c b/fs/ocfs2/resize.c
> index b0733c08ed13..ac3ec2c21119 100644
> --- a/fs/ocfs2/resize.c
> +++ b/fs/ocfs2/resize.c
> @@ -276,7 +276,7 @@ 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))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		return -EROFS;
>  
>  	if (new_clusters < 0)
> @@ -466,7 +466,7 @@ 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))
> +	if (unlikely(ocfs2_emergency_state(osb)))
>  		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");