2021-05-21 20:27:34

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: immutable file can have null address in compressed chunk

If we released compressed blocks having an immutable bit, we can see less
number of compressed block addresses. Let's fix wrong BUG_ON.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/compress.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d4f7371fb0d8..1189740aa141 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -927,7 +927,8 @@ static int __f2fs_cluster_blocks(struct inode *inode,
}
}

- f2fs_bug_on(F2FS_I_SB(inode), !compr && ret != cluster_size);
+ f2fs_bug_on(F2FS_I_SB(inode),
+ !compr && ret != cluster_size && !IS_IMMUTABLE(inode));
}
fail:
f2fs_put_dnode(&dn);
--
2.31.1.818.g46aad6cb9e-goog


2021-05-21 20:28:40

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 2/2] f2fs: support RO feature

Given RO feature in superblock, we don't need to check provisioning/reserve
spaces and SSA area.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/segment.c | 3 +++
fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c0bead0df66a..2c6913261586 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -168,6 +168,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_FEATURE_CASEFOLD 0x1000
#define F2FS_FEATURE_COMPRESSION 0x2000
+#define F2FS_FEATURE_RO 0x4000

#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
#define NR_CURSEG_DATA_TYPE (3)
#define NR_CURSEG_NODE_TYPE (3)
#define NR_CURSEG_INMEM_TYPE (2)
+#define NR_CURSEG_RO_TYPE (2)
#define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
#define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 8668df7870d0..67cec8f858a2 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
{
int i;

+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
+ return 0;
+
/*
* In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
* In LFS curseg, all blkaddr after .next_blkoff should be unused.
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index b29de80ab60e..312bfab54693 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
static void default_options(struct f2fs_sb_info *sbi)
{
/* init some FS parameters */
- F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
+ else
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+
F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
@@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
err = parse_options(sb, data, true);
if (err)
goto restore_opts;
+
+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
+ !(*flags & SB_RDONLY))
+ goto restore_opts;
+
checkpoint_changed =
disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);

@@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);

+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
+ goto no_reserved;
if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
ovp_segments == 0 || reserved_segments == 0)) {
f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
return 1;
}
-
+no_reserved:
user_block_count = le64_to_cpu(ckpt->user_block_count);
segment_count_main = le32_to_cpu(raw_super->segment_count_main);
log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
- if (!user_block_count || user_block_count >=
+ if (!user_block_count || user_block_count >
segment_count_main << log_blocks_per_seg) {
f2fs_err(sbi, "Wrong user_block_count: %u",
user_block_count);
@@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
+ goto check_data;
+
for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
le32_to_cpu(ckpt->cur_node_segno[j])) {
@@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
+check_data:
for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
+ goto skip_cross;
+
for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
le32_to_cpu(ckpt->cur_data_segno[j])) {
@@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
-
+skip_cross:
sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);

@@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
if (err)
goto free_options;

+ if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
+ !f2fs_readonly(sbi->sb)) {
+ f2fs_info(sbi, "Allow to mount readonly mode only");
+ err = -EINVAL;
+ goto free_options;
+ }
+
sb->s_maxbytes = max_file_blocks(NULL) <<
le32_to_cpu(raw_super->log_blocksize);
sb->s_max_links = F2FS_LINK_MAX;
--
2.31.1.818.g46aad6cb9e-goog

2021-05-24 11:54:52

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support RO feature

On 2021/5/22 3:02, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.

Cool, any solution to update files of ro f2fs image if there is such
scenario?

>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 3 +++
> fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
> 3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c0bead0df66a..2c6913261586 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
> #define F2FS_FEATURE_CASEFOLD 0x1000
> #define F2FS_FEATURE_COMPRESSION 0x2000
> +#define F2FS_FEATURE_RO 0x4000
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> ((raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_DATA_TYPE (3)
> #define NR_CURSEG_NODE_TYPE (3)
> #define NR_CURSEG_INMEM_TYPE (2)
> +#define NR_CURSEG_RO_TYPE (2)
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 8668df7870d0..67cec8f858a2 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> {
> int i;
>
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))

Why not using F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))?

Ditto for all below __F2FS_HAS_FEATURE().

Thanks,

> + return 0;
> +
> /*
> * In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
> * In LFS curseg, all blkaddr after .next_blkoff should be unused.
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index b29de80ab60e..312bfab54693 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> static void default_options(struct f2fs_sb_info *sbi)
> {
> /* init some FS parameters */
> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> + else
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> +
> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> @@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> err = parse_options(sb, data, true);
> if (err)
> goto restore_opts;
> +
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> + !(*flags & SB_RDONLY))
> + goto restore_opts;
> +
> checkpoint_changed =
> disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);
>
> @@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + goto no_reserved;
> if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> ovp_segments == 0 || reserved_segments == 0)) {
> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> return 1;
> }
> -
> +no_reserved:
> user_block_count = le64_to_cpu(ckpt->user_block_count);
> segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> - if (!user_block_count || user_block_count >=
> + if (!user_block_count || user_block_count >
> segment_count_main << log_blocks_per_seg) {
> f2fs_err(sbi, "Wrong user_block_count: %u",
> user_block_count);
> @@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + goto check_data;
> +
> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> le32_to_cpu(ckpt->cur_node_segno[j])) {
> @@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> +check_data:
> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + goto skip_cross;
> +
> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> le32_to_cpu(ckpt->cur_data_segno[j])) {
> @@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> -
> +skip_cross:
> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>
> @@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> if (err)
> goto free_options;
>
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> + !f2fs_readonly(sbi->sb)) {
> + f2fs_info(sbi, "Allow to mount readonly mode only");
> + err = -EINVAL;
> + goto free_options;
> + }
> +
> sb->s_maxbytes = max_file_blocks(NULL) <<
> le32_to_cpu(raw_super->log_blocksize);
> sb->s_max_links = F2FS_LINK_MAX;
>

2021-05-24 17:09:01

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support RO feature

On 05/24, Chao Yu wrote:
> On 2021/5/22 3:02, Jaegeuk Kim wrote:
> > Given RO feature in superblock, we don't need to check provisioning/reserve
> > spaces and SSA area.
>
> Cool, any solution to update files of ro f2fs image if there is such
> scenario?

Hmm, overlayfs?

>
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/f2fs.h | 2 ++
> > fs/f2fs/segment.c | 3 +++
> > fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
> > 3 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index c0bead0df66a..2c6913261586 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> > #define F2FS_FEATURE_SB_CHKSUM 0x0800
> > #define F2FS_FEATURE_CASEFOLD 0x1000
> > #define F2FS_FEATURE_COMPRESSION 0x2000
> > +#define F2FS_FEATURE_RO 0x4000
> > #define __F2FS_HAS_FEATURE(raw_super, mask) \
> > ((raw_super->feature & cpu_to_le32(mask)) != 0)
> > @@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> > #define NR_CURSEG_DATA_TYPE (3)
> > #define NR_CURSEG_NODE_TYPE (3)
> > #define NR_CURSEG_INMEM_TYPE (2)
> > +#define NR_CURSEG_RO_TYPE (2)
> > #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> > #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 8668df7870d0..67cec8f858a2 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> > {
> > int i;
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>
> Why not using F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))?

Oh, updated.

>
> Ditto for all below __F2FS_HAS_FEATURE().
>
> Thanks,
>
> > + return 0;
> > +
> > /*
> > * In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
> > * In LFS curseg, all blkaddr after .next_blkoff should be unused.
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index b29de80ab60e..312bfab54693 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> > static void default_options(struct f2fs_sb_info *sbi)
> > {
> > /* init some FS parameters */
> > - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> > + else
> > + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> > +
> > F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> > F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> > F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> > @@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> > err = parse_options(sb, data, true);
> > if (err)
> > goto restore_opts;
> > +
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> > + !(*flags & SB_RDONLY))
> > + goto restore_opts;
> > +
> > checkpoint_changed =
> > disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);
> > @@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> > reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + goto no_reserved;
> > if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> > ovp_segments == 0 || reserved_segments == 0)) {
> > f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> > return 1;
> > }
> > -
> > +no_reserved:
> > user_block_count = le64_to_cpu(ckpt->user_block_count);
> > segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> > log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> > - if (!user_block_count || user_block_count >=
> > + if (!user_block_count || user_block_count >
> > segment_count_main << log_blocks_per_seg) {
> > f2fs_err(sbi, "Wrong user_block_count: %u",
> > user_block_count);
> > @@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> > le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> > return 1;
> > +
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + goto check_data;
> > +
> > for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> > if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> > le32_to_cpu(ckpt->cur_node_segno[j])) {
> > @@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > }
> > }
> > }
> > +check_data:
> > for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> > if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> > le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> > return 1;
> > +
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + goto skip_cross;
> > +
> > for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> > if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> > le32_to_cpu(ckpt->cur_data_segno[j])) {
> > @@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > }
> > }
> > }
> > -
> > +skip_cross:
> > sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> > nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
> > @@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> > if (err)
> > goto free_options;
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> > + !f2fs_readonly(sbi->sb)) {
> > + f2fs_info(sbi, "Allow to mount readonly mode only");
> > + err = -EINVAL;
> > + goto free_options;
> > + }
> > +
> > sb->s_maxbytes = max_file_blocks(NULL) <<
> > le32_to_cpu(raw_super->log_blocksize);
> > sb->s_max_links = F2FS_LINK_MAX;
> >

2021-05-25 02:32:48

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support RO feature

On 2021/5/25 1:07, Jaegeuk Kim wrote:
> On 05/24, Chao Yu wrote:
>> On 2021/5/22 3:02, Jaegeuk Kim wrote:
>>> Given RO feature in superblock, we don't need to check provisioning/reserve
>>> spaces and SSA area.
>>
>> Cool, any solution to update files of ro f2fs image if there is such
>> scenario?
>
> Hmm, overlayfs?

I mean using f2fs-tools, maybe...

Thanks,

>
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <[email protected]>
>>> ---
>>> fs/f2fs/f2fs.h | 2 ++
>>> fs/f2fs/segment.c | 3 +++
>>> fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
>>> 3 files changed, 36 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index c0bead0df66a..2c6913261586 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
>>> #define F2FS_FEATURE_SB_CHKSUM 0x0800
>>> #define F2FS_FEATURE_CASEFOLD 0x1000
>>> #define F2FS_FEATURE_COMPRESSION 0x2000
>>> +#define F2FS_FEATURE_RO 0x4000
>>> #define __F2FS_HAS_FEATURE(raw_super, mask) \
>>> ((raw_super->feature & cpu_to_le32(mask)) != 0)
>>> @@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
>>> #define NR_CURSEG_DATA_TYPE (3)
>>> #define NR_CURSEG_NODE_TYPE (3)
>>> #define NR_CURSEG_INMEM_TYPE (2)
>>> +#define NR_CURSEG_RO_TYPE (2)
>>> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
>>> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 8668df7870d0..67cec8f858a2 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
>>> {
>>> int i;
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>
>> Why not using F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))?
>
> Oh, updated.
>
>>
>> Ditto for all below __F2FS_HAS_FEATURE().
>>
>> Thanks,
>>
>>> + return 0;
>>> +
>>> /*
>>> * In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
>>> * In LFS curseg, all blkaddr after .next_blkoff should be unused.
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index b29de80ab60e..312bfab54693 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
>>> static void default_options(struct f2fs_sb_info *sbi)
>>> {
>>> /* init some FS parameters */
>>> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
>>> + else
>>> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
>>> +
>>> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
>>> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
>>> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
>>> @@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>>> err = parse_options(sb, data, true);
>>> if (err)
>>> goto restore_opts;
>>> +
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
>>> + !(*flags & SB_RDONLY))
>>> + goto restore_opts;
>>> +
>>> checkpoint_changed =
>>> disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);
>>> @@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
>>> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + goto no_reserved;
>>> if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
>>> ovp_segments == 0 || reserved_segments == 0)) {
>>> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
>>> return 1;
>>> }
>>> -
>>> +no_reserved:
>>> user_block_count = le64_to_cpu(ckpt->user_block_count);
>>> segment_count_main = le32_to_cpu(raw_super->segment_count_main);
>>> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
>>> - if (!user_block_count || user_block_count >=
>>> + if (!user_block_count || user_block_count >
>>> segment_count_main << log_blocks_per_seg) {
>>> f2fs_err(sbi, "Wrong user_block_count: %u",
>>> user_block_count);
>>> @@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
>>> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
>>> return 1;
>>> +
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + goto check_data;
>>> +
>>> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
>>> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
>>> le32_to_cpu(ckpt->cur_node_segno[j])) {
>>> @@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> }
>>> }
>>> }
>>> +check_data:
>>> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
>>> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
>>> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
>>> return 1;
>>> +
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + goto skip_cross;
>>> +
>>> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
>>> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
>>> le32_to_cpu(ckpt->cur_data_segno[j])) {
>>> @@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> }
>>> }
>>> }
>>> -
>>> +skip_cross:
>>> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
>>> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>>> @@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>> if (err)
>>> goto free_options;
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
>>> + !f2fs_readonly(sbi->sb)) {
>>> + f2fs_info(sbi, "Allow to mount readonly mode only");
>>> + err = -EINVAL;
>>> + goto free_options;
>>> + }
>>> +
>>> sb->s_maxbytes = max_file_blocks(NULL) <<
>>> le32_to_cpu(raw_super->log_blocksize);
>>> sb->s_max_links = F2FS_LINK_MAX;
>>>
> .
>

2021-05-26 09:12:55

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: immutable file can have null address in compressed chunk

On 2021/5/22 3:02, Jaegeuk Kim wrote:
> If we released compressed blocks having an immutable bit, we can see less
> number of compressed block addresses. Let's fix wrong BUG_ON.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>

It looks it will be better to merge this into
("f2fs: compress: remove unneeded preallocation")?

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2021-05-26 09:44:21

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support RO feature

On 2021/5/22 3:02, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 3 +++
> fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
> 3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c0bead0df66a..2c6913261586 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
> #define F2FS_FEATURE_CASEFOLD 0x1000
> #define F2FS_FEATURE_COMPRESSION 0x2000
> +#define F2FS_FEATURE_RO 0x4000
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> ((raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_DATA_TYPE (3)
> #define NR_CURSEG_NODE_TYPE (3)
> #define NR_CURSEG_INMEM_TYPE (2)
> +#define NR_CURSEG_RO_TYPE (2)
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 8668df7870d0..67cec8f858a2 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> {
> int i;
>
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + return 0;

We need to skip this sanity check because .next_blkoff may point to end position
of image?

> +
> /*
> * In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
> * In LFS curseg, all blkaddr after .next_blkoff should be unused.
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index b29de80ab60e..312bfab54693 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> static void default_options(struct f2fs_sb_info *sbi)
> {
> /* init some FS parameters */
> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> + else
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> +
> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> @@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> err = parse_options(sb, data, true);
> if (err)
> goto restore_opts;
> +
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> + !(*flags & SB_RDONLY))

err = -EROFS; ?

> + goto restore_opts;
> +
> checkpoint_changed =
> disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);
>
> @@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + goto no_reserved;
> if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> ovp_segments == 0 || reserved_segments == 0)) {
> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> return 1;
> }

Well, why not:

if (!F2FS_HAS_FEATURE(, RO) && unlikely()) {
}

> -
> +no_reserved:
> user_block_count = le64_to_cpu(ckpt->user_block_count);
> segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> - if (!user_block_count || user_block_count >=
> + if (!user_block_count || user_block_count >

Does this change related to RO feature? if so, let's split condition for
RO feature here?

> segment_count_main << log_blocks_per_seg) {
> f2fs_err(sbi, "Wrong user_block_count: %u",
> user_block_count);
> @@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + goto check_data;
> +
> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> le32_to_cpu(ckpt->cur_node_segno[j])) {
> @@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> +check_data:
> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> + goto skip_cross;
> +
> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> le32_to_cpu(ckpt->cur_data_segno[j])) {
> @@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> -
> +skip_cross:
> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>
> @@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> if (err)
> goto free_options;
>
> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> + !f2fs_readonly(sbi->sb)) {
> + f2fs_info(sbi, "Allow to mount readonly mode only");
> + err = -EINVAL;
> + goto free_options;
> + }

How about relocating this to parse_options() like other features did, then
we don't need to handle this in both fill_super() and remount()?

Thanks,

> +
> sb->s_maxbytes = max_file_blocks(NULL) <<
> le32_to_cpu(raw_super->log_blocksize);
> sb->s_max_links = F2FS_LINK_MAX;
>

2021-05-26 14:02:20

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support RO feature

On 05/26, Chao Yu wrote:
> On 2021/5/22 3:02, Jaegeuk Kim wrote:
> > Given RO feature in superblock, we don't need to check provisioning/reserve
> > spaces and SSA area.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/f2fs.h | 2 ++
> > fs/f2fs/segment.c | 3 +++
> > fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
> > 3 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index c0bead0df66a..2c6913261586 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> > #define F2FS_FEATURE_SB_CHKSUM 0x0800
> > #define F2FS_FEATURE_CASEFOLD 0x1000
> > #define F2FS_FEATURE_COMPRESSION 0x2000
> > +#define F2FS_FEATURE_RO 0x4000
> > #define __F2FS_HAS_FEATURE(raw_super, mask) \
> > ((raw_super->feature & cpu_to_le32(mask)) != 0)
> > @@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> > #define NR_CURSEG_DATA_TYPE (3)
> > #define NR_CURSEG_NODE_TYPE (3)
> > #define NR_CURSEG_INMEM_TYPE (2)
> > +#define NR_CURSEG_RO_TYPE (2)
> > #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> > #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 8668df7870d0..67cec8f858a2 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> > {
> > int i;
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + return 0;
>
> We need to skip this sanity check because .next_blkoff may point to end position
> of image?

Since we only use hot data and node. Let me check them only.

>
> > +
> > /*
> > * In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
> > * In LFS curseg, all blkaddr after .next_blkoff should be unused.
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index b29de80ab60e..312bfab54693 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> > static void default_options(struct f2fs_sb_info *sbi)
> > {
> > /* init some FS parameters */
> > - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> > + else
> > + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> > +
> > F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> > F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> > F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> > @@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> > err = parse_options(sb, data, true);
> > if (err)
> > goto restore_opts;
> > +
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> > + !(*flags & SB_RDONLY))
>
> err = -EROFS; ?

Done.

>
> > + goto restore_opts;
> > +
> > checkpoint_changed =
> > disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);
> > @@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> > reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + goto no_reserved;
> > if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> > ovp_segments == 0 || reserved_segments == 0)) {
> > f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> > return 1;
> > }
>
> Well, why not:
>
> if (!F2FS_HAS_FEATURE(, RO) && unlikely()) {
> }

Done.

>
> > -
> > +no_reserved:
> > user_block_count = le64_to_cpu(ckpt->user_block_count);
> > segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> > log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> > - if (!user_block_count || user_block_count >=
> > + if (!user_block_count || user_block_count >
>
> Does this change related to RO feature? if so, let's split condition for
> RO feature here?

Done.

>
> > segment_count_main << log_blocks_per_seg) {
> > f2fs_err(sbi, "Wrong user_block_count: %u",
> > user_block_count);
> > @@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> > le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> > return 1;
> > +
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + goto check_data;
> > +
> > for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> > if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> > le32_to_cpu(ckpt->cur_node_segno[j])) {
> > @@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > }
> > }
> > }
> > +check_data:
> > for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> > if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> > le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> > return 1;
> > +
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
> > + goto skip_cross;
> > +
> > for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> > if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> > le32_to_cpu(ckpt->cur_data_segno[j])) {
> > @@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > }
> > }
> > }
> > -
> > +skip_cross:
> > sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> > nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
> > @@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> > if (err)
> > goto free_options;
> > + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
> > + !f2fs_readonly(sbi->sb)) {
> > + f2fs_info(sbi, "Allow to mount readonly mode only");
> > + err = -EINVAL;
> > + goto free_options;
> > + }
>
> How about relocating this to parse_options() like other features did, then
> we don't need to handle this in both fill_super() and remount()?

Done. But not sure what you mean "we don't need to handle both".

Thanks,

>
> Thanks,
>
> > +
> > sb->s_maxbytes = max_file_blocks(NULL) <<
> > le32_to_cpu(raw_super->log_blocksize);
> > sb->s_max_links = F2FS_LINK_MAX;
> >

2021-05-26 14:06:26

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: immutable file can have null address in compressed chunk

On 05/26, Chao Yu wrote:
> On 2021/5/22 3:02, Jaegeuk Kim wrote:
> > If we released compressed blocks having an immutable bit, we can see less
> > number of compressed block addresses. Let's fix wrong BUG_ON.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
>
> It looks it will be better to merge this into
> ("f2fs: compress: remove unneeded preallocation")?

Right, fixed there. :)

>
> Reviewed-by: Chao Yu <[email protected]>
>
> Thanks,

2021-05-26 18:47:52

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] f2fs: support RO feature

Given RO feature in superblock, we don't need to check provisioning/reserve
spaces and SSA area.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/segment.c | 4 ++++
fs/f2fs/super.c | 35 ++++++++++++++++++++++++++++++-----
3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eaf57b5f3c4b..9ad502f92529 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -168,6 +168,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_FEATURE_CASEFOLD 0x1000
#define F2FS_FEATURE_COMPRESSION 0x2000
+#define F2FS_FEATURE_RO 0x4000

#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
#define NR_CURSEG_DATA_TYPE (3)
#define NR_CURSEG_NODE_TYPE (3)
#define NR_CURSEG_INMEM_TYPE (2)
+#define NR_CURSEG_RO_TYPE (2)
#define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
#define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 8668df7870d0..02e0c38be7eb 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
unsigned int blkofs = curseg->next_blkoff;

+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
+ i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
+ continue;
+
sanity_check_seg_type(sbi, curseg->seg_type);

if (f2fs_test_bit(blkofs, se->cur_valid_map))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e70aca8f97bd..7769ed789b38 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1162,6 +1162,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
*/
if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
+ f2fs_info(sbi, "Allow to mount readonly mode only");
+ err = -EINVAL;
+ goto free_options;
+ }
return 0;
}

@@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
static void default_options(struct f2fs_sb_info *sbi)
{
/* init some FS parameters */
- F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
+ else
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+
F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
@@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
goto skip;

+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
+ err = -EROFS;
+ goto restore_opts;
+ }
+
#ifdef CONFIG_QUOTA
if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
err = dquot_suspend(sb, -1);
@@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);

- if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
+ if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
+ unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
ovp_segments == 0 || reserved_segments == 0)) {
f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
return 1;
}
-
user_block_count = le64_to_cpu(ckpt->user_block_count);
- segment_count_main = le32_to_cpu(raw_super->segment_count_main);
+ segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
+ F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0;
log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
if (!user_block_count || user_block_count >=
segment_count_main << log_blocks_per_seg) {
@@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ goto check_data;
+
for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
le32_to_cpu(ckpt->cur_node_segno[j])) {
@@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
+check_data:
for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ goto skip_cross;
+
for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
le32_to_cpu(ckpt->cur_data_segno[j])) {
@@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
-
+skip_cross:
sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);

--
2.32.0.rc0.204.g9fa02ecfa5-goog

2021-05-26 18:48:11

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v2] f2fs: support RO feature

On 05/26, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 4 ++++
> fs/f2fs/super.c | 35 ++++++++++++++++++++++++++++++-----
> 3 files changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index eaf57b5f3c4b..9ad502f92529 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
> #define F2FS_FEATURE_CASEFOLD 0x1000
> #define F2FS_FEATURE_COMPRESSION 0x2000
> +#define F2FS_FEATURE_RO 0x4000
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> ((raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_DATA_TYPE (3)
> #define NR_CURSEG_NODE_TYPE (3)
> #define NR_CURSEG_INMEM_TYPE (2)
> +#define NR_CURSEG_RO_TYPE (2)
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 8668df7870d0..02e0c38be7eb 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
> unsigned int blkofs = curseg->next_blkoff;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
> + continue;
> +
> sanity_check_seg_type(sbi, curseg->seg_type);
>
> if (f2fs_test_bit(blkofs, se->cur_valid_map))
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index e70aca8f97bd..7769ed789b38 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1162,6 +1162,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> */
> if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
> + f2fs_info(sbi, "Allow to mount readonly mode only");
> + err = -EINVAL;
> + goto free_options;

Fixed:
if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
f2fs_err(sbi, "Allow to mount readonly mode only");
return EINVAL;
}

> + }
> return 0;
> }
>
> @@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> static void default_options(struct f2fs_sb_info *sbi)
> {
> /* init some FS parameters */
> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> + else
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> +
> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> @@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
> goto skip;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
> + err = -EROFS;
> + goto restore_opts;
> + }
> +
> #ifdef CONFIG_QUOTA
> if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
> err = dquot_suspend(sb, -1);
> @@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>
> - if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> + if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> ovp_segments == 0 || reserved_segments == 0)) {
> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> return 1;
> }
> -
> user_block_count = le64_to_cpu(ckpt->user_block_count);
> - segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> + segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
> + F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0;
> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> if (!user_block_count || user_block_count >=
> segment_count_main << log_blocks_per_seg) {
> @@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto check_data;
> +
> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> le32_to_cpu(ckpt->cur_node_segno[j])) {
> @@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> +check_data:
> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto skip_cross;
> +
> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> le32_to_cpu(ckpt->cur_data_segno[j])) {
> @@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> -
> +skip_cross:
> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2021-05-26 18:53:30

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v3] f2fs: support RO feature

Given RO feature in superblock, we don't need to check provisioning/reserve
spaces and SSA area.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
Change log from v2:
- allow curseg updates
- fix some bugs

fs/f2fs/f2fs.h | 2 ++
fs/f2fs/segment.c | 4 ++++
fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++------
3 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eaf57b5f3c4b..9ad502f92529 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -168,6 +168,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_FEATURE_CASEFOLD 0x1000
#define F2FS_FEATURE_COMPRESSION 0x2000
+#define F2FS_FEATURE_RO 0x4000

#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
#define NR_CURSEG_DATA_TYPE (3)
#define NR_CURSEG_NODE_TYPE (3)
#define NR_CURSEG_INMEM_TYPE (2)
+#define NR_CURSEG_RO_TYPE (2)
#define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
#define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 8668df7870d0..02e0c38be7eb 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
unsigned int blkofs = curseg->next_blkoff;

+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
+ i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
+ continue;
+
sanity_check_seg_type(sbi, curseg->seg_type);

if (f2fs_test_bit(blkofs, se->cur_valid_map))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e70aca8f97bd..6788e7b71e27 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -555,7 +555,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
int ret;

if (!options)
- return 0;
+ goto default_check;

while ((p = strsep(&options, ",")) != NULL) {
int token;
@@ -1090,6 +1090,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
return -EINVAL;
}
}
+default_check:
#ifdef CONFIG_QUOTA
if (f2fs_check_quota_options(sbi))
return -EINVAL;
@@ -1162,6 +1163,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
*/
if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
+ f2fs_err(sbi, "Allow to mount readonly mode only");
+ return -EROFS;
+ }
return 0;
}

@@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
static void default_options(struct f2fs_sb_info *sbi)
{
/* init some FS parameters */
- F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
+ else
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+
F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
@@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
goto skip;

+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
+ err = -EROFS;
+ goto restore_opts;
+ }
+
#ifdef CONFIG_QUOTA
if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
err = dquot_suspend(sb, -1);
@@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);

- if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
+ if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
+ unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
ovp_segments == 0 || reserved_segments == 0)) {
f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
return 1;
}
-
user_block_count = le64_to_cpu(ckpt->user_block_count);
- segment_count_main = le32_to_cpu(raw_super->segment_count_main);
+ segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
+ (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0);
log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
if (!user_block_count || user_block_count >=
segment_count_main << log_blocks_per_seg) {
@@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ goto check_data;
+
for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
le32_to_cpu(ckpt->cur_node_segno[j])) {
@@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
+check_data:
for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ goto skip_cross;
+
for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
le32_to_cpu(ckpt->cur_data_segno[j])) {
@@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
-
+skip_cross:
sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);

--
2.32.0.rc0.204.g9fa02ecfa5-goog

2021-05-26 22:23:30

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support RO feature

On 2021/5/26 21:58, Jaegeuk Kim wrote:
> On 05/26, Chao Yu wrote:
>> On 2021/5/22 3:02, Jaegeuk Kim wrote:
>>> Given RO feature in superblock, we don't need to check provisioning/reserve
>>> spaces and SSA area.
>>>
>>> Signed-off-by: Jaegeuk Kim <[email protected]>
>>> ---
>>> fs/f2fs/f2fs.h | 2 ++
>>> fs/f2fs/segment.c | 3 +++
>>> fs/f2fs/super.c | 35 +++++++++++++++++++++++++++++++----
>>> 3 files changed, 36 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index c0bead0df66a..2c6913261586 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
>>> #define F2FS_FEATURE_SB_CHKSUM 0x0800
>>> #define F2FS_FEATURE_CASEFOLD 0x1000
>>> #define F2FS_FEATURE_COMPRESSION 0x2000
>>> +#define F2FS_FEATURE_RO 0x4000
>>> #define __F2FS_HAS_FEATURE(raw_super, mask) \
>>> ((raw_super->feature & cpu_to_le32(mask)) != 0)
>>> @@ -939,6 +940,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
>>> #define NR_CURSEG_DATA_TYPE (3)
>>> #define NR_CURSEG_NODE_TYPE (3)
>>> #define NR_CURSEG_INMEM_TYPE (2)
>>> +#define NR_CURSEG_RO_TYPE (2)
>>> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
>>> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 8668df7870d0..67cec8f858a2 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -4674,6 +4674,9 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
>>> {
>>> int i;
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + return 0;
>>
>> We need to skip this sanity check because .next_blkoff may point to end position
>> of image?
>
> Since we only use hot data and node. Let me check them only.
>
>>
>>> +
>>> /*
>>> * In LFS/SSR curseg, .next_blkoff should point to an unused blkaddr;
>>> * In LFS curseg, all blkaddr after .next_blkoff should be unused.
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index b29de80ab60e..312bfab54693 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -1819,7 +1819,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
>>> static void default_options(struct f2fs_sb_info *sbi)
>>> {
>>> /* init some FS parameters */
>>> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
>>> + else
>>> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
>>> +
>>> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
>>> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
>>> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
>>> @@ -1994,6 +1998,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>>> err = parse_options(sb, data, true);
>>> if (err)
>>> goto restore_opts;
>>> +
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
>>> + !(*flags & SB_RDONLY))
>>
>> err = -EROFS; ?
>
> Done.
>
>>
>>> + goto restore_opts;
>>> +
>>> checkpoint_changed =
>>> disable_checkpoint != test_opt(sbi, DISABLE_CHECKPOINT);
>>> @@ -3137,16 +3146,18 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
>>> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + goto no_reserved;
>>> if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
>>> ovp_segments == 0 || reserved_segments == 0)) {
>>> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
>>> return 1;
>>> }
>>
>> Well, why not:
>>
>> if (!F2FS_HAS_FEATURE(, RO) && unlikely()) {
>> }
>
> Done.
>
>>
>>> -
>>> +no_reserved:
>>> user_block_count = le64_to_cpu(ckpt->user_block_count);
>>> segment_count_main = le32_to_cpu(raw_super->segment_count_main);
>>> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
>>> - if (!user_block_count || user_block_count >=
>>> + if (!user_block_count || user_block_count >
>>
>> Does this change related to RO feature? if so, let's split condition for
>> RO feature here?
>
> Done.
>
>>
>>> segment_count_main << log_blocks_per_seg) {
>>> f2fs_err(sbi, "Wrong user_block_count: %u",
>>> user_block_count);
>>> @@ -3175,6 +3186,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
>>> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
>>> return 1;
>>> +
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + goto check_data;
>>> +
>>> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
>>> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
>>> le32_to_cpu(ckpt->cur_node_segno[j])) {
>>> @@ -3185,10 +3200,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> }
>>> }
>>> }
>>> +check_data:
>>> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
>>> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
>>> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
>>> return 1;
>>> +
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO))
>>> + goto skip_cross;
>>> +
>>> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
>>> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
>>> le32_to_cpu(ckpt->cur_data_segno[j])) {
>>> @@ -3210,7 +3230,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> }
>>> }
>>> }
>>> -
>>> +skip_cross:
>>> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
>>> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>>> @@ -3703,6 +3723,13 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>> if (err)
>>> goto free_options;
>>> + if (__F2FS_HAS_FEATURE(sbi->raw_super, F2FS_FEATURE_RO) &&
>>> + !f2fs_readonly(sbi->sb)) {
>>> + f2fs_info(sbi, "Allow to mount readonly mode only");
>>> + err = -EINVAL;
>>> + goto free_options;
>>> + }
>>
>> How about relocating this to parse_options() like other features did, then
>> we don't need to handle this in both fill_super() and remount()?
>
> Done. But not sure what you mean "we don't need to handle both".

Oh, I mean both fill_super() and remount() will call parse_options(),
so if we want to avoid mounting ro image with rw mountoption, we just
need to add one condition check in parse_option().

Thanks,

>
> Thanks,
>
>>
>> Thanks,
>>
>>> +
>>> sb->s_maxbytes = max_file_blocks(NULL) <<
>>> le32_to_cpu(raw_super->log_blocksize);
>>> sb->s_max_links = F2FS_LINK_MAX;
>>>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>

2021-05-27 04:57:29

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v3] f2fs: support RO feature

On 2021/5/26 22:43, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> Change log from v2:
> - allow curseg updates
> - fix some bugs
>
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 4 ++++
> fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++------
> 3 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index eaf57b5f3c4b..9ad502f92529 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
> #define F2FS_FEATURE_CASEFOLD 0x1000
> #define F2FS_FEATURE_COMPRESSION 0x2000
> +#define F2FS_FEATURE_RO 0x4000
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> ((raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_DATA_TYPE (3)
> #define NR_CURSEG_NODE_TYPE (3)
> #define NR_CURSEG_INMEM_TYPE (2)
> +#define NR_CURSEG_RO_TYPE (2)
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 8668df7870d0..02e0c38be7eb 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
> unsigned int blkofs = curseg->next_blkoff;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
> + continue;
> +
> sanity_check_seg_type(sbi, curseg->seg_type);
>
> if (f2fs_test_bit(blkofs, se->cur_valid_map))
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index e70aca8f97bd..6788e7b71e27 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -555,7 +555,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> int ret;
>
> if (!options)
> - return 0;
> + goto default_check;
>
> while ((p = strsep(&options, ",")) != NULL) {
> int token;
> @@ -1090,6 +1090,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> return -EINVAL;
> }
> }
> +default_check:
> #ifdef CONFIG_QUOTA
> if (f2fs_check_quota_options(sbi))
> return -EINVAL;
> @@ -1162,6 +1163,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> */
> if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
> + f2fs_err(sbi, "Allow to mount readonly mode only");
> + return -EROFS;
> + }
> return 0;
> }
>
> @@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> static void default_options(struct f2fs_sb_info *sbi)
> {
> /* init some FS parameters */
> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> + else
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> +
> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> @@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
> goto skip;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
> + err = -EROFS;
> + goto restore_opts;
> + }

remount() -> parse_options() will fail due to below check, so it doesn't need
to check again? Am I missing something?

@@ -1162,6 +1163,11 @@

static int parse_options(struct super_block *sb, char *options, bool is_remount)

*/
if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
+ f2fs_err(sbi, "Allow to mount readonly mode only");
+ return -EROFS;
+ }
return 0;
}

Thanks,

> +
> #ifdef CONFIG_QUOTA
> if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
> err = dquot_suspend(sb, -1);
> @@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>
> - if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> + if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> ovp_segments == 0 || reserved_segments == 0)) {
> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> return 1;
> }
> -
> user_block_count = le64_to_cpu(ckpt->user_block_count);
> - segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> + segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
> + (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0);
> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> if (!user_block_count || user_block_count >=
> segment_count_main << log_blocks_per_seg) {
> @@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto check_data;
> +
> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> le32_to_cpu(ckpt->cur_node_segno[j])) {
> @@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> +check_data:
> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto skip_cross;
> +
> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> le32_to_cpu(ckpt->cur_data_segno[j])) {
> @@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> -
> +skip_cross:
> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>
>

2021-06-02 15:19:10

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v3] f2fs: support RO feature

On 2021/5/26 22:43, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.

It needs to update below two sysfs entries for RO feature?
/sys/fs/f2fs/<device>/features
/sys/fs/f2fs/features/..

Thanks,

>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> Change log from v2:
> - allow curseg updates
> - fix some bugs
>
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 4 ++++
> fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++------
> 3 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index eaf57b5f3c4b..9ad502f92529 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
> #define F2FS_FEATURE_CASEFOLD 0x1000
> #define F2FS_FEATURE_COMPRESSION 0x2000
> +#define F2FS_FEATURE_RO 0x4000
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> ((raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_DATA_TYPE (3)
> #define NR_CURSEG_NODE_TYPE (3)
> #define NR_CURSEG_INMEM_TYPE (2)
> +#define NR_CURSEG_RO_TYPE (2)
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 8668df7870d0..02e0c38be7eb 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
> unsigned int blkofs = curseg->next_blkoff;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
> + continue;
> +
> sanity_check_seg_type(sbi, curseg->seg_type);
>
> if (f2fs_test_bit(blkofs, se->cur_valid_map))
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index e70aca8f97bd..6788e7b71e27 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -555,7 +555,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> int ret;
>
> if (!options)
> - return 0;
> + goto default_check;
>
> while ((p = strsep(&options, ",")) != NULL) {
> int token;
> @@ -1090,6 +1090,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> return -EINVAL;
> }
> }
> +default_check:
> #ifdef CONFIG_QUOTA
> if (f2fs_check_quota_options(sbi))
> return -EINVAL;
> @@ -1162,6 +1163,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> */
> if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
> + f2fs_err(sbi, "Allow to mount readonly mode only");
> + return -EROFS;
> + }
> return 0;
> }
>
> @@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> static void default_options(struct f2fs_sb_info *sbi)
> {
> /* init some FS parameters */
> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> + else
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> +
> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> @@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
> goto skip;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
> + err = -EROFS;
> + goto restore_opts;
> + }
> +
> #ifdef CONFIG_QUOTA
> if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
> err = dquot_suspend(sb, -1);
> @@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>
> - if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> + if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> ovp_segments == 0 || reserved_segments == 0)) {
> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> return 1;
> }
> -
> user_block_count = le64_to_cpu(ckpt->user_block_count);
> - segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> + segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
> + (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0);
> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> if (!user_block_count || user_block_count >=
> segment_count_main << log_blocks_per_seg) {
> @@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto check_data;
> +
> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> le32_to_cpu(ckpt->cur_node_segno[j])) {
> @@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> +check_data:
> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto skip_cross;
> +
> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> le32_to_cpu(ckpt->cur_data_segno[j])) {
> @@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> -
> +skip_cross:
> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>
>

2021-06-02 19:05:48

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v4] f2fs: support RO feature

Given RO feature in superblock, we don't need to check provisioning/reserve
spaces and SSA area.

Signed-off-by: Jaegeuk Kim <[email protected]>
---

Change log from v3:
- add feature sysfs entries

fs/f2fs/f2fs.h | 3 +++
fs/f2fs/segment.c | 4 ++++
fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++------
fs/f2fs/sysfs.c | 8 ++++++++
4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eaf57b5f3c4b..8903c43091f8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -168,6 +168,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_FEATURE_CASEFOLD 0x1000
#define F2FS_FEATURE_COMPRESSION 0x2000
+#define F2FS_FEATURE_RO 0x4000

#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
#define NR_CURSEG_DATA_TYPE (3)
#define NR_CURSEG_NODE_TYPE (3)
#define NR_CURSEG_INMEM_TYPE (2)
+#define NR_CURSEG_RO_TYPE (2)
#define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
#define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)

@@ -4128,6 +4130,7 @@ F2FS_FEATURE_FUNCS(verity, VERITY);
F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
F2FS_FEATURE_FUNCS(casefold, CASEFOLD);
F2FS_FEATURE_FUNCS(compression, COMPRESSION);
+F2FS_FEATURE_FUNCS(readonly, RO);

#ifdef CONFIG_BLK_DEV_ZONED
static inline bool f2fs_blkz_is_seq(struct f2fs_sb_info *sbi, int devi,
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 380ef34e1a59..376c33ab71e2 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
unsigned int blkofs = curseg->next_blkoff;

+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
+ i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
+ continue;
+
sanity_check_seg_type(sbi, curseg->seg_type);

if (f2fs_test_bit(blkofs, se->cur_valid_map))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2fa59c674cd9..fb490383c767 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -555,7 +555,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
int ret;

if (!options)
- return 0;
+ goto default_check;

while ((p = strsep(&options, ",")) != NULL) {
int token;
@@ -1090,6 +1090,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
return -EINVAL;
}
}
+default_check:
#ifdef CONFIG_QUOTA
if (f2fs_check_quota_options(sbi))
return -EINVAL;
@@ -1162,6 +1163,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
*/
if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
+ f2fs_err(sbi, "Allow to mount readonly mode only");
+ return -EROFS;
+ }
return 0;
}

@@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
static void default_options(struct f2fs_sb_info *sbi)
{
/* init some FS parameters */
- F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
+ else
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+
F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
@@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
goto skip;

+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
+ err = -EROFS;
+ goto restore_opts;
+ }
+
#ifdef CONFIG_QUOTA
if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
err = dquot_suspend(sb, -1);
@@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);

- if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
+ if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
+ unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
ovp_segments == 0 || reserved_segments == 0)) {
f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
return 1;
}
-
user_block_count = le64_to_cpu(ckpt->user_block_count);
- segment_count_main = le32_to_cpu(raw_super->segment_count_main);
+ segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
+ (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0);
log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
if (!user_block_count || user_block_count >=
segment_count_main << log_blocks_per_seg) {
@@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ goto check_data;
+
for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
le32_to_cpu(ckpt->cur_node_segno[j])) {
@@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
+check_data:
for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
+ goto skip_cross;
+
for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
le32_to_cpu(ckpt->cur_data_segno[j])) {
@@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
-
+skip_cross:
sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);

diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 09e3f258eb52..62fbe4f20dd6 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -158,6 +158,9 @@ static ssize_t features_show(struct f2fs_attr *a,
if (f2fs_sb_has_casefold(sbi))
len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "casefold");
+ if (f2fs_sb_has_readonly(sbi))
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
+ len ? ", " : "", "readonly");
if (f2fs_sb_has_compression(sbi))
len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "compression");
@@ -578,6 +581,7 @@ enum feat_id {
FEAT_SB_CHECKSUM,
FEAT_CASEFOLD,
FEAT_COMPRESSION,
+ FEAT_RO,
FEAT_TEST_DUMMY_ENCRYPTION_V2,
};

@@ -599,6 +603,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
case FEAT_SB_CHECKSUM:
case FEAT_CASEFOLD:
case FEAT_COMPRESSION:
+ case FEAT_RO:
case FEAT_TEST_DUMMY_ENCRYPTION_V2:
return sprintf(buf, "supported\n");
}
@@ -723,12 +728,14 @@ F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
#ifdef CONFIG_UNICODE
F2FS_FEATURE_RO_ATTR(casefold, FEAT_CASEFOLD);
#endif
+F2FS_FEATURE_RO_ATTR(readonly, FEAT_RO);
#ifdef CONFIG_F2FS_FS_COMPRESSION
F2FS_FEATURE_RO_ATTR(compression, FEAT_COMPRESSION);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_written_block, compr_written_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_saved_block, compr_saved_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_new_inode, compr_new_inode);
#endif
+
/* For ATGC */
F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_ratio, candidate_ratio);
F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_count, max_candidate_count);
@@ -834,6 +841,7 @@ static struct attribute *f2fs_feat_attrs[] = {
#ifdef CONFIG_UNICODE
ATTR_LIST(casefold),
#endif
+ ATTR_LIST(readonly),
#ifdef CONFIG_F2FS_FS_COMPRESSION
ATTR_LIST(compression),
#endif
--
2.32.0.rc0.204.g9fa02ecfa5-goog

2021-06-03 15:53:51

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v4] f2fs: support RO feature

On 2021/6/3 3:02, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
>
> Change log from v3:
> - add feature sysfs entries
>
> fs/f2fs/f2fs.h | 3 +++
> fs/f2fs/segment.c | 4 ++++
> fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++------
> fs/f2fs/sysfs.c | 8 ++++++++
> 4 files changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index eaf57b5f3c4b..8903c43091f8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -168,6 +168,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
> #define F2FS_FEATURE_CASEFOLD 0x1000
> #define F2FS_FEATURE_COMPRESSION 0x2000
> +#define F2FS_FEATURE_RO 0x4000
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> ((raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_DATA_TYPE (3)
> #define NR_CURSEG_NODE_TYPE (3)
> #define NR_CURSEG_INMEM_TYPE (2)
> +#define NR_CURSEG_RO_TYPE (2)
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> @@ -4128,6 +4130,7 @@ F2FS_FEATURE_FUNCS(verity, VERITY);
> F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
> F2FS_FEATURE_FUNCS(casefold, CASEFOLD);
> F2FS_FEATURE_FUNCS(compression, COMPRESSION);
> +F2FS_FEATURE_FUNCS(readonly, RO);

If so, we'd better to use f2fs_sb_has_readonly() instead of F2FS_HAS_FEATURE()?

Thanks,

>
> #ifdef CONFIG_BLK_DEV_ZONED
> static inline bool f2fs_blkz_is_seq(struct f2fs_sb_info *sbi, int devi,
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 380ef34e1a59..376c33ab71e2 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
> struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
> unsigned int blkofs = curseg->next_blkoff;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
> + continue;
> +
> sanity_check_seg_type(sbi, curseg->seg_type);
>
> if (f2fs_test_bit(blkofs, se->cur_valid_map))
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 2fa59c674cd9..fb490383c767 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -555,7 +555,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> int ret;
>
> if (!options)
> - return 0;
> + goto default_check;
>
> while ((p = strsep(&options, ",")) != NULL) {
> int token;
> @@ -1090,6 +1090,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> return -EINVAL;
> }
> }
> +default_check:
> #ifdef CONFIG_QUOTA
> if (f2fs_check_quota_options(sbi))
> return -EINVAL;
> @@ -1162,6 +1163,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> */
> if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !f2fs_readonly(sbi->sb)) {
> + f2fs_err(sbi, "Allow to mount readonly mode only");
> + return -EROFS;
> + }
> return 0;
> }
>
> @@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> static void default_options(struct f2fs_sb_info *sbi)
> {
> /* init some FS parameters */
> - F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
> + else
> + F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
> +
> F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
> @@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
> goto skip;
>
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) && !(*flags & SB_RDONLY)) {
> + err = -EROFS;
> + goto restore_opts;
> + }
> +
> #ifdef CONFIG_QUOTA
> if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
> err = dquot_suspend(sb, -1);
> @@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
> reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);
>
> - if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> + if (!F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) &&
> + unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
> ovp_segments == 0 || reserved_segments == 0)) {
> f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
> return 1;
> }
> -
> user_block_count = le64_to_cpu(ckpt->user_block_count);
> - segment_count_main = le32_to_cpu(raw_super->segment_count_main);
> + segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
> + (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO) ? 1 : 0);
> log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
> if (!user_block_count || user_block_count >=
> segment_count_main << log_blocks_per_seg) {
> @@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto check_data;
> +
> for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
> le32_to_cpu(ckpt->cur_node_segno[j])) {
> @@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> +check_data:
> for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
> le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
> return 1;
> +
> + if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_RO))
> + goto skip_cross;
> +
> for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
> if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
> le32_to_cpu(ckpt->cur_data_segno[j])) {
> @@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> }
> }
> }
> -
> +skip_cross:
> sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
> nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);
>
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 09e3f258eb52..62fbe4f20dd6 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -158,6 +158,9 @@ static ssize_t features_show(struct f2fs_attr *a,
> if (f2fs_sb_has_casefold(sbi))
> len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
> len ? ", " : "", "casefold");
> + if (f2fs_sb_has_readonly(sbi))
> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
> + len ? ", " : "", "readonly");
> if (f2fs_sb_has_compression(sbi))
> len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
> len ? ", " : "", "compression");
> @@ -578,6 +581,7 @@ enum feat_id {
> FEAT_SB_CHECKSUM,
> FEAT_CASEFOLD,
> FEAT_COMPRESSION,
> + FEAT_RO,
> FEAT_TEST_DUMMY_ENCRYPTION_V2,
> };
>
> @@ -599,6 +603,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> case FEAT_SB_CHECKSUM:
> case FEAT_CASEFOLD:
> case FEAT_COMPRESSION:
> + case FEAT_RO:
> case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> return sprintf(buf, "supported\n");
> }
> @@ -723,12 +728,14 @@ F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
> #ifdef CONFIG_UNICODE
> F2FS_FEATURE_RO_ATTR(casefold, FEAT_CASEFOLD);
> #endif
> +F2FS_FEATURE_RO_ATTR(readonly, FEAT_RO);
> #ifdef CONFIG_F2FS_FS_COMPRESSION
> F2FS_FEATURE_RO_ATTR(compression, FEAT_COMPRESSION);
> F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_written_block, compr_written_block);
> F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_saved_block, compr_saved_block);
> F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_new_inode, compr_new_inode);
> #endif
> +
> /* For ATGC */
> F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_ratio, candidate_ratio);
> F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_count, max_candidate_count);
> @@ -834,6 +841,7 @@ static struct attribute *f2fs_feat_attrs[] = {
> #ifdef CONFIG_UNICODE
> ATTR_LIST(casefold),
> #endif
> + ATTR_LIST(readonly),
> #ifdef CONFIG_F2FS_FS_COMPRESSION
> ATTR_LIST(compression),
> #endif
>

2021-06-03 16:13:18

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v5] f2fs: support RO feature

Given RO feature in superblock, we don't need to check provisioning/reserve
spaces and SSA area.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
Change log from v4:
- use f2fs_sb_has_readonly

fs/f2fs/f2fs.h | 3 +++
fs/f2fs/segment.c | 4 ++++
fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++------
fs/f2fs/sysfs.c | 8 ++++++++
4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eaf57b5f3c4b..8903c43091f8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -168,6 +168,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_FEATURE_CASEFOLD 0x1000
#define F2FS_FEATURE_COMPRESSION 0x2000
+#define F2FS_FEATURE_RO 0x4000

#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -940,6 +941,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
#define NR_CURSEG_DATA_TYPE (3)
#define NR_CURSEG_NODE_TYPE (3)
#define NR_CURSEG_INMEM_TYPE (2)
+#define NR_CURSEG_RO_TYPE (2)
#define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
#define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)

@@ -4128,6 +4130,7 @@ F2FS_FEATURE_FUNCS(verity, VERITY);
F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
F2FS_FEATURE_FUNCS(casefold, CASEFOLD);
F2FS_FEATURE_FUNCS(compression, COMPRESSION);
+F2FS_FEATURE_FUNCS(readonly, RO);

#ifdef CONFIG_BLK_DEV_ZONED
static inline bool f2fs_blkz_is_seq(struct f2fs_sb_info *sbi, int devi,
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 380ef34e1a59..54847eebc5ca 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4683,6 +4683,10 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
struct seg_entry *se = get_seg_entry(sbi, curseg->segno);
unsigned int blkofs = curseg->next_blkoff;

+ if (f2fs_sb_has_readonly(sbi) &&
+ i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
+ continue;
+
sanity_check_seg_type(sbi, curseg->seg_type);

if (f2fs_test_bit(blkofs, se->cur_valid_map))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2fa59c674cd9..df8ac902886d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -555,7 +555,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
int ret;

if (!options)
- return 0;
+ goto default_check;

while ((p = strsep(&options, ",")) != NULL) {
int token;
@@ -1090,6 +1090,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
return -EINVAL;
}
}
+default_check:
#ifdef CONFIG_QUOTA
if (f2fs_check_quota_options(sbi))
return -EINVAL;
@@ -1162,6 +1163,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
*/
if (F2FS_OPTION(sbi).active_logs != NR_CURSEG_TYPE)
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
+
+ if (f2fs_sb_has_readonly(sbi) && !f2fs_readonly(sbi->sb)) {
+ f2fs_err(sbi, "Allow to mount readonly mode only");
+ return -EROFS;
+ }
return 0;
}

@@ -1819,7 +1825,11 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
static void default_options(struct f2fs_sb_info *sbi)
{
/* init some FS parameters */
- F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+ if (f2fs_sb_has_readonly(sbi))
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_RO_TYPE;
+ else
+ F2FS_OPTION(sbi).active_logs = NR_CURSEG_PERSIST_TYPE;
+
F2FS_OPTION(sbi).inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
F2FS_OPTION(sbi).whint_mode = WHINT_MODE_OFF;
F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_DEFAULT;
@@ -2001,6 +2011,11 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
goto skip;

+ if (f2fs_sb_has_readonly(sbi) && !(*flags & SB_RDONLY)) {
+ err = -EROFS;
+ goto restore_opts;
+ }
+
#ifdef CONFIG_QUOTA
if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
err = dquot_suspend(sb, -1);
@@ -3134,14 +3149,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
ovp_segments = le32_to_cpu(ckpt->overprov_segment_count);
reserved_segments = le32_to_cpu(ckpt->rsvd_segment_count);

- if (unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
+ if (!f2fs_sb_has_readonly(sbi) &&
+ unlikely(fsmeta < F2FS_MIN_META_SEGMENTS ||
ovp_segments == 0 || reserved_segments == 0)) {
f2fs_err(sbi, "Wrong layout: check mkfs.f2fs version");
return 1;
}
-
user_block_count = le64_to_cpu(ckpt->user_block_count);
- segment_count_main = le32_to_cpu(raw_super->segment_count_main);
+ segment_count_main = le32_to_cpu(raw_super->segment_count_main) +
+ (f2fs_sb_has_readonly(sbi) ? 1 : 0);
log_blocks_per_seg = le32_to_cpu(raw_super->log_blocks_per_seg);
if (!user_block_count || user_block_count >=
segment_count_main << log_blocks_per_seg) {
@@ -3172,6 +3188,10 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
if (le32_to_cpu(ckpt->cur_node_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_node_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (f2fs_sb_has_readonly(sbi))
+ goto check_data;
+
for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
le32_to_cpu(ckpt->cur_node_segno[j])) {
@@ -3182,10 +3202,15 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
+check_data:
for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
return 1;
+
+ if (f2fs_sb_has_readonly(sbi))
+ goto skip_cross;
+
for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
le32_to_cpu(ckpt->cur_data_segno[j])) {
@@ -3207,7 +3232,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
}
}
-
+skip_cross:
sit_bitmap_size = le32_to_cpu(ckpt->sit_ver_bitmap_bytesize);
nat_bitmap_size = le32_to_cpu(ckpt->nat_ver_bitmap_bytesize);

diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index dc71bc968c72..c579d5d3a916 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -158,6 +158,9 @@ static ssize_t features_show(struct f2fs_attr *a,
if (f2fs_sb_has_casefold(sbi))
len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "casefold");
+ if (f2fs_sb_has_readonly(sbi))
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
+ len ? ", " : "", "readonly");
if (f2fs_sb_has_compression(sbi))
len += scnprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "compression");
@@ -578,6 +581,7 @@ enum feat_id {
FEAT_SB_CHECKSUM,
FEAT_CASEFOLD,
FEAT_COMPRESSION,
+ FEAT_RO,
FEAT_TEST_DUMMY_ENCRYPTION_V2,
};

@@ -599,6 +603,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
case FEAT_SB_CHECKSUM:
case FEAT_CASEFOLD:
case FEAT_COMPRESSION:
+ case FEAT_RO:
case FEAT_TEST_DUMMY_ENCRYPTION_V2:
return sprintf(buf, "supported\n");
}
@@ -721,12 +726,14 @@ F2FS_FEATURE_RO_ATTR(verity, FEAT_VERITY);
#endif
F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
F2FS_FEATURE_RO_ATTR(casefold, FEAT_CASEFOLD);
+F2FS_FEATURE_RO_ATTR(readonly, FEAT_RO);
#ifdef CONFIG_F2FS_FS_COMPRESSION
F2FS_FEATURE_RO_ATTR(compression, FEAT_COMPRESSION);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_written_block, compr_written_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_saved_block, compr_saved_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_new_inode, compr_new_inode);
#endif
+
/* For ATGC */
F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_ratio, candidate_ratio);
F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_count, max_candidate_count);
@@ -830,6 +837,7 @@ static struct attribute *f2fs_feat_attrs[] = {
#endif
ATTR_LIST(sb_checksum),
ATTR_LIST(casefold),
+ ATTR_LIST(readonly),
#ifdef CONFIG_F2FS_FS_COMPRESSION
ATTR_LIST(compression),
#endif
--
2.32.0.rc0.204.g9fa02ecfa5-goog

2021-06-04 23:49:44

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 2/2 v5] f2fs: support RO feature

On 2021/6/4 0:08, Jaegeuk Kim wrote:
> Given RO feature in superblock, we don't need to check provisioning/reserve
> spaces and SSA area.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,