* [f2fs-dev] [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-kernel, linux-f2fs-devel
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
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix to check readonly condition correctly
2023-04-02 11:28 [f2fs-dev] [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-kernel, linux-f2fs-devel
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
_______________________________________________
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-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-kernel, linux-f2fs-devel
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
_______________________________________________
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-03 17:59 ` Jaegeuk Kim
2023-04-04 15:03 ` Chao Yu
@ 2023-04-10 2:19 ` Chao Yu
2023-04-11 15:58 ` 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-kernel, linux-f2fs-devel
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
_______________________________________________
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-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
_______________________________________________
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 ` 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
_______________________________________________
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 [f2fs-dev] [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 ` 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).