2024-02-07 16:46:31

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH] f2fs: kill heap-based allocation

No one uses this feature. Let's kill it.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
Documentation/filesystems/f2fs.rst | 3 --
fs/f2fs/f2fs.h | 51 ++++++++++++++--------------
fs/f2fs/gc.c | 5 ++-
fs/f2fs/segment.c | 54 ++++--------------------------
fs/f2fs/segment.h | 10 ------
fs/f2fs/super.c | 15 ---------
6 files changed, 34 insertions(+), 104 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 9ac5083dae8e..1ff751009c43 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -126,9 +126,6 @@ norecovery Disable the roll-forward recovery routine, mounted read-
discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
enabled, f2fs will issue discard/TRIM commands when a
segment is cleaned.
-no_heap Disable heap-style segment allocation which finds free
- segments for data from the beginning of main area, while
- for node from the end of main area.
nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
by default if CONFIG_F2FS_FS_XATTR is selected.
noacl Disable POSIX Access Control List. Note: acl is enabled
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 34d718301392..40eb590ed646 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -88,32 +88,31 @@ extern const char *f2fs_fault_name[FAULT_MAX];
*/
#define F2FS_MOUNT_DISABLE_ROLL_FORWARD 0x00000001
#define F2FS_MOUNT_DISCARD 0x00000002
-#define F2FS_MOUNT_NOHEAP 0x00000004
-#define F2FS_MOUNT_XATTR_USER 0x00000008
-#define F2FS_MOUNT_POSIX_ACL 0x00000010
-#define F2FS_MOUNT_DISABLE_EXT_IDENTIFY 0x00000020
-#define F2FS_MOUNT_INLINE_XATTR 0x00000040
-#define F2FS_MOUNT_INLINE_DATA 0x00000080
-#define F2FS_MOUNT_INLINE_DENTRY 0x00000100
-#define F2FS_MOUNT_FLUSH_MERGE 0x00000200
-#define F2FS_MOUNT_NOBARRIER 0x00000400
-#define F2FS_MOUNT_FASTBOOT 0x00000800
-#define F2FS_MOUNT_READ_EXTENT_CACHE 0x00001000
-#define F2FS_MOUNT_DATA_FLUSH 0x00002000
-#define F2FS_MOUNT_FAULT_INJECTION 0x00004000
-#define F2FS_MOUNT_USRQUOTA 0x00008000
-#define F2FS_MOUNT_GRPQUOTA 0x00010000
-#define F2FS_MOUNT_PRJQUOTA 0x00020000
-#define F2FS_MOUNT_QUOTA 0x00040000
-#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00080000
-#define F2FS_MOUNT_RESERVE_ROOT 0x00100000
-#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x00200000
-#define F2FS_MOUNT_NORECOVERY 0x00400000
-#define F2FS_MOUNT_ATGC 0x00800000
-#define F2FS_MOUNT_MERGE_CHECKPOINT 0x01000000
-#define F2FS_MOUNT_GC_MERGE 0x02000000
-#define F2FS_MOUNT_COMPRESS_CACHE 0x04000000
-#define F2FS_MOUNT_AGE_EXTENT_CACHE 0x08000000
+#define F2FS_MOUNT_XATTR_USER 0x00000004
+#define F2FS_MOUNT_POSIX_ACL 0x00000008
+#define F2FS_MOUNT_DISABLE_EXT_IDENTIFY 0x00000010
+#define F2FS_MOUNT_INLINE_XATTR 0x00000020
+#define F2FS_MOUNT_INLINE_DATA 0x00000040
+#define F2FS_MOUNT_INLINE_DENTRY 0x00000080
+#define F2FS_MOUNT_FLUSH_MERGE 0x00000100
+#define F2FS_MOUNT_NOBARRIER 0x00000200
+#define F2FS_MOUNT_FASTBOOT 0x00000400
+#define F2FS_MOUNT_READ_EXTENT_CACHE 0x00000800
+#define F2FS_MOUNT_DATA_FLUSH 0x00001000
+#define F2FS_MOUNT_FAULT_INJECTION 0x00002000
+#define F2FS_MOUNT_USRQUOTA 0x00004000
+#define F2FS_MOUNT_GRPQUOTA 0x00008000
+#define F2FS_MOUNT_PRJQUOTA 0x00010000
+#define F2FS_MOUNT_QUOTA 0x00020000
+#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00040000
+#define F2FS_MOUNT_RESERVE_ROOT 0x00080000
+#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x00100000
+#define F2FS_MOUNT_NORECOVERY 0x00200000
+#define F2FS_MOUNT_ATGC 0x00400000
+#define F2FS_MOUNT_MERGE_CHECKPOINT 0x00800000
+#define F2FS_MOUNT_GC_MERGE 0x01000000
+#define F2FS_MOUNT_COMPRESS_CACHE 0x02000000
+#define F2FS_MOUNT_AGE_EXTENT_CACHE 0x04000000

#define F2FS_OPTION(sbi) ((sbi)->mount_opt)
#define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 0a1a50b68df8..8a9cdc5a72c5 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
p->max_search > sbi->max_victim_search)
p->max_search = sbi->max_victim_search;

- /* let's select beginning hot/small space first in no_heap mode*/
+ /* let's select beginning hot/small space first. */
if (f2fs_need_rand_seg(sbi))
p->offset = get_random_u32_below(MAIN_SECS(sbi) *
SEGS_PER_SEC(sbi));
- else if (test_opt(sbi, NOHEAP) &&
- (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
+ else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
p->offset = 0;
else
p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 1013276ad12a..4e985750c938 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2632,16 +2632,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
* This function should be returned with success, otherwise BUG
*/
static void get_new_segment(struct f2fs_sb_info *sbi,
- unsigned int *newseg, bool new_sec, int dir)
+ unsigned int *newseg, bool new_sec)
{
struct free_segmap_info *free_i = FREE_I(sbi);
unsigned int segno, secno, zoneno;
unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
- unsigned int left_start = hint;
bool init = true;
- int go_left = 0;
int i;

spin_lock(&free_i->segmap_lock);
@@ -2655,30 +2653,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
find_other_zone:
secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
if (secno >= MAIN_SECS(sbi)) {
- if (dir == ALLOC_RIGHT) {
- secno = find_first_zero_bit(free_i->free_secmap,
+ secno = find_first_zero_bit(free_i->free_secmap,
MAIN_SECS(sbi));
- f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
- } else {
- go_left = 1;
- left_start = hint - 1;
- }
- }
- if (go_left == 0)
- goto skip_left;
-
- while (test_bit(left_start, free_i->free_secmap)) {
- if (left_start > 0) {
- left_start--;
- continue;
- }
- left_start = find_first_zero_bit(free_i->free_secmap,
- MAIN_SECS(sbi));
- f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
- break;
+ f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
}
- secno = left_start;
-skip_left:
segno = GET_SEG_FROM_SEC(sbi, secno);
zoneno = GET_ZONE_FROM_SEC(sbi, secno);

@@ -2689,21 +2667,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
goto got_it;
if (zoneno == old_zoneno)
goto got_it;
- if (dir == ALLOC_LEFT) {
- if (!go_left && zoneno + 1 >= total_zones)
- goto got_it;
- if (go_left && zoneno == 0)
- goto got_it;
- }
for (i = 0; i < NR_CURSEG_TYPE; i++)
if (CURSEG_I(sbi, i)->zone == zoneno)
break;

if (i < NR_CURSEG_TYPE) {
/* zone is in user, try another */
- if (go_left)
- hint = zoneno * sbi->secs_per_zone - 1;
- else if (zoneno + 1 >= total_zones)
+ if (zoneno + 1 >= total_zones)
hint = 0;
else
hint = (zoneno + 1) * sbi->secs_per_zone;
@@ -2762,8 +2732,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
return 0;

- if (test_opt(sbi, NOHEAP) &&
- (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
+ if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
return 0;

if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
@@ -2783,21 +2752,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
{
struct curseg_info *curseg = CURSEG_I(sbi, type);
- unsigned short seg_type = curseg->seg_type;
unsigned int segno = curseg->segno;
- int dir = ALLOC_LEFT;

if (curseg->inited)
- write_sum_page(sbi, curseg->sum_blk,
- GET_SUM_BLOCK(sbi, segno));
- if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
- dir = ALLOC_RIGHT;
-
- if (test_opt(sbi, NOHEAP))
- dir = ALLOC_RIGHT;
-
+ write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
segno = __get_next_segno(sbi, type);
- get_new_segment(sbi, &segno, new_sec, dir);
+ get_new_segment(sbi, &segno, new_sec);
curseg->next_segno = segno;
reset_curseg(sbi, type, 1);
curseg->alloc_type = LFS;
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index b725ae1a7043..60d93a16f2ac 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -130,16 +130,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
#define SECTOR_TO_BLOCK(sectors) \
((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)

-/*
- * indicate a block allocation direction: RIGHT and LEFT.
- * RIGHT means allocating new sections towards the end of volume.
- * LEFT means the opposite direction.
- */
-enum {
- ALLOC_RIGHT = 0,
- ALLOC_LEFT
-};
-
/*
* In the victim_sel_policy->alloc_mode, there are three block allocation modes.
* LFS writes data sequentially with cleaning operations.
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 3fb44afd0cd1..c04c0d21705f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -111,8 +111,6 @@ enum {
Opt_norecovery,
Opt_discard,
Opt_nodiscard,
- Opt_noheap,
- Opt_heap,
Opt_user_xattr,
Opt_nouser_xattr,
Opt_acl,
@@ -189,8 +187,6 @@ static match_table_t f2fs_tokens = {
{Opt_norecovery, "norecovery"},
{Opt_discard, "discard"},
{Opt_nodiscard, "nodiscard"},
- {Opt_noheap, "no_heap"},
- {Opt_heap, "heap"},
{Opt_user_xattr, "user_xattr"},
{Opt_nouser_xattr, "nouser_xattr"},
{Opt_acl, "acl"},
@@ -726,12 +722,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
}
clear_opt(sbi, DISCARD);
break;
- case Opt_noheap:
- set_opt(sbi, NOHEAP);
- break;
- case Opt_heap:
- clear_opt(sbi, NOHEAP);
- break;
#ifdef CONFIG_F2FS_FS_XATTR
case Opt_user_xattr:
set_opt(sbi, XATTR_USER);
@@ -1956,10 +1946,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
} else {
seq_puts(seq, ",nodiscard");
}
- if (test_opt(sbi, NOHEAP))
- seq_puts(seq, ",no_heap");
- else
- seq_puts(seq, ",heap");
#ifdef CONFIG_F2FS_FS_XATTR
if (test_opt(sbi, XATTR_USER))
seq_puts(seq, ",user_xattr");
@@ -2136,7 +2122,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
set_opt(sbi, INLINE_XATTR);
set_opt(sbi, INLINE_DATA);
set_opt(sbi, INLINE_DENTRY);
- set_opt(sbi, NOHEAP);
set_opt(sbi, MERGE_CHECKPOINT);
F2FS_OPTION(sbi).unusable_cap = 0;
sbi->sb->s_flags |= SB_LAZYTIME;
--
2.43.0.594.gd9cf4e227d-goog



2024-02-07 23:01:31

by Daeho Jeong

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: kill heap-based allocation

Reviewed-by: Daeho Jeong <[email protected]>

On Wed, Feb 7, 2024 at 8:50 AM Jaegeuk Kim <[email protected]> wrote:
>
> No one uses this feature. Let's kill it.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> Documentation/filesystems/f2fs.rst | 3 --
> fs/f2fs/f2fs.h | 51 ++++++++++++++--------------
> fs/f2fs/gc.c | 5 ++-
> fs/f2fs/segment.c | 54 ++++--------------------------
> fs/f2fs/segment.h | 10 ------
> fs/f2fs/super.c | 15 ---------
> 6 files changed, 34 insertions(+), 104 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 9ac5083dae8e..1ff751009c43 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -126,9 +126,6 @@ norecovery Disable the roll-forward recovery routine, mounted read-
> discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
> enabled, f2fs will issue discard/TRIM commands when a
> segment is cleaned.
> -no_heap Disable heap-style segment allocation which finds free
> - segments for data from the beginning of main area, while
> - for node from the end of main area.
> nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
> by default if CONFIG_F2FS_FS_XATTR is selected.
> noacl Disable POSIX Access Control List. Note: acl is enabled
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 34d718301392..40eb590ed646 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -88,32 +88,31 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> */
> #define F2FS_MOUNT_DISABLE_ROLL_FORWARD 0x00000001
> #define F2FS_MOUNT_DISCARD 0x00000002
> -#define F2FS_MOUNT_NOHEAP 0x00000004
> -#define F2FS_MOUNT_XATTR_USER 0x00000008
> -#define F2FS_MOUNT_POSIX_ACL 0x00000010
> -#define F2FS_MOUNT_DISABLE_EXT_IDENTIFY 0x00000020
> -#define F2FS_MOUNT_INLINE_XATTR 0x00000040
> -#define F2FS_MOUNT_INLINE_DATA 0x00000080
> -#define F2FS_MOUNT_INLINE_DENTRY 0x00000100
> -#define F2FS_MOUNT_FLUSH_MERGE 0x00000200
> -#define F2FS_MOUNT_NOBARRIER 0x00000400
> -#define F2FS_MOUNT_FASTBOOT 0x00000800
> -#define F2FS_MOUNT_READ_EXTENT_CACHE 0x00001000
> -#define F2FS_MOUNT_DATA_FLUSH 0x00002000
> -#define F2FS_MOUNT_FAULT_INJECTION 0x00004000
> -#define F2FS_MOUNT_USRQUOTA 0x00008000
> -#define F2FS_MOUNT_GRPQUOTA 0x00010000
> -#define F2FS_MOUNT_PRJQUOTA 0x00020000
> -#define F2FS_MOUNT_QUOTA 0x00040000
> -#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00080000
> -#define F2FS_MOUNT_RESERVE_ROOT 0x00100000
> -#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x00200000
> -#define F2FS_MOUNT_NORECOVERY 0x00400000
> -#define F2FS_MOUNT_ATGC 0x00800000
> -#define F2FS_MOUNT_MERGE_CHECKPOINT 0x01000000
> -#define F2FS_MOUNT_GC_MERGE 0x02000000
> -#define F2FS_MOUNT_COMPRESS_CACHE 0x04000000
> -#define F2FS_MOUNT_AGE_EXTENT_CACHE 0x08000000
> +#define F2FS_MOUNT_XATTR_USER 0x00000004
> +#define F2FS_MOUNT_POSIX_ACL 0x00000008
> +#define F2FS_MOUNT_DISABLE_EXT_IDENTIFY 0x00000010
> +#define F2FS_MOUNT_INLINE_XATTR 0x00000020
> +#define F2FS_MOUNT_INLINE_DATA 0x00000040
> +#define F2FS_MOUNT_INLINE_DENTRY 0x00000080
> +#define F2FS_MOUNT_FLUSH_MERGE 0x00000100
> +#define F2FS_MOUNT_NOBARRIER 0x00000200
> +#define F2FS_MOUNT_FASTBOOT 0x00000400
> +#define F2FS_MOUNT_READ_EXTENT_CACHE 0x00000800
> +#define F2FS_MOUNT_DATA_FLUSH 0x00001000
> +#define F2FS_MOUNT_FAULT_INJECTION 0x00002000
> +#define F2FS_MOUNT_USRQUOTA 0x00004000
> +#define F2FS_MOUNT_GRPQUOTA 0x00008000
> +#define F2FS_MOUNT_PRJQUOTA 0x00010000
> +#define F2FS_MOUNT_QUOTA 0x00020000
> +#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00040000
> +#define F2FS_MOUNT_RESERVE_ROOT 0x00080000
> +#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x00100000
> +#define F2FS_MOUNT_NORECOVERY 0x00200000
> +#define F2FS_MOUNT_ATGC 0x00400000
> +#define F2FS_MOUNT_MERGE_CHECKPOINT 0x00800000
> +#define F2FS_MOUNT_GC_MERGE 0x01000000
> +#define F2FS_MOUNT_COMPRESS_CACHE 0x02000000
> +#define F2FS_MOUNT_AGE_EXTENT_CACHE 0x04000000
>
> #define F2FS_OPTION(sbi) ((sbi)->mount_opt)
> #define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 0a1a50b68df8..8a9cdc5a72c5 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
> p->max_search > sbi->max_victim_search)
> p->max_search = sbi->max_victim_search;
>
> - /* let's select beginning hot/small space first in no_heap mode*/
> + /* let's select beginning hot/small space first. */
> if (f2fs_need_rand_seg(sbi))
> p->offset = get_random_u32_below(MAIN_SECS(sbi) *
> SEGS_PER_SEC(sbi));
> - else if (test_opt(sbi, NOHEAP) &&
> - (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
> + else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
> p->offset = 0;
> else
> p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 1013276ad12a..4e985750c938 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2632,16 +2632,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
> * This function should be returned with success, otherwise BUG
> */
> static void get_new_segment(struct f2fs_sb_info *sbi,
> - unsigned int *newseg, bool new_sec, int dir)
> + unsigned int *newseg, bool new_sec)
> {
> struct free_segmap_info *free_i = FREE_I(sbi);
> unsigned int segno, secno, zoneno;
> unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
> unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
> unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
> - unsigned int left_start = hint;
> bool init = true;
> - int go_left = 0;
> int i;
>
> spin_lock(&free_i->segmap_lock);
> @@ -2655,30 +2653,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> find_other_zone:
> secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
> if (secno >= MAIN_SECS(sbi)) {
> - if (dir == ALLOC_RIGHT) {
> - secno = find_first_zero_bit(free_i->free_secmap,
> + secno = find_first_zero_bit(free_i->free_secmap,
> MAIN_SECS(sbi));
> - f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> - } else {
> - go_left = 1;
> - left_start = hint - 1;
> - }
> - }
> - if (go_left == 0)
> - goto skip_left;
> -
> - while (test_bit(left_start, free_i->free_secmap)) {
> - if (left_start > 0) {
> - left_start--;
> - continue;
> - }
> - left_start = find_first_zero_bit(free_i->free_secmap,
> - MAIN_SECS(sbi));
> - f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
> - break;
> + f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> }
> - secno = left_start;
> -skip_left:
> segno = GET_SEG_FROM_SEC(sbi, secno);
> zoneno = GET_ZONE_FROM_SEC(sbi, secno);
>
> @@ -2689,21 +2667,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> goto got_it;
> if (zoneno == old_zoneno)
> goto got_it;
> - if (dir == ALLOC_LEFT) {
> - if (!go_left && zoneno + 1 >= total_zones)
> - goto got_it;
> - if (go_left && zoneno == 0)
> - goto got_it;
> - }
> for (i = 0; i < NR_CURSEG_TYPE; i++)
> if (CURSEG_I(sbi, i)->zone == zoneno)
> break;
>
> if (i < NR_CURSEG_TYPE) {
> /* zone is in user, try another */
> - if (go_left)
> - hint = zoneno * sbi->secs_per_zone - 1;
> - else if (zoneno + 1 >= total_zones)
> + if (zoneno + 1 >= total_zones)
> hint = 0;
> else
> hint = (zoneno + 1) * sbi->secs_per_zone;
> @@ -2762,8 +2732,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
> return 0;
>
> - if (test_opt(sbi, NOHEAP) &&
> - (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
> + if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
> return 0;
>
> if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
> @@ -2783,21 +2752,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
> {
> struct curseg_info *curseg = CURSEG_I(sbi, type);
> - unsigned short seg_type = curseg->seg_type;
> unsigned int segno = curseg->segno;
> - int dir = ALLOC_LEFT;
>
> if (curseg->inited)
> - write_sum_page(sbi, curseg->sum_blk,
> - GET_SUM_BLOCK(sbi, segno));
> - if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
> - dir = ALLOC_RIGHT;
> -
> - if (test_opt(sbi, NOHEAP))
> - dir = ALLOC_RIGHT;
> -
> + write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
> segno = __get_next_segno(sbi, type);
> - get_new_segment(sbi, &segno, new_sec, dir);
> + get_new_segment(sbi, &segno, new_sec);
> curseg->next_segno = segno;
> reset_curseg(sbi, type, 1);
> curseg->alloc_type = LFS;
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index b725ae1a7043..60d93a16f2ac 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -130,16 +130,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> #define SECTOR_TO_BLOCK(sectors) \
> ((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>
> -/*
> - * indicate a block allocation direction: RIGHT and LEFT.
> - * RIGHT means allocating new sections towards the end of volume.
> - * LEFT means the opposite direction.
> - */
> -enum {
> - ALLOC_RIGHT = 0,
> - ALLOC_LEFT
> -};
> -
> /*
> * In the victim_sel_policy->alloc_mode, there are three block allocation modes.
> * LFS writes data sequentially with cleaning operations.
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 3fb44afd0cd1..c04c0d21705f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -111,8 +111,6 @@ enum {
> Opt_norecovery,
> Opt_discard,
> Opt_nodiscard,
> - Opt_noheap,
> - Opt_heap,
> Opt_user_xattr,
> Opt_nouser_xattr,
> Opt_acl,
> @@ -189,8 +187,6 @@ static match_table_t f2fs_tokens = {
> {Opt_norecovery, "norecovery"},
> {Opt_discard, "discard"},
> {Opt_nodiscard, "nodiscard"},
> - {Opt_noheap, "no_heap"},
> - {Opt_heap, "heap"},
> {Opt_user_xattr, "user_xattr"},
> {Opt_nouser_xattr, "nouser_xattr"},
> {Opt_acl, "acl"},
> @@ -726,12 +722,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> }
> clear_opt(sbi, DISCARD);
> break;
> - case Opt_noheap:
> - set_opt(sbi, NOHEAP);
> - break;
> - case Opt_heap:
> - clear_opt(sbi, NOHEAP);
> - break;
> #ifdef CONFIG_F2FS_FS_XATTR
> case Opt_user_xattr:
> set_opt(sbi, XATTR_USER);
> @@ -1956,10 +1946,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> } else {
> seq_puts(seq, ",nodiscard");
> }
> - if (test_opt(sbi, NOHEAP))
> - seq_puts(seq, ",no_heap");
> - else
> - seq_puts(seq, ",heap");
> #ifdef CONFIG_F2FS_FS_XATTR
> if (test_opt(sbi, XATTR_USER))
> seq_puts(seq, ",user_xattr");
> @@ -2136,7 +2122,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
> set_opt(sbi, INLINE_XATTR);
> set_opt(sbi, INLINE_DATA);
> set_opt(sbi, INLINE_DENTRY);
> - set_opt(sbi, NOHEAP);
> set_opt(sbi, MERGE_CHECKPOINT);
> F2FS_OPTION(sbi).unusable_cap = 0;
> sbi->sb->s_flags |= SB_LAZYTIME;
> --
> 2.43.0.594.gd9cf4e227d-goog
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-20 08:23:23

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: kill heap-based allocation

On 2024/2/8 0:46, Jaegeuk Kim wrote:
> No one uses this feature. Let's kill it.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> Documentation/filesystems/f2fs.rst | 3 --
> fs/f2fs/f2fs.h | 51 ++++++++++++++--------------
> fs/f2fs/gc.c | 5 ++-
> fs/f2fs/segment.c | 54 ++++--------------------------
> fs/f2fs/segment.h | 10 ------
> fs/f2fs/super.c | 15 ---------
> 6 files changed, 34 insertions(+), 104 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 9ac5083dae8e..1ff751009c43 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -126,9 +126,6 @@ norecovery Disable the roll-forward recovery routine, mounted read-
> discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
> enabled, f2fs will issue discard/TRIM commands when a
> segment is cleaned.
> -no_heap Disable heap-style segment allocation which finds free
> - segments for data from the beginning of main area, while
> - for node from the end of main area.
> nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
> by default if CONFIG_F2FS_FS_XATTR is selected.
> noacl Disable POSIX Access Control List. Note: acl is enabled
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 34d718301392..40eb590ed646 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -88,32 +88,31 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> */
> #define F2FS_MOUNT_DISABLE_ROLL_FORWARD 0x00000001
> #define F2FS_MOUNT_DISCARD 0x00000002
> -#define F2FS_MOUNT_NOHEAP 0x00000004
> -#define F2FS_MOUNT_XATTR_USER 0x00000008
> -#define F2FS_MOUNT_POSIX_ACL 0x00000010
> -#define F2FS_MOUNT_DISABLE_EXT_IDENTIFY 0x00000020
> -#define F2FS_MOUNT_INLINE_XATTR 0x00000040
> -#define F2FS_MOUNT_INLINE_DATA 0x00000080
> -#define F2FS_MOUNT_INLINE_DENTRY 0x00000100
> -#define F2FS_MOUNT_FLUSH_MERGE 0x00000200
> -#define F2FS_MOUNT_NOBARRIER 0x00000400
> -#define F2FS_MOUNT_FASTBOOT 0x00000800
> -#define F2FS_MOUNT_READ_EXTENT_CACHE 0x00001000
> -#define F2FS_MOUNT_DATA_FLUSH 0x00002000
> -#define F2FS_MOUNT_FAULT_INJECTION 0x00004000
> -#define F2FS_MOUNT_USRQUOTA 0x00008000
> -#define F2FS_MOUNT_GRPQUOTA 0x00010000
> -#define F2FS_MOUNT_PRJQUOTA 0x00020000
> -#define F2FS_MOUNT_QUOTA 0x00040000
> -#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00080000
> -#define F2FS_MOUNT_RESERVE_ROOT 0x00100000
> -#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x00200000
> -#define F2FS_MOUNT_NORECOVERY 0x00400000
> -#define F2FS_MOUNT_ATGC 0x00800000
> -#define F2FS_MOUNT_MERGE_CHECKPOINT 0x01000000
> -#define F2FS_MOUNT_GC_MERGE 0x02000000
> -#define F2FS_MOUNT_COMPRESS_CACHE 0x04000000
> -#define F2FS_MOUNT_AGE_EXTENT_CACHE 0x08000000
> +#define F2FS_MOUNT_XATTR_USER 0x00000004
> +#define F2FS_MOUNT_POSIX_ACL 0x00000008
> +#define F2FS_MOUNT_DISABLE_EXT_IDENTIFY 0x00000010
> +#define F2FS_MOUNT_INLINE_XATTR 0x00000020
> +#define F2FS_MOUNT_INLINE_DATA 0x00000040
> +#define F2FS_MOUNT_INLINE_DENTRY 0x00000080
> +#define F2FS_MOUNT_FLUSH_MERGE 0x00000100
> +#define F2FS_MOUNT_NOBARRIER 0x00000200
> +#define F2FS_MOUNT_FASTBOOT 0x00000400
> +#define F2FS_MOUNT_READ_EXTENT_CACHE 0x00000800
> +#define F2FS_MOUNT_DATA_FLUSH 0x00001000
> +#define F2FS_MOUNT_FAULT_INJECTION 0x00002000
> +#define F2FS_MOUNT_USRQUOTA 0x00004000
> +#define F2FS_MOUNT_GRPQUOTA 0x00008000
> +#define F2FS_MOUNT_PRJQUOTA 0x00010000
> +#define F2FS_MOUNT_QUOTA 0x00020000
> +#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00040000
> +#define F2FS_MOUNT_RESERVE_ROOT 0x00080000
> +#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x00100000
> +#define F2FS_MOUNT_NORECOVERY 0x00200000
> +#define F2FS_MOUNT_ATGC 0x00400000
> +#define F2FS_MOUNT_MERGE_CHECKPOINT 0x00800000
> +#define F2FS_MOUNT_GC_MERGE 0x01000000
> +#define F2FS_MOUNT_COMPRESS_CACHE 0x02000000
> +#define F2FS_MOUNT_AGE_EXTENT_CACHE 0x04000000
>
> #define F2FS_OPTION(sbi) ((sbi)->mount_opt)
> #define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 0a1a50b68df8..8a9cdc5a72c5 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
> p->max_search > sbi->max_victim_search)
> p->max_search = sbi->max_victim_search;
>
> - /* let's select beginning hot/small space first in no_heap mode*/
> + /* let's select beginning hot/small space first. */
> if (f2fs_need_rand_seg(sbi))
> p->offset = get_random_u32_below(MAIN_SECS(sbi) *
> SEGS_PER_SEC(sbi));
> - else if (test_opt(sbi, NOHEAP) &&
> - (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
> + else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
> p->offset = 0;
> else
> p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 1013276ad12a..4e985750c938 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2632,16 +2632,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
> * This function should be returned with success, otherwise BUG
> */
> static void get_new_segment(struct f2fs_sb_info *sbi,
> - unsigned int *newseg, bool new_sec, int dir)
> + unsigned int *newseg, bool new_sec)
> {
> struct free_segmap_info *free_i = FREE_I(sbi);
> unsigned int segno, secno, zoneno;
> unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
> unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
> unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
> - unsigned int left_start = hint;
> bool init = true;
> - int go_left = 0;
> int i;
>
> spin_lock(&free_i->segmap_lock);
> @@ -2655,30 +2653,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> find_other_zone:
> secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
> if (secno >= MAIN_SECS(sbi)) {
> - if (dir == ALLOC_RIGHT) {
> - secno = find_first_zero_bit(free_i->free_secmap,
> + secno = find_first_zero_bit(free_i->free_secmap,
> MAIN_SECS(sbi));
> - f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> - } else {
> - go_left = 1;
> - left_start = hint - 1;
> - }
> - }
> - if (go_left == 0)
> - goto skip_left;
> -
> - while (test_bit(left_start, free_i->free_secmap)) {
> - if (left_start > 0) {
> - left_start--;
> - continue;
> - }
> - left_start = find_first_zero_bit(free_i->free_secmap,
> - MAIN_SECS(sbi));
> - f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
> - break;
> + f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> }
> - secno = left_start;
> -skip_left:
> segno = GET_SEG_FROM_SEC(sbi, secno);
> zoneno = GET_ZONE_FROM_SEC(sbi, secno);
>
> @@ -2689,21 +2667,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> goto got_it;
> if (zoneno == old_zoneno)
> goto got_it;
> - if (dir == ALLOC_LEFT) {
> - if (!go_left && zoneno + 1 >= total_zones)
> - goto got_it;
> - if (go_left && zoneno == 0)
> - goto got_it;
> - }
> for (i = 0; i < NR_CURSEG_TYPE; i++)
> if (CURSEG_I(sbi, i)->zone == zoneno)
> break;
>
> if (i < NR_CURSEG_TYPE) {
> /* zone is in user, try another */
> - if (go_left)
> - hint = zoneno * sbi->secs_per_zone - 1;
> - else if (zoneno + 1 >= total_zones)
> + if (zoneno + 1 >= total_zones)
> hint = 0;
> else
> hint = (zoneno + 1) * sbi->secs_per_zone;
> @@ -2762,8 +2732,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
> return 0;
>
> - if (test_opt(sbi, NOHEAP) &&
> - (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
> + if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
> return 0;
>
> if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
> @@ -2783,21 +2752,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
> {
> struct curseg_info *curseg = CURSEG_I(sbi, type);
> - unsigned short seg_type = curseg->seg_type;
> unsigned int segno = curseg->segno;
> - int dir = ALLOC_LEFT;
>
> if (curseg->inited)
> - write_sum_page(sbi, curseg->sum_blk,
> - GET_SUM_BLOCK(sbi, segno));
> - if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
> - dir = ALLOC_RIGHT;
> -
> - if (test_opt(sbi, NOHEAP))
> - dir = ALLOC_RIGHT;
> -
> + write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
> segno = __get_next_segno(sbi, type);
> - get_new_segment(sbi, &segno, new_sec, dir);
> + get_new_segment(sbi, &segno, new_sec);
> curseg->next_segno = segno;
> reset_curseg(sbi, type, 1);
> curseg->alloc_type = LFS;
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index b725ae1a7043..60d93a16f2ac 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -130,16 +130,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> #define SECTOR_TO_BLOCK(sectors) \
> ((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>
> -/*
> - * indicate a block allocation direction: RIGHT and LEFT.
> - * RIGHT means allocating new sections towards the end of volume.
> - * LEFT means the opposite direction.
> - */
> -enum {
> - ALLOC_RIGHT = 0,
> - ALLOC_LEFT
> -};
> -
> /*
> * In the victim_sel_policy->alloc_mode, there are three block allocation modes.
> * LFS writes data sequentially with cleaning operations.
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 3fb44afd0cd1..c04c0d21705f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -111,8 +111,6 @@ enum {
> Opt_norecovery,
> Opt_discard,
> Opt_nodiscard,
> - Opt_noheap,
> - Opt_heap,
> Opt_user_xattr,
> Opt_nouser_xattr,
> Opt_acl,
> @@ -189,8 +187,6 @@ static match_table_t f2fs_tokens = {
> {Opt_norecovery, "norecovery"},
> {Opt_discard, "discard"},
> {Opt_nodiscard, "nodiscard"},
> - {Opt_noheap, "no_heap"},
> - {Opt_heap, "heap"},
> {Opt_user_xattr, "user_xattr"},
> {Opt_nouser_xattr, "nouser_xattr"},
> {Opt_acl, "acl"},
> @@ -726,12 +722,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> }
> clear_opt(sbi, DISCARD);
> break;
> - case Opt_noheap:
> - set_opt(sbi, NOHEAP);
> - break;
> - case Opt_heap:
> - clear_opt(sbi, NOHEAP);
> - break;

If user uses noheap/heap mount option, this may cause mount failure
due to below handling:

default:
f2fs_err(sbi, "Unrecognized mount option \"%s\" or missing value",
p);
return -EINVAL;

Can we keep forward compatibility by continuing to handle noheap/heap
mount option, and leave kernel message as notification for such cases?

Thanks,

> #ifdef CONFIG_F2FS_FS_XATTR
> case Opt_user_xattr:
> set_opt(sbi, XATTR_USER);
> @@ -1956,10 +1946,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> } else {
> seq_puts(seq, ",nodiscard");
> }
> - if (test_opt(sbi, NOHEAP))
> - seq_puts(seq, ",no_heap");
> - else
> - seq_puts(seq, ",heap");
> #ifdef CONFIG_F2FS_FS_XATTR
> if (test_opt(sbi, XATTR_USER))
> seq_puts(seq, ",user_xattr");
> @@ -2136,7 +2122,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
> set_opt(sbi, INLINE_XATTR);
> set_opt(sbi, INLINE_DATA);
> set_opt(sbi, INLINE_DENTRY);
> - set_opt(sbi, NOHEAP);
> set_opt(sbi, MERGE_CHECKPOINT);
> F2FS_OPTION(sbi).unusable_cap = 0;
> sbi->sb->s_flags |= SB_LAZYTIME;

2024-02-20 20:52:05

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH v2] f2fs: kill heap-based allocation

No one uses this feature. Let's kill it.

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

Change log from v1:
- keep mount options but give warnings instead

Documentation/filesystems/f2fs.rst | 4 +--
fs/f2fs/gc.c | 5 ++-
fs/f2fs/segment.c | 54 ++++--------------------------
fs/f2fs/segment.h | 10 ------
fs/f2fs/super.c | 9 +----
5 files changed, 11 insertions(+), 71 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 9ac5083dae8e..c30a800604fd 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -126,9 +126,7 @@ norecovery Disable the roll-forward recovery routine, mounted read-
discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
enabled, f2fs will issue discard/TRIM commands when a
segment is cleaned.
-no_heap Disable heap-style segment allocation which finds free
- segments for data from the beginning of main area, while
- for node from the end of main area.
+no_heap Deprecated.
nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
by default if CONFIG_F2FS_FS_XATTR is selected.
noacl Disable POSIX Access Control List. Note: acl is enabled
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 501b93b45b6c..a089a938355b 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
p->max_search > sbi->max_victim_search)
p->max_search = sbi->max_victim_search;

- /* let's select beginning hot/small space first in no_heap mode*/
+ /* let's select beginning hot/small space first. */
if (f2fs_need_rand_seg(sbi))
p->offset = get_random_u32_below(MAIN_SECS(sbi) *
SEGS_PER_SEC(sbi));
- else if (test_opt(sbi, NOHEAP) &&
- (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
+ else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
p->offset = 0;
else
p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 94c4f7b16c19..09af17af4e7a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2634,16 +2634,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
* This function should be returned with success, otherwise BUG
*/
static void get_new_segment(struct f2fs_sb_info *sbi,
- unsigned int *newseg, bool new_sec, int dir)
+ unsigned int *newseg, bool new_sec)
{
struct free_segmap_info *free_i = FREE_I(sbi);
unsigned int segno, secno, zoneno;
unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
- unsigned int left_start = hint;
bool init = true;
- int go_left = 0;
int i;

spin_lock(&free_i->segmap_lock);
@@ -2657,30 +2655,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
find_other_zone:
secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
if (secno >= MAIN_SECS(sbi)) {
- if (dir == ALLOC_RIGHT) {
- secno = find_first_zero_bit(free_i->free_secmap,
+ secno = find_first_zero_bit(free_i->free_secmap,
MAIN_SECS(sbi));
- f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
- } else {
- go_left = 1;
- left_start = hint - 1;
- }
- }
- if (go_left == 0)
- goto skip_left;
-
- while (test_bit(left_start, free_i->free_secmap)) {
- if (left_start > 0) {
- left_start--;
- continue;
- }
- left_start = find_first_zero_bit(free_i->free_secmap,
- MAIN_SECS(sbi));
- f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
- break;
+ f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
}
- secno = left_start;
-skip_left:
segno = GET_SEG_FROM_SEC(sbi, secno);
zoneno = GET_ZONE_FROM_SEC(sbi, secno);

@@ -2691,21 +2669,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
goto got_it;
if (zoneno == old_zoneno)
goto got_it;
- if (dir == ALLOC_LEFT) {
- if (!go_left && zoneno + 1 >= total_zones)
- goto got_it;
- if (go_left && zoneno == 0)
- goto got_it;
- }
for (i = 0; i < NR_CURSEG_TYPE; i++)
if (CURSEG_I(sbi, i)->zone == zoneno)
break;

if (i < NR_CURSEG_TYPE) {
/* zone is in user, try another */
- if (go_left)
- hint = zoneno * sbi->secs_per_zone - 1;
- else if (zoneno + 1 >= total_zones)
+ if (zoneno + 1 >= total_zones)
hint = 0;
else
hint = (zoneno + 1) * sbi->secs_per_zone;
@@ -2763,8 +2733,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
return 0;

- if (test_opt(sbi, NOHEAP) &&
- (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
+ if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
return 0;

if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
@@ -2784,21 +2753,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
{
struct curseg_info *curseg = CURSEG_I(sbi, type);
- unsigned short seg_type = curseg->seg_type;
unsigned int segno = curseg->segno;
- int dir = ALLOC_LEFT;

if (curseg->inited)
- write_sum_page(sbi, curseg->sum_blk,
- GET_SUM_BLOCK(sbi, segno));
- if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
- dir = ALLOC_RIGHT;
-
- if (test_opt(sbi, NOHEAP))
- dir = ALLOC_RIGHT;
-
+ write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
segno = __get_next_segno(sbi, type);
- get_new_segment(sbi, &segno, new_sec, dir);
+ get_new_segment(sbi, &segno, new_sec);
curseg->next_segno = segno;
reset_curseg(sbi, type, 1);
curseg->alloc_type = LFS;
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 6fadfb634aac..0f1a5210c163 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -131,16 +131,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
#define SECTOR_TO_BLOCK(sectors) \
((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)

-/*
- * indicate a block allocation direction: RIGHT and LEFT.
- * RIGHT means allocating new sections towards the end of volume.
- * LEFT means the opposite direction.
- */
-enum {
- ALLOC_RIGHT = 0,
- ALLOC_LEFT
-};
-
/*
* In the victim_sel_policy->alloc_mode, there are three block allocation modes.
* LFS writes data sequentially with cleaning operations.
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 5aaf44c4698b..916e82e9c307 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -733,10 +733,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
clear_opt(sbi, DISCARD);
break;
case Opt_noheap:
- set_opt(sbi, NOHEAP);
- break;
case Opt_heap:
- clear_opt(sbi, NOHEAP);
+ f2fs_warn(sbi, "heap/no_heap options were deprecated");
break;
#ifdef CONFIG_F2FS_FS_XATTR
case Opt_user_xattr:
@@ -1962,10 +1960,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
} else {
seq_puts(seq, ",nodiscard");
}
- if (test_opt(sbi, NOHEAP))
- seq_puts(seq, ",no_heap");
- else
- seq_puts(seq, ",heap");
#ifdef CONFIG_F2FS_FS_XATTR
if (test_opt(sbi, XATTR_USER))
seq_puts(seq, ",user_xattr");
@@ -2142,7 +2136,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
set_opt(sbi, INLINE_XATTR);
set_opt(sbi, INLINE_DATA);
set_opt(sbi, INLINE_DENTRY);
- set_opt(sbi, NOHEAP);
set_opt(sbi, MERGE_CHECKPOINT);
F2FS_OPTION(sbi).unusable_cap = 0;
sbi->sb->s_flags |= SB_LAZYTIME;
--
2.44.0.rc0.258.g7320e95886-goog


2024-02-21 01:17:10

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: kill heap-based allocation

On 2024/2/21 4:51, Jaegeuk Kim wrote:
> No one uses this feature. Let's kill it.
>
> Reviewed-by: Daeho Jeong <[email protected]>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
>
> Change log from v1:
> - keep mount options but give warnings instead
>
> Documentation/filesystems/f2fs.rst | 4 +--
> fs/f2fs/gc.c | 5 ++-
> fs/f2fs/segment.c | 54 ++++--------------------------
> fs/f2fs/segment.h | 10 ------
> fs/f2fs/super.c | 9 +----
> 5 files changed, 11 insertions(+), 71 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 9ac5083dae8e..c30a800604fd 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -126,9 +126,7 @@ norecovery Disable the roll-forward recovery routine, mounted read-
> discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
> enabled, f2fs will issue discard/TRIM commands when a
> segment is cleaned.
> -no_heap Disable heap-style segment allocation which finds free
> - segments for data from the beginning of main area, while
> - for node from the end of main area.
> +no_heap Deprecated.

heap/no_heap Deprecated

Otherwise, it looks good to me.

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

BTW, do we need to kill heap-based allocation in mkfs.f2fs as well?

Thanks,

> nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
> by default if CONFIG_F2FS_FS_XATTR is selected.
> noacl Disable POSIX Access Control List. Note: acl is enabled
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 501b93b45b6c..a089a938355b 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
> p->max_search > sbi->max_victim_search)
> p->max_search = sbi->max_victim_search;
>
> - /* let's select beginning hot/small space first in no_heap mode*/
> + /* let's select beginning hot/small space first. */
> if (f2fs_need_rand_seg(sbi))
> p->offset = get_random_u32_below(MAIN_SECS(sbi) *
> SEGS_PER_SEC(sbi));
> - else if (test_opt(sbi, NOHEAP) &&
> - (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
> + else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
> p->offset = 0;
> else
> p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 94c4f7b16c19..09af17af4e7a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2634,16 +2634,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
> * This function should be returned with success, otherwise BUG
> */
> static void get_new_segment(struct f2fs_sb_info *sbi,
> - unsigned int *newseg, bool new_sec, int dir)
> + unsigned int *newseg, bool new_sec)
> {
> struct free_segmap_info *free_i = FREE_I(sbi);
> unsigned int segno, secno, zoneno;
> unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
> unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
> unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
> - unsigned int left_start = hint;
> bool init = true;
> - int go_left = 0;
> int i;
>
> spin_lock(&free_i->segmap_lock);
> @@ -2657,30 +2655,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> find_other_zone:
> secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
> if (secno >= MAIN_SECS(sbi)) {
> - if (dir == ALLOC_RIGHT) {
> - secno = find_first_zero_bit(free_i->free_secmap,
> + secno = find_first_zero_bit(free_i->free_secmap,
> MAIN_SECS(sbi));
> - f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> - } else {
> - go_left = 1;
> - left_start = hint - 1;
> - }
> - }
> - if (go_left == 0)
> - goto skip_left;
> -
> - while (test_bit(left_start, free_i->free_secmap)) {
> - if (left_start > 0) {
> - left_start--;
> - continue;
> - }
> - left_start = find_first_zero_bit(free_i->free_secmap,
> - MAIN_SECS(sbi));
> - f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
> - break;
> + f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> }
> - secno = left_start;
> -skip_left:
> segno = GET_SEG_FROM_SEC(sbi, secno);
> zoneno = GET_ZONE_FROM_SEC(sbi, secno);
>
> @@ -2691,21 +2669,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> goto got_it;
> if (zoneno == old_zoneno)
> goto got_it;
> - if (dir == ALLOC_LEFT) {
> - if (!go_left && zoneno + 1 >= total_zones)
> - goto got_it;
> - if (go_left && zoneno == 0)
> - goto got_it;
> - }
> for (i = 0; i < NR_CURSEG_TYPE; i++)
> if (CURSEG_I(sbi, i)->zone == zoneno)
> break;
>
> if (i < NR_CURSEG_TYPE) {
> /* zone is in user, try another */
> - if (go_left)
> - hint = zoneno * sbi->secs_per_zone - 1;
> - else if (zoneno + 1 >= total_zones)
> + if (zoneno + 1 >= total_zones)
> hint = 0;
> else
> hint = (zoneno + 1) * sbi->secs_per_zone;
> @@ -2763,8 +2733,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
> return 0;
>
> - if (test_opt(sbi, NOHEAP) &&
> - (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
> + if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
> return 0;
>
> if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
> @@ -2784,21 +2753,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
> {
> struct curseg_info *curseg = CURSEG_I(sbi, type);
> - unsigned short seg_type = curseg->seg_type;
> unsigned int segno = curseg->segno;
> - int dir = ALLOC_LEFT;
>
> if (curseg->inited)
> - write_sum_page(sbi, curseg->sum_blk,
> - GET_SUM_BLOCK(sbi, segno));
> - if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
> - dir = ALLOC_RIGHT;
> -
> - if (test_opt(sbi, NOHEAP))
> - dir = ALLOC_RIGHT;
> -
> + write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
> segno = __get_next_segno(sbi, type);
> - get_new_segment(sbi, &segno, new_sec, dir);
> + get_new_segment(sbi, &segno, new_sec);
> curseg->next_segno = segno;
> reset_curseg(sbi, type, 1);
> curseg->alloc_type = LFS;
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 6fadfb634aac..0f1a5210c163 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -131,16 +131,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> #define SECTOR_TO_BLOCK(sectors) \
> ((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>
> -/*
> - * indicate a block allocation direction: RIGHT and LEFT.
> - * RIGHT means allocating new sections towards the end of volume.
> - * LEFT means the opposite direction.
> - */
> -enum {
> - ALLOC_RIGHT = 0,
> - ALLOC_LEFT
> -};
> -
> /*
> * In the victim_sel_policy->alloc_mode, there are three block allocation modes.
> * LFS writes data sequentially with cleaning operations.
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 5aaf44c4698b..916e82e9c307 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -733,10 +733,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> clear_opt(sbi, DISCARD);
> break;
> case Opt_noheap:
> - set_opt(sbi, NOHEAP);
> - break;
> case Opt_heap:
> - clear_opt(sbi, NOHEAP);
> + f2fs_warn(sbi, "heap/no_heap options were deprecated");
> break;
> #ifdef CONFIG_F2FS_FS_XATTR
> case Opt_user_xattr:
> @@ -1962,10 +1960,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> } else {
> seq_puts(seq, ",nodiscard");
> }
> - if (test_opt(sbi, NOHEAP))
> - seq_puts(seq, ",no_heap");
> - else
> - seq_puts(seq, ",heap");
> #ifdef CONFIG_F2FS_FS_XATTR
> if (test_opt(sbi, XATTR_USER))
> seq_puts(seq, ",user_xattr");
> @@ -2142,7 +2136,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
> set_opt(sbi, INLINE_XATTR);
> set_opt(sbi, INLINE_DATA);
> set_opt(sbi, INLINE_DENTRY);
> - set_opt(sbi, NOHEAP);
> set_opt(sbi, MERGE_CHECKPOINT);
> F2FS_OPTION(sbi).unusable_cap = 0;
> sbi->sb->s_flags |= SB_LAZYTIME;

2024-02-21 01:57:25

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: kill heap-based allocation

On 02/21, Chao Yu wrote:
> On 2024/2/21 4:51, Jaegeuk Kim wrote:
> > No one uses this feature. Let's kill it.
> >
> > Reviewed-by: Daeho Jeong <[email protected]>
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> >
> > Change log from v1:
> > - keep mount options but give warnings instead
> >
> > Documentation/filesystems/f2fs.rst | 4 +--
> > fs/f2fs/gc.c | 5 ++-
> > fs/f2fs/segment.c | 54 ++++--------------------------
> > fs/f2fs/segment.h | 10 ------
> > fs/f2fs/super.c | 9 +----
> > 5 files changed, 11 insertions(+), 71 deletions(-)
> >
> > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> > index 9ac5083dae8e..c30a800604fd 100644
> > --- a/Documentation/filesystems/f2fs.rst
> > +++ b/Documentation/filesystems/f2fs.rst
> > @@ -126,9 +126,7 @@ norecovery Disable the roll-forward recovery routine, mounted read-
> > discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
> > enabled, f2fs will issue discard/TRIM commands when a
> > segment is cleaned.
> > -no_heap Disable heap-style segment allocation which finds free
> > - segments for data from the beginning of main area, while
> > - for node from the end of main area.
> > +no_heap Deprecated.
>
> heap/no_heap Deprecated
>
> Otherwise, it looks good to me.
>
> Reviewed-by: Chao Yu <[email protected]>
>
> BTW, do we need to kill heap-based allocation in mkfs.f2fs as well?

I was about to work on it after kernel change tho. Can you write one? :)

>
> Thanks,
>
> > nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
> > by default if CONFIG_F2FS_FS_XATTR is selected.
> > noacl Disable POSIX Access Control List. Note: acl is enabled
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 501b93b45b6c..a089a938355b 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
> > p->max_search > sbi->max_victim_search)
> > p->max_search = sbi->max_victim_search;
> > - /* let's select beginning hot/small space first in no_heap mode*/
> > + /* let's select beginning hot/small space first. */
> > if (f2fs_need_rand_seg(sbi))
> > p->offset = get_random_u32_below(MAIN_SECS(sbi) *
> > SEGS_PER_SEC(sbi));
> > - else if (test_opt(sbi, NOHEAP) &&
> > - (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
> > + else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
> > p->offset = 0;
> > else
> > p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 94c4f7b16c19..09af17af4e7a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -2634,16 +2634,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
> > * This function should be returned with success, otherwise BUG
> > */
> > static void get_new_segment(struct f2fs_sb_info *sbi,
> > - unsigned int *newseg, bool new_sec, int dir)
> > + unsigned int *newseg, bool new_sec)
> > {
> > struct free_segmap_info *free_i = FREE_I(sbi);
> > unsigned int segno, secno, zoneno;
> > unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
> > unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
> > unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
> > - unsigned int left_start = hint;
> > bool init = true;
> > - int go_left = 0;
> > int i;
> > spin_lock(&free_i->segmap_lock);
> > @@ -2657,30 +2655,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> > find_other_zone:
> > secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
> > if (secno >= MAIN_SECS(sbi)) {
> > - if (dir == ALLOC_RIGHT) {
> > - secno = find_first_zero_bit(free_i->free_secmap,
> > + secno = find_first_zero_bit(free_i->free_secmap,
> > MAIN_SECS(sbi));
> > - f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> > - } else {
> > - go_left = 1;
> > - left_start = hint - 1;
> > - }
> > - }
> > - if (go_left == 0)
> > - goto skip_left;
> > -
> > - while (test_bit(left_start, free_i->free_secmap)) {
> > - if (left_start > 0) {
> > - left_start--;
> > - continue;
> > - }
> > - left_start = find_first_zero_bit(free_i->free_secmap,
> > - MAIN_SECS(sbi));
> > - f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
> > - break;
> > + f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
> > }
> > - secno = left_start;
> > -skip_left:
> > segno = GET_SEG_FROM_SEC(sbi, secno);
> > zoneno = GET_ZONE_FROM_SEC(sbi, secno);
> > @@ -2691,21 +2669,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
> > goto got_it;
> > if (zoneno == old_zoneno)
> > goto got_it;
> > - if (dir == ALLOC_LEFT) {
> > - if (!go_left && zoneno + 1 >= total_zones)
> > - goto got_it;
> > - if (go_left && zoneno == 0)
> > - goto got_it;
> > - }
> > for (i = 0; i < NR_CURSEG_TYPE; i++)
> > if (CURSEG_I(sbi, i)->zone == zoneno)
> > break;
> > if (i < NR_CURSEG_TYPE) {
> > /* zone is in user, try another */
> > - if (go_left)
> > - hint = zoneno * sbi->secs_per_zone - 1;
> > - else if (zoneno + 1 >= total_zones)
> > + if (zoneno + 1 >= total_zones)
> > hint = 0;
> > else
> > hint = (zoneno + 1) * sbi->secs_per_zone;
> > @@ -2763,8 +2733,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> > if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
> > return 0;
> > - if (test_opt(sbi, NOHEAP) &&
> > - (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
> > + if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
> > return 0;
> > if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
> > @@ -2784,21 +2753,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
> > static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
> > {
> > struct curseg_info *curseg = CURSEG_I(sbi, type);
> > - unsigned short seg_type = curseg->seg_type;
> > unsigned int segno = curseg->segno;
> > - int dir = ALLOC_LEFT;
> > if (curseg->inited)
> > - write_sum_page(sbi, curseg->sum_blk,
> > - GET_SUM_BLOCK(sbi, segno));
> > - if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
> > - dir = ALLOC_RIGHT;
> > -
> > - if (test_opt(sbi, NOHEAP))
> > - dir = ALLOC_RIGHT;
> > -
> > + write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
> > segno = __get_next_segno(sbi, type);
> > - get_new_segment(sbi, &segno, new_sec, dir);
> > + get_new_segment(sbi, &segno, new_sec);
> > curseg->next_segno = segno;
> > reset_curseg(sbi, type, 1);
> > curseg->alloc_type = LFS;
> > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > index 6fadfb634aac..0f1a5210c163 100644
> > --- a/fs/f2fs/segment.h
> > +++ b/fs/f2fs/segment.h
> > @@ -131,16 +131,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> > #define SECTOR_TO_BLOCK(sectors) \
> > ((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
> > -/*
> > - * indicate a block allocation direction: RIGHT and LEFT.
> > - * RIGHT means allocating new sections towards the end of volume.
> > - * LEFT means the opposite direction.
> > - */
> > -enum {
> > - ALLOC_RIGHT = 0,
> > - ALLOC_LEFT
> > -};
> > -
> > /*
> > * In the victim_sel_policy->alloc_mode, there are three block allocation modes.
> > * LFS writes data sequentially with cleaning operations.
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 5aaf44c4698b..916e82e9c307 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -733,10 +733,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> > clear_opt(sbi, DISCARD);
> > break;
> > case Opt_noheap:
> > - set_opt(sbi, NOHEAP);
> > - break;
> > case Opt_heap:
> > - clear_opt(sbi, NOHEAP);
> > + f2fs_warn(sbi, "heap/no_heap options were deprecated");
> > break;
> > #ifdef CONFIG_F2FS_FS_XATTR
> > case Opt_user_xattr:
> > @@ -1962,10 +1960,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> > } else {
> > seq_puts(seq, ",nodiscard");
> > }
> > - if (test_opt(sbi, NOHEAP))
> > - seq_puts(seq, ",no_heap");
> > - else
> > - seq_puts(seq, ",heap");
> > #ifdef CONFIG_F2FS_FS_XATTR
> > if (test_opt(sbi, XATTR_USER))
> > seq_puts(seq, ",user_xattr");
> > @@ -2142,7 +2136,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
> > set_opt(sbi, INLINE_XATTR);
> > set_opt(sbi, INLINE_DATA);
> > set_opt(sbi, INLINE_DENTRY);
> > - set_opt(sbi, NOHEAP);
> > set_opt(sbi, MERGE_CHECKPOINT);
> > F2FS_OPTION(sbi).unusable_cap = 0;
> > sbi->sb->s_flags |= SB_LAZYTIME;

2024-02-21 02:18:30

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: kill heap-based allocation

On 2024/2/21 9:57, Jaegeuk Kim wrote:
> On 02/21, Chao Yu wrote:
>> On 2024/2/21 4:51, Jaegeuk Kim wrote:
>>> No one uses this feature. Let's kill it.
>>>
>>> Reviewed-by: Daeho Jeong <[email protected]>
>>> Signed-off-by: Jaegeuk Kim <[email protected]>
>>> ---
>>>
>>> Change log from v1:
>>> - keep mount options but give warnings instead
>>>
>>> Documentation/filesystems/f2fs.rst | 4 +--
>>> fs/f2fs/gc.c | 5 ++-
>>> fs/f2fs/segment.c | 54 ++++--------------------------
>>> fs/f2fs/segment.h | 10 ------
>>> fs/f2fs/super.c | 9 +----
>>> 5 files changed, 11 insertions(+), 71 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
>>> index 9ac5083dae8e..c30a800604fd 100644
>>> --- a/Documentation/filesystems/f2fs.rst
>>> +++ b/Documentation/filesystems/f2fs.rst
>>> @@ -126,9 +126,7 @@ norecovery Disable the roll-forward recovery routine, mounted read-
>>> discard/nodiscard Enable/disable real-time discard in f2fs, if discard is
>>> enabled, f2fs will issue discard/TRIM commands when a
>>> segment is cleaned.
>>> -no_heap Disable heap-style segment allocation which finds free
>>> - segments for data from the beginning of main area, while
>>> - for node from the end of main area.
>>> +no_heap Deprecated.
>>
>> heap/no_heap Deprecated
>>
>> Otherwise, it looks good to me.
>>
>> Reviewed-by: Chao Yu <[email protected]>
>>
>> BTW, do we need to kill heap-based allocation in mkfs.f2fs as well?
>
> I was about to work on it after kernel change tho. Can you write one? :)

No problem.

Thanks,

>
>>
>> Thanks,
>>
>>> nouser_xattr Disable Extended User Attributes. Note: xattr is enabled
>>> by default if CONFIG_F2FS_FS_XATTR is selected.
>>> noacl Disable POSIX Access Control List. Note: acl is enabled
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 501b93b45b6c..a089a938355b 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -280,12 +280,11 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
>>> p->max_search > sbi->max_victim_search)
>>> p->max_search = sbi->max_victim_search;
>>> - /* let's select beginning hot/small space first in no_heap mode*/
>>> + /* let's select beginning hot/small space first. */
>>> if (f2fs_need_rand_seg(sbi))
>>> p->offset = get_random_u32_below(MAIN_SECS(sbi) *
>>> SEGS_PER_SEC(sbi));
>>> - else if (test_opt(sbi, NOHEAP) &&
>>> - (type == CURSEG_HOT_DATA || IS_NODESEG(type)))
>>> + else if (type == CURSEG_HOT_DATA || IS_NODESEG(type))
>>> p->offset = 0;
>>> else
>>> p->offset = SIT_I(sbi)->last_victim[p->gc_mode];
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 94c4f7b16c19..09af17af4e7a 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -2634,16 +2634,14 @@ static int is_next_segment_free(struct f2fs_sb_info *sbi,
>>> * This function should be returned with success, otherwise BUG
>>> */
>>> static void get_new_segment(struct f2fs_sb_info *sbi,
>>> - unsigned int *newseg, bool new_sec, int dir)
>>> + unsigned int *newseg, bool new_sec)
>>> {
>>> struct free_segmap_info *free_i = FREE_I(sbi);
>>> unsigned int segno, secno, zoneno;
>>> unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
>>> unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
>>> unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
>>> - unsigned int left_start = hint;
>>> bool init = true;
>>> - int go_left = 0;
>>> int i;
>>> spin_lock(&free_i->segmap_lock);
>>> @@ -2657,30 +2655,10 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
>>> find_other_zone:
>>> secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
>>> if (secno >= MAIN_SECS(sbi)) {
>>> - if (dir == ALLOC_RIGHT) {
>>> - secno = find_first_zero_bit(free_i->free_secmap,
>>> + secno = find_first_zero_bit(free_i->free_secmap,
>>> MAIN_SECS(sbi));
>>> - f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
>>> - } else {
>>> - go_left = 1;
>>> - left_start = hint - 1;
>>> - }
>>> - }
>>> - if (go_left == 0)
>>> - goto skip_left;
>>> -
>>> - while (test_bit(left_start, free_i->free_secmap)) {
>>> - if (left_start > 0) {
>>> - left_start--;
>>> - continue;
>>> - }
>>> - left_start = find_first_zero_bit(free_i->free_secmap,
>>> - MAIN_SECS(sbi));
>>> - f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
>>> - break;
>>> + f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
>>> }
>>> - secno = left_start;
>>> -skip_left:
>>> segno = GET_SEG_FROM_SEC(sbi, secno);
>>> zoneno = GET_ZONE_FROM_SEC(sbi, secno);
>>> @@ -2691,21 +2669,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
>>> goto got_it;
>>> if (zoneno == old_zoneno)
>>> goto got_it;
>>> - if (dir == ALLOC_LEFT) {
>>> - if (!go_left && zoneno + 1 >= total_zones)
>>> - goto got_it;
>>> - if (go_left && zoneno == 0)
>>> - goto got_it;
>>> - }
>>> for (i = 0; i < NR_CURSEG_TYPE; i++)
>>> if (CURSEG_I(sbi, i)->zone == zoneno)
>>> break;
>>> if (i < NR_CURSEG_TYPE) {
>>> /* zone is in user, try another */
>>> - if (go_left)
>>> - hint = zoneno * sbi->secs_per_zone - 1;
>>> - else if (zoneno + 1 >= total_zones)
>>> + if (zoneno + 1 >= total_zones)
>>> hint = 0;
>>> else
>>> hint = (zoneno + 1) * sbi->secs_per_zone;
>>> @@ -2763,8 +2733,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
>>> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
>>> return 0;
>>> - if (test_opt(sbi, NOHEAP) &&
>>> - (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type)))
>>> + if (seg_type == CURSEG_HOT_DATA || IS_NODESEG(seg_type))
>>> return 0;
>>> if (SIT_I(sbi)->last_victim[ALLOC_NEXT])
>>> @@ -2784,21 +2753,12 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
>>> static void new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
>>> {
>>> struct curseg_info *curseg = CURSEG_I(sbi, type);
>>> - unsigned short seg_type = curseg->seg_type;
>>> unsigned int segno = curseg->segno;
>>> - int dir = ALLOC_LEFT;
>>> if (curseg->inited)
>>> - write_sum_page(sbi, curseg->sum_blk,
>>> - GET_SUM_BLOCK(sbi, segno));
>>> - if (seg_type == CURSEG_WARM_DATA || seg_type == CURSEG_COLD_DATA)
>>> - dir = ALLOC_RIGHT;
>>> -
>>> - if (test_opt(sbi, NOHEAP))
>>> - dir = ALLOC_RIGHT;
>>> -
>>> + write_sum_page(sbi, curseg->sum_blk, GET_SUM_BLOCK(sbi, segno));
>>> segno = __get_next_segno(sbi, type);
>>> - get_new_segment(sbi, &segno, new_sec, dir);
>>> + get_new_segment(sbi, &segno, new_sec);
>>> curseg->next_segno = segno;
>>> reset_curseg(sbi, type, 1);
>>> curseg->alloc_type = LFS;
>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>> index 6fadfb634aac..0f1a5210c163 100644
>>> --- a/fs/f2fs/segment.h
>>> +++ b/fs/f2fs/segment.h
>>> @@ -131,16 +131,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>>> #define SECTOR_TO_BLOCK(sectors) \
>>> ((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>>> -/*
>>> - * indicate a block allocation direction: RIGHT and LEFT.
>>> - * RIGHT means allocating new sections towards the end of volume.
>>> - * LEFT means the opposite direction.
>>> - */
>>> -enum {
>>> - ALLOC_RIGHT = 0,
>>> - ALLOC_LEFT
>>> -};
>>> -
>>> /*
>>> * In the victim_sel_policy->alloc_mode, there are three block allocation modes.
>>> * LFS writes data sequentially with cleaning operations.
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 5aaf44c4698b..916e82e9c307 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -733,10 +733,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>> clear_opt(sbi, DISCARD);
>>> break;
>>> case Opt_noheap:
>>> - set_opt(sbi, NOHEAP);
>>> - break;
>>> case Opt_heap:
>>> - clear_opt(sbi, NOHEAP);
>>> + f2fs_warn(sbi, "heap/no_heap options were deprecated");
>>> break;
>>> #ifdef CONFIG_F2FS_FS_XATTR
>>> case Opt_user_xattr:
>>> @@ -1962,10 +1960,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
>>> } else {
>>> seq_puts(seq, ",nodiscard");
>>> }
>>> - if (test_opt(sbi, NOHEAP))
>>> - seq_puts(seq, ",no_heap");
>>> - else
>>> - seq_puts(seq, ",heap");
>>> #ifdef CONFIG_F2FS_FS_XATTR
>>> if (test_opt(sbi, XATTR_USER))
>>> seq_puts(seq, ",user_xattr");
>>> @@ -2142,7 +2136,6 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
>>> set_opt(sbi, INLINE_XATTR);
>>> set_opt(sbi, INLINE_DATA);
>>> set_opt(sbi, INLINE_DENTRY);
>>> - set_opt(sbi, NOHEAP);
>>> set_opt(sbi, MERGE_CHECKPOINT);
>>> F2FS_OPTION(sbi).unusable_cap = 0;
>>> sbi->sb->s_flags |= SB_LAZYTIME;