From: Chao Yu <chao@kernel.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: kill heap-based allocation
Date: Wed, 21 Feb 2024 10:18:17 +0800 [thread overview]
Message-ID: <36ebed2d-36e9-4e0b-ba2c-f85ad35f4a7b@kernel.org> (raw)
In-Reply-To: <ZdVYfHg2R8B1Uj2r@google.com>
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 <daehojeong@google.com>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>
>>> 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 <chao@kernel.org>
>>
>> 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;
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
prev parent reply other threads:[~2024-02-21 2:18 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-07 16:46 [f2fs-dev] [PATCH] f2fs: kill heap-based allocation Jaegeuk Kim
2024-02-07 23:01 ` Daeho Jeong
2024-02-20 8:22 ` Chao Yu
2024-02-20 20:51 ` [f2fs-dev] [PATCH v2] " Jaegeuk Kim
2024-02-21 1:16 ` Chao Yu
2024-02-21 1:57 ` Jaegeuk Kim
2024-02-21 2:18 ` Chao Yu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=36ebed2d-36e9-4e0b-ba2c-f85ad35f4a7b@kernel.org \
--to=chao@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).