| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250507090312.614-1-yohan.joung@sk.com>
Date: Wed, 7 May 2025 18:03:12 +0900
From: "yohan.joung" <yohan.joung@...com>
To: linux-f2fs-devel@...ts.sourceforge.net
Cc: chao@...nel.org,
daehojeong@...gle.com,
jaegeuk@...nel.org,
linux-kernel@...r.kernel.org,
pilhyun.kim@...com,
yohan.joung@...com
Subject: Re: [f2fs-dev] [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry
>On 5/7/25 12:30, yohan.joung wrote:
>> when performing buffered writes in a large section,
>> overhead is incurred due to the iteration through
>> ckpt_valid_blocks within the section.
>> when SEGS_PER_SEC is 128, this overhead accounts for 20% within
>> the f2fs_write_single_data_page routine.
>> as the size of the section increases, the overhead also grows.
>> to handle this problem ckpt_valid_blocks is
>> added within the section entries.
>>
>> Test
>> insmod null_blk.ko nr_devices=1 completion_nsec=1 submit_queues=8
>> hw_queue_depth=64 max_sectors=512 bs=4096 memory_backed=1
>> make_f2fs /dev/block/nullb0
>> make_f2fs -s 128 /dev/block/nullb0
>> fio --bs=512k --size=1536M --rw=write --name=1
>> --filename=/mnt/test_dir/seq_write
>> --ioengine=io_uring --iodepth=64 --end_fsync=1
>>
>> before
>> SEGS_PER_SEC 1
>> 2556MiB/s
>> SEGS_PER_SEC 128
>> 2145MiB/s
>>
>> after
>> SEGS_PER_SEC 1
>> 2556MiB/s
>> SEGS_PER_SEC 128
>> 2556MiB/s
>>
>> Signed-off-by: yohan.joung <yohan.joung@...com>
>> ---
>> fs/f2fs/segment.c | 29 ++++++++++++++++++++++-------
>> fs/f2fs/segment.h | 29 ++++++++++++++++++-----------
>> 2 files changed, 40 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 671bc5a8fd4a..09b66a755559 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -2447,7 +2447,7 @@ static void update_segment_mtime(struct f2fs_sb_info *sbi, block_t blkaddr,
>> * that the consecutive input blocks belong to the same segment.
>> */
>> static int update_sit_entry_for_release(struct f2fs_sb_info *sbi, struct seg_entry *se,
>> - block_t blkaddr, unsigned int offset, int del)
>> + unsigned int segno, block_t blkaddr, unsigned int offset, int del)
>> {
>> bool exist;
>> #ifdef CONFIG_F2FS_CHECK_FS
>> @@ -2492,15 +2492,18 @@ static int update_sit_entry_for_release(struct f2fs_sb_info *sbi, struct seg_ent
>> f2fs_test_and_clear_bit(offset + i, se->discard_map))
>> sbi->discard_blks++;
>>
>> - if (!f2fs_test_bit(offset + i, se->ckpt_valid_map))
>> + if (!f2fs_test_bit(offset + i, se->ckpt_valid_map)) {
>> se->ckpt_valid_blocks -= 1;
>> + if (__is_large_section(sbi))
>> + get_sec_entry(sbi, segno)->ckpt_valid_blocks -= 1;
>> + }
>> }
>>
>> return del;
>> }
>>
>> static int update_sit_entry_for_alloc(struct f2fs_sb_info *sbi, struct seg_entry *se,
>> - block_t blkaddr, unsigned int offset, int del)
>> + unsigned int segno, block_t blkaddr, unsigned int offset, int del)
>> {
>> bool exist;
>> #ifdef CONFIG_F2FS_CHECK_FS
>> @@ -2533,12 +2536,18 @@ static int update_sit_entry_for_alloc(struct f2fs_sb_info *sbi, struct seg_entry
>> * or newly invalidated.
>> */
>> if (!is_sbi_flag_set(sbi, SBI_CP_DISABLED)) {
>> - if (!f2fs_test_and_set_bit(offset, se->ckpt_valid_map))
>> + if (!f2fs_test_and_set_bit(offset, se->ckpt_valid_map)) {
>> se->ckpt_valid_blocks++;
>> + if (__is_large_section(sbi))
>> + get_sec_entry(sbi, segno)->ckpt_valid_blocks++;
>
>What about adding sanity_check_valid_blocks() to check consistency between
>valid blocks of segments and valid blocks of section? Covered w/
>CONFIG_F2FS_CHECK_FS.
I will add it, thanks
>
>> + }
>> }
>>
>> - if (!f2fs_test_bit(offset, se->ckpt_valid_map))
>> + if (!f2fs_test_bit(offset, se->ckpt_valid_map)) {
>> se->ckpt_valid_blocks += del;
>> + if (__is_large_section(sbi))
>> + get_sec_entry(sbi, segno)->ckpt_valid_blocks += del;
>> + }
>>
>> return del;
>> }
>> @@ -2569,9 +2578,9 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
>>
>> /* Update valid block bitmap */
>> if (del > 0) {
>> - del = update_sit_entry_for_alloc(sbi, se, blkaddr, offset, del);
>> + del = update_sit_entry_for_alloc(sbi, se, segno, blkaddr, offset, del);
>> } else {
>> - del = update_sit_entry_for_release(sbi, se, blkaddr, offset, del);
>> + del = update_sit_entry_for_release(sbi, se, segno, blkaddr, offset, del);
>> }
>>
>> __mark_sit_entry_dirty(sbi, segno);
>> @@ -5029,6 +5038,12 @@ static int build_sit_entries(struct f2fs_sb_info *sbi)
>> }
>> up_read(&curseg->journal_rwsem);
>>
>> + /* update ckpt_ckpt_valid_block */
>> + if (__is_large_section(sbi)) {
>> + for (unsigned int segno = 0; segno < MAIN_SEGS(sbi); segno += SEGS_PER_SEC(sbi))
>
> unsigned int segno;
>
> for (segno = 0; segno < MAIN_SEGS(sbi);
> segno += SEGS_PER_SEC(sbi))
>
>> + set_ckpt_valid_blocks(sbi, segno);
>
>ckpt_valid_blocks will be reset in f2fs_flush_sit_entries -> seg_info_to_raw_sit,
>we need to call set_ckpt_valid_blocks() for each updated section?
>
>Thanks,
going to add set_ckpt_valid_blocks to seg_info_to_raw_sit
Thanks.
>
>> + }
>> +
>> if (err)
>> return err;
>>
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index f5d30f32ebdb..e91d944f003a 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -211,6 +211,7 @@ struct seg_entry {
>>
>> struct sec_entry {
>> unsigned int valid_blocks; /* # of valid blocks in a section */
>> + unsigned int ckpt_valid_blocks; /* # of valid blocks last cp in a section */
>> };
>>
>> #define MAX_SKIP_GC_COUNT 16
>> @@ -347,20 +348,26 @@ static inline unsigned int get_valid_blocks(struct f2fs_sb_info *sbi,
>> static inline unsigned int get_ckpt_valid_blocks(struct f2fs_sb_info *sbi,
>> unsigned int segno, bool use_section)
>> {
>> - if (use_section && __is_large_section(sbi)) {
>> - unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
>> - unsigned int start_segno = GET_SEG_FROM_SEC(sbi, secno);
>> - unsigned int blocks = 0;
>> - int i;
>> + if (use_section && __is_large_section(sbi))
>> + return get_sec_entry(sbi, segno)->ckpt_valid_blocks;
>> + else
>> + return get_seg_entry(sbi, segno)->ckpt_valid_blocks;
>> +}
>> +
>> +static inline void set_ckpt_valid_blocks(struct f2fs_sb_info *sbi,
>> + unsigned int segno)
>> +{
>> + unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
>> + unsigned int start_segno = GET_SEG_FROM_SEC(sbi, secno);
>> + unsigned int blocks = 0;
>> + int i;
>>
>> - for (i = 0; i < SEGS_PER_SEC(sbi); i++, start_segno++) {
>> - struct seg_entry *se = get_seg_entry(sbi, start_segno);
>> + for (i = 0; i < SEGS_PER_SEC(sbi); i++, start_segno++) {
>> + struct seg_entry *se = get_seg_entry(sbi, start_segno);
>>
>> - blocks += se->ckpt_valid_blocks;
>> - }
>> - return blocks;
>> + blocks += se->ckpt_valid_blocks;
>> }
>> - return get_seg_entry(sbi, segno)->ckpt_valid_blocks;
>> + get_sec_entry(sbi, segno)->ckpt_valid_blocks = blocks;
>> }
>>
>> static inline void seg_info_from_raw_sit(struct seg_entry *se,
Powered by blists - more mailing lists