linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] f2fs: fix to check readonly condition correctly
@ 2023-04-02 11:28 Chao Yu
  2023-04-03 17:59 ` Jaegeuk Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2023-04-02 11:28 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

This patch does below changes:

- If f2fs enables readonly feature or device is readonly, allow to
mount readonly mode only
- Introduce f2fs_dev_is_readonly() to indicate whether image or device
is readonly
- remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
and f2fs_convert_inline_inode()
- enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
is writable

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/checkpoint.c |  2 +-
 fs/f2fs/f2fs.h       | 10 +++++-----
 fs/f2fs/inline.c     |  3 +--
 fs/f2fs/super.c      | 13 +++++++------
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 8e1db5752fff..1eef597ed393 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	unsigned long long ckpt_ver;
 	int err = 0;
 
-	if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi))
+	if (f2fs_readonly(sbi->sb))
 		return -EROFS;
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 2d4a7ef62537..7de95133478a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi)
 	return false;
 }
 
+static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi)
+{
+	return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi);
+}
+
 static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi)
 {
 	return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS;
@@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs,
 	}
 }
 
-static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi)
-{
-	return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
-}
-
 #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
 #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
 
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 72269e7efd26..2c36f2dc2317 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
 	struct page *ipage, *page;
 	int err = 0;
 
-	if (!f2fs_has_inline_data(inode) ||
-			f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
+	if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb))
 		return 0;
 
 	err = f2fs_dquot_initialize(inode);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d016f398fcad..db7649010c12 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 		return -EINVAL;
 	}
 
-	if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) {
+	if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
+		f2fs_err(sbi, "Allow to mount readonly mode only");
+		return -EROFS;
+	}
+
+	if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) {
 		f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode");
 		return -EINVAL;
 	}
 
-	if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
-		f2fs_err(sbi, "Allow to mount readonly mode only");
-		return -EROFS;
-	}
 	return 0;
 }
 
@@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi)
 	set_opt(sbi, MERGE_CHECKPOINT);
 	F2FS_OPTION(sbi).unusable_cap = 0;
 	sbi->sb->s_flags |= SB_LAZYTIME;
-	if (!f2fs_is_readonly(sbi))
+	if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi))
 		set_opt(sbi, FLUSH_MERGE);
 	if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi))
 		set_opt(sbi, DISCARD);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix to check readonly condition correctly
  2023-04-02 11:28 [PATCH] f2fs: fix to check readonly condition correctly Chao Yu
@ 2023-04-03 17:59 ` Jaegeuk Kim
  2023-04-04 15:03   ` Chao Yu
  2023-04-10  2:19   ` Chao Yu
  0 siblings, 2 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2023-04-03 17:59 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel

On 04/02, Chao Yu wrote:
> This patch does below changes:
> 
> - If f2fs enables readonly feature or device is readonly, allow to
> mount readonly mode only
> - Introduce f2fs_dev_is_readonly() to indicate whether image or device
> is readonly
> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
> and f2fs_convert_inline_inode()
> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
> is writable

What is the problem to solve here?

> 
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/checkpoint.c |  2 +-
>  fs/f2fs/f2fs.h       | 10 +++++-----
>  fs/f2fs/inline.c     |  3 +--
>  fs/f2fs/super.c      | 13 +++++++------
>  4 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 8e1db5752fff..1eef597ed393 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	unsigned long long ckpt_ver;
>  	int err = 0;
>  
> -	if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi))
> +	if (f2fs_readonly(sbi->sb))
>  		return -EROFS;
>  
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 2d4a7ef62537..7de95133478a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi)
>  	return false;
>  }
>  
> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi)
> +{
> +	return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi);
> +}
> +
>  static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi)
>  {
>  	return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS;
> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs,
>  	}
>  }
>  
> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi)
> -{
> -	return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
> -}
> -
>  #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
>  #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
>  
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 72269e7efd26..2c36f2dc2317 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
>  	struct page *ipage, *page;
>  	int err = 0;
>  
> -	if (!f2fs_has_inline_data(inode) ||
> -			f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
> +	if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb))
>  		return 0;
>  
>  	err = f2fs_dquot_initialize(inode);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d016f398fcad..db7649010c12 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>  		return -EINVAL;
>  	}
>  
> -	if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) {
> +	if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
> +		f2fs_err(sbi, "Allow to mount readonly mode only");
> +		return -EROFS;
> +	}
> +
> +	if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) {
>  		f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode");
>  		return -EINVAL;
>  	}
>  
> -	if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
> -		f2fs_err(sbi, "Allow to mount readonly mode only");
> -		return -EROFS;
> -	}
>  	return 0;
>  }
>  
> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi)
>  	set_opt(sbi, MERGE_CHECKPOINT);
>  	F2FS_OPTION(sbi).unusable_cap = 0;
>  	sbi->sb->s_flags |= SB_LAZYTIME;
> -	if (!f2fs_is_readonly(sbi))
> +	if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi))
>  		set_opt(sbi, FLUSH_MERGE);
>  	if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi))
>  		set_opt(sbi, DISCARD);
> -- 
> 2.36.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix to check readonly condition correctly
  2023-04-03 17:59 ` Jaegeuk Kim
@ 2023-04-04 15:03   ` Chao Yu
  2023-04-10  2:19   ` Chao Yu
  1 sibling, 0 replies; 6+ messages in thread
From: Chao Yu @ 2023-04-04 15:03 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel

On 2023/4/4 1:59, Jaegeuk Kim wrote:
> On 04/02, Chao Yu wrote:
>> This patch does below changes:
>>
>> - If f2fs enables readonly feature or device is readonly, allow to
>> mount readonly mode only
>> - Introduce f2fs_dev_is_readonly() to indicate whether image or device
>> is readonly
>> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
>> and f2fs_convert_inline_inode()
>> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
>> is writable
> 
> What is the problem to solve here?

With below case, it can mount multi-device image w/ rw option, however
one of secondary device is set as ro, later update will cause panic, so
I introduced f2fs_dev_is_readonly(), and check multi-devices rw status
in parse_options() w/ it in order to avoid such inconsistent mount status.

mkfs.f2fs -c /dev/zram1 /dev/zram0 -f
blockdev --setro /dev/zram1
mount -t f2fs /dev/zram0 /mnt/f2fs/
mount: /mnt/f2fs: WARNING: source write-protected, mounted read-only.
mount -t f2fs -o remount,rw /mnt/f2fs/
dd if=/dev/zero  of=/mnt/f2fs/file bs=1M count=8192

kernel BUG at fs/f2fs/inline.c:258!
RIP: 0010:f2fs_write_inline_data+0x23e/0x2d0 [f2fs]
Call Trace:
  f2fs_write_single_data_page+0x26b/0x9f0 [f2fs]
  f2fs_write_cache_pages+0x389/0xa60 [f2fs]
  __f2fs_write_data_pages+0x26b/0x2d0 [f2fs]
  f2fs_write_data_pages+0x2e/0x40 [f2fs]
  do_writepages+0xd3/0x1b0
  __writeback_single_inode+0x5b/0x420
  writeback_sb_inodes+0x236/0x5a0
  __writeback_inodes_wb+0x56/0xf0
  wb_writeback+0x2a3/0x490
  wb_do_writeback+0x2b2/0x330
  wb_workfn+0x6a/0x260
  process_one_work+0x270/0x5e0
  worker_thread+0x52/0x3e0
  kthread+0xf4/0x120
  ret_from_fork+0x29/0x50

Thanks,

> 
>>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/checkpoint.c |  2 +-
>>   fs/f2fs/f2fs.h       | 10 +++++-----
>>   fs/f2fs/inline.c     |  3 +--
>>   fs/f2fs/super.c      | 13 +++++++------
>>   4 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 8e1db5752fff..1eef597ed393 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>   	unsigned long long ckpt_ver;
>>   	int err = 0;
>>   
>> -	if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi))
>> +	if (f2fs_readonly(sbi->sb))
>>   		return -EROFS;
>>   
>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 2d4a7ef62537..7de95133478a 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi)
>>   	return false;
>>   }
>>   
>> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi)
>> +{
>> +	return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi);
>> +}
>> +
>>   static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi)
>>   {
>>   	return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS;
>> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs,
>>   	}
>>   }
>>   
>> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi)
>> -{
>> -	return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
>> -}
>> -
>>   #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
>>   #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
>>   
>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>> index 72269e7efd26..2c36f2dc2317 100644
>> --- a/fs/f2fs/inline.c
>> +++ b/fs/f2fs/inline.c
>> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
>>   	struct page *ipage, *page;
>>   	int err = 0;
>>   
>> -	if (!f2fs_has_inline_data(inode) ||
>> -			f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
>> +	if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb))
>>   		return 0;
>>   
>>   	err = f2fs_dquot_initialize(inode);
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index d016f398fcad..db7649010c12 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>   		return -EINVAL;
>>   	}
>>   
>> -	if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) {
>> +	if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
>> +		f2fs_err(sbi, "Allow to mount readonly mode only");
>> +		return -EROFS;
>> +	}
>> +
>> +	if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) {
>>   		f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode");
>>   		return -EINVAL;
>>   	}
>>   
>> -	if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
>> -		f2fs_err(sbi, "Allow to mount readonly mode only");
>> -		return -EROFS;
>> -	}
>>   	return 0;
>>   }
>>   
>> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi)
>>   	set_opt(sbi, MERGE_CHECKPOINT);
>>   	F2FS_OPTION(sbi).unusable_cap = 0;
>>   	sbi->sb->s_flags |= SB_LAZYTIME;
>> -	if (!f2fs_is_readonly(sbi))
>> +	if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi))
>>   		set_opt(sbi, FLUSH_MERGE);
>>   	if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi))
>>   		set_opt(sbi, DISCARD);
>> -- 
>> 2.36.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix to check readonly condition correctly
  2023-04-03 17:59 ` Jaegeuk Kim
  2023-04-04 15:03   ` Chao Yu
@ 2023-04-10  2:19   ` Chao Yu
  2023-04-11 15:58     ` [f2fs-dev] " Chao Yu
  1 sibling, 1 reply; 6+ messages in thread
From: Chao Yu @ 2023-04-10  2:19 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel

On 2023/4/4 1:59, Jaegeuk Kim wrote:
> On 04/02, Chao Yu wrote:
>> This patch does below changes:
>>
>> - If f2fs enables readonly feature or device is readonly, allow to
>> mount readonly mode only
>> - Introduce f2fs_dev_is_readonly() to indicate whether image or device
>> is readonly
>> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
>> and f2fs_convert_inline_inode()
>> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
>> is writable

Jaegeuk,

Please drop this patch, and help to check below one, thanks. :)

f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only()

Thanks,

> 
> What is the problem to solve here?
> 
>>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/checkpoint.c |  2 +-
>>   fs/f2fs/f2fs.h       | 10 +++++-----
>>   fs/f2fs/inline.c     |  3 +--
>>   fs/f2fs/super.c      | 13 +++++++------
>>   4 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 8e1db5752fff..1eef597ed393 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>   	unsigned long long ckpt_ver;
>>   	int err = 0;
>>   
>> -	if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi))
>> +	if (f2fs_readonly(sbi->sb))
>>   		return -EROFS;
>>   
>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 2d4a7ef62537..7de95133478a 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi)
>>   	return false;
>>   }
>>   
>> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi)
>> +{
>> +	return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi);
>> +}
>> +
>>   static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi)
>>   {
>>   	return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS;
>> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs,
>>   	}
>>   }
>>   
>> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi)
>> -{
>> -	return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
>> -}
>> -
>>   #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
>>   #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
>>   
>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>> index 72269e7efd26..2c36f2dc2317 100644
>> --- a/fs/f2fs/inline.c
>> +++ b/fs/f2fs/inline.c
>> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
>>   	struct page *ipage, *page;
>>   	int err = 0;
>>   
>> -	if (!f2fs_has_inline_data(inode) ||
>> -			f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
>> +	if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb))
>>   		return 0;
>>   
>>   	err = f2fs_dquot_initialize(inode);
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index d016f398fcad..db7649010c12 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>   		return -EINVAL;
>>   	}
>>   
>> -	if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) {
>> +	if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
>> +		f2fs_err(sbi, "Allow to mount readonly mode only");
>> +		return -EROFS;
>> +	}
>> +
>> +	if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) {
>>   		f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode");
>>   		return -EINVAL;
>>   	}
>>   
>> -	if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
>> -		f2fs_err(sbi, "Allow to mount readonly mode only");
>> -		return -EROFS;
>> -	}
>>   	return 0;
>>   }
>>   
>> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi)
>>   	set_opt(sbi, MERGE_CHECKPOINT);
>>   	F2FS_OPTION(sbi).unusable_cap = 0;
>>   	sbi->sb->s_flags |= SB_LAZYTIME;
>> -	if (!f2fs_is_readonly(sbi))
>> +	if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi))
>>   		set_opt(sbi, FLUSH_MERGE);
>>   	if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi))
>>   		set_opt(sbi, DISCARD);
>> -- 
>> 2.36.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix to check readonly condition correctly
  2023-04-10  2:19   ` Chao Yu
@ 2023-04-11 15:58     ` Chao Yu
  2023-04-11 16:53       ` Jaegeuk Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2023-04-11 15:58 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2023/4/10 10:19, Chao Yu wrote:
> On 2023/4/4 1:59, Jaegeuk Kim wrote:
>> On 04/02, Chao Yu wrote:
>>> This patch does below changes:
>>>
>>> - If f2fs enables readonly feature or device is readonly, allow to
>>> mount readonly mode only
>>> - Introduce f2fs_dev_is_readonly() to indicate whether image or device
>>> is readonly
>>> - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
>>> and f2fs_convert_inline_inode()
>>> - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
>>> is writable
> 
> Jaegeuk,
> 
> Please drop this patch, and help to check below one, thanks. :)
> 
> f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only()

Oh, I didn't make it clear.

I found you applied both v1 and v2 of this patch.

So I mean how about keeping v2:

[PATCH v2] f2fs: fix to check readonly condition correctly
https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=2bce08d26c0fe3a4ce1cca03ee0ea0c59d98c0b2

and apply

[PATCH] f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only()
https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=7ec21ff2e8f521842938415af741fc72f39c0135

to instead

[PATCH] f2fs: fix to check readonly condition correctly
https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=66eb18a8b892afe085097747521fb280a1577ea4

Thanks,

> 
> Thanks,
> 
>>
>> What is the problem to solve here?
>>
>>>
>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>> ---
>>>    fs/f2fs/checkpoint.c |  2 +-
>>>    fs/f2fs/f2fs.h       | 10 +++++-----
>>>    fs/f2fs/inline.c     |  3 +--
>>>    fs/f2fs/super.c      | 13 +++++++------
>>>    4 files changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index 8e1db5752fff..1eef597ed393 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>    	unsigned long long ckpt_ver;
>>>    	int err = 0;
>>>    
>>> -	if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi))
>>> +	if (f2fs_readonly(sbi->sb))
>>>    		return -EROFS;
>>>    
>>>    	if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 2d4a7ef62537..7de95133478a 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi)
>>>    	return false;
>>>    }
>>>    
>>> +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi)
>>> +{
>>> +	return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi);
>>> +}
>>> +
>>>    static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi)
>>>    {
>>>    	return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS;
>>> @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs,
>>>    	}
>>>    }
>>>    
>>> -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi)
>>> -{
>>> -	return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
>>> -}
>>> -
>>>    #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
>>>    #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
>>>    
>>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>>> index 72269e7efd26..2c36f2dc2317 100644
>>> --- a/fs/f2fs/inline.c
>>> +++ b/fs/f2fs/inline.c
>>> @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
>>>    	struct page *ipage, *page;
>>>    	int err = 0;
>>>    
>>> -	if (!f2fs_has_inline_data(inode) ||
>>> -			f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
>>> +	if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb))
>>>    		return 0;
>>>    
>>>    	err = f2fs_dquot_initialize(inode);
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index d016f398fcad..db7649010c12 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>>    		return -EINVAL;
>>>    	}
>>>    
>>> -	if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) {
>>> +	if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
>>> +		f2fs_err(sbi, "Allow to mount readonly mode only");
>>> +		return -EROFS;
>>> +	}
>>> +
>>> +	if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) {
>>>    		f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode");
>>>    		return -EINVAL;
>>>    	}
>>>    
>>> -	if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
>>> -		f2fs_err(sbi, "Allow to mount readonly mode only");
>>> -		return -EROFS;
>>> -	}
>>>    	return 0;
>>>    }
>>>    
>>> @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi)
>>>    	set_opt(sbi, MERGE_CHECKPOINT);
>>>    	F2FS_OPTION(sbi).unusable_cap = 0;
>>>    	sbi->sb->s_flags |= SB_LAZYTIME;
>>> -	if (!f2fs_is_readonly(sbi))
>>> +	if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi))
>>>    		set_opt(sbi, FLUSH_MERGE);
>>>    	if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi))
>>>    		set_opt(sbi, DISCARD);
>>> -- 
>>> 2.36.1
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix to check readonly condition correctly
  2023-04-11 15:58     ` [f2fs-dev] " Chao Yu
@ 2023-04-11 16:53       ` Jaegeuk Kim
  0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2023-04-11 16:53 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 04/11, Chao Yu wrote:
> On 2023/4/10 10:19, Chao Yu wrote:
> > On 2023/4/4 1:59, Jaegeuk Kim wrote:
> > > On 04/02, Chao Yu wrote:
> > > > This patch does below changes:
> > > > 
> > > > - If f2fs enables readonly feature or device is readonly, allow to
> > > > mount readonly mode only
> > > > - Introduce f2fs_dev_is_readonly() to indicate whether image or device
> > > > is readonly
> > > > - remove unnecessary f2fs_hw_is_readonly() in f2fs_write_checkpoint()
> > > > and f2fs_convert_inline_inode()
> > > > - enable FLUSH_MERGE only if f2fs is mounted as rw and image/device
> > > > is writable
> > 
> > Jaegeuk,
> > 
> > Please drop this patch, and help to check below one, thanks. :)
> > 
> > f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only()
> 
> Oh, I didn't make it clear.
> 
> I found you applied both v1 and v2 of this patch.

Yeah, I was keeping the fixes locally. Please check again.

> 
> So I mean how about keeping v2:
> 
> [PATCH v2] f2fs: fix to check readonly condition correctly
> https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=2bce08d26c0fe3a4ce1cca03ee0ea0c59d98c0b2
> 
> and apply
> 
> [PATCH] f2fs: use f2fs_hw_is_readonly() instead of bdev_read_only()
> https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=7ec21ff2e8f521842938415af741fc72f39c0135
> 
> to instead
> 
> [PATCH] f2fs: fix to check readonly condition correctly
> https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev-test&id=66eb18a8b892afe085097747521fb280a1577ea4
> 
> Thanks,
> 
> > 
> > Thanks,
> > 
> > > 
> > > What is the problem to solve here?
> > > 
> > > > 
> > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > ---
> > > >    fs/f2fs/checkpoint.c |  2 +-
> > > >    fs/f2fs/f2fs.h       | 10 +++++-----
> > > >    fs/f2fs/inline.c     |  3 +--
> > > >    fs/f2fs/super.c      | 13 +++++++------
> > > >    4 files changed, 14 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > > index 8e1db5752fff..1eef597ed393 100644
> > > > --- a/fs/f2fs/checkpoint.c
> > > > +++ b/fs/f2fs/checkpoint.c
> > > > @@ -1604,7 +1604,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> > > >    	unsigned long long ckpt_ver;
> > > >    	int err = 0;
> > > > -	if (f2fs_readonly(sbi->sb) || f2fs_hw_is_readonly(sbi))
> > > > +	if (f2fs_readonly(sbi->sb))
> > > >    		return -EROFS;
> > > >    	if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
> > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > index 2d4a7ef62537..7de95133478a 100644
> > > > --- a/fs/f2fs/f2fs.h
> > > > +++ b/fs/f2fs/f2fs.h
> > > > @@ -4446,6 +4446,11 @@ static inline bool f2fs_hw_is_readonly(struct f2fs_sb_info *sbi)
> > > >    	return false;
> > > >    }
> > > > +static inline bool f2fs_dev_is_readonly(struct f2fs_sb_info *sbi)
> > > > +{
> > > > +	return f2fs_sb_has_readonly(sbi) || f2fs_hw_is_readonly(sbi);
> > > > +}
> > > > +
> > > >    static inline bool f2fs_lfs_mode(struct f2fs_sb_info *sbi)
> > > >    {
> > > >    	return F2FS_OPTION(sbi).fs_mode == FS_MODE_LFS;
> > > > @@ -4546,11 +4551,6 @@ static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs,
> > > >    	}
> > > >    }
> > > > -static inline bool f2fs_is_readonly(struct f2fs_sb_info *sbi)
> > > > -{
> > > > -	return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
> > > > -}
> > > > -
> > > >    #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
> > > >    #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
> > > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > > index 72269e7efd26..2c36f2dc2317 100644
> > > > --- a/fs/f2fs/inline.c
> > > > +++ b/fs/f2fs/inline.c
> > > > @@ -203,8 +203,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
> > > >    	struct page *ipage, *page;
> > > >    	int err = 0;
> > > > -	if (!f2fs_has_inline_data(inode) ||
> > > > -			f2fs_hw_is_readonly(sbi) || f2fs_readonly(sbi->sb))
> > > > +	if (!f2fs_has_inline_data(inode) || f2fs_readonly(sbi->sb))
> > > >    		return 0;
> > > >    	err = f2fs_dquot_initialize(inode);
> > > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > > index d016f398fcad..db7649010c12 100644
> > > > --- a/fs/f2fs/super.c
> > > > +++ b/fs/f2fs/super.c
> > > > @@ -1382,15 +1382,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> > > >    		return -EINVAL;
> > > >    	}
> > > > -	if (f2fs_is_readonly(sbi) && test_opt(sbi, FLUSH_MERGE)) {
> > > > +	if (f2fs_dev_is_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
> > > > +		f2fs_err(sbi, "Allow to mount readonly mode only");
> > > > +		return -EROFS;
> > > > +	}
> > > > +
> > > > +	if (f2fs_readonly(sbi->sb) && test_opt(sbi, FLUSH_MERGE)) {
> > > >    		f2fs_err(sbi, "FLUSH_MERGE not compatible with readonly mode");
> > > >    		return -EINVAL;
> > > >    	}
> > > > -	if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
> > > > -		f2fs_err(sbi, "Allow to mount readonly mode only");
> > > > -		return -EROFS;
> > > > -	}
> > > >    	return 0;
> > > >    }
> > > > @@ -2122,7 +2123,7 @@ static void default_options(struct f2fs_sb_info *sbi)
> > > >    	set_opt(sbi, MERGE_CHECKPOINT);
> > > >    	F2FS_OPTION(sbi).unusable_cap = 0;
> > > >    	sbi->sb->s_flags |= SB_LAZYTIME;
> > > > -	if (!f2fs_is_readonly(sbi))
> > > > +	if (!f2fs_readonly(sbi->sb) && !f2fs_dev_is_readonly(sbi))
> > > >    		set_opt(sbi, FLUSH_MERGE);
> > > >    	if (f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi))
> > > >    		set_opt(sbi, DISCARD);
> > > > -- 
> > > > 2.36.1
> > 
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-11 16:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-02 11:28 [PATCH] f2fs: fix to check readonly condition correctly Chao Yu
2023-04-03 17:59 ` Jaegeuk Kim
2023-04-04 15:03   ` Chao Yu
2023-04-10  2:19   ` Chao Yu
2023-04-11 15:58     ` [f2fs-dev] " Chao Yu
2023-04-11 16:53       ` Jaegeuk Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).