Message ID | 20230402112825.42486-1-chao@kernel.org (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [f2fs-dev] f2fs: fix to check readonly condition correctly | expand |
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
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
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
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
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
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);
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(-)