mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode.
@ 2025-05-07  4:30 yohan.joung
  2025-05-07  4:30 ` [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry yohan.joung
  2025-05-07  8:25 ` [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode Chao Yu
  0 siblings, 2 replies; 5+ messages in thread
From: yohan.joung @ 2025-05-07  4:30 UTC (permalink / raw)
  To: jaegeuk, chao, daehojeong
  Cc: linux-kernel, linux-f2fs-devel, pilhyun.kim, yohan.joung

In LFS mode, the previous segment cannot use invalid blocks,
so the remaining blocks from the next_blkoff of the current segment
to the end of the section are calculated.

Signed-off-by: yohan.joung <yohan.joung@sk.com>
---
 fs/f2fs/segment.h | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 03c0f59be5a8..f5d30f32ebdb 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -102,6 +102,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
 #define CAP_SEGS_PER_SEC(sbi)					\
 	(SEGS_PER_SEC(sbi) -					\
 	BLKS_TO_SEGS(sbi, (sbi)->unusable_blocks_per_sec))
+#define GET_START_SEG_FROM_SEC(sbi, segno)			\
+	(rounddown(segno, SEGS_PER_SEC(sbi)))
 #define GET_SEC_FROM_SEG(sbi, segno)				\
 	(((segno) == -1) ? -1 : (segno) / SEGS_PER_SEC(sbi))
 #define GET_SEG_FROM_SEC(sbi, secno)				\
@@ -582,8 +584,14 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
 		if (unlikely(segno == NULL_SEGNO))
 			return false;
 
-		left_blocks = CAP_BLKS_PER_SEC(sbi) -
-				get_ckpt_valid_blocks(sbi, segno, true);
+		if (f2fs_lfs_mode(sbi) && __is_large_section(sbi)) {
+			left_blocks = CAP_BLKS_PER_SEC(sbi) -
+				(segno - GET_START_SEG_FROM_SEC(sbi, segno)) * BLKS_PER_SEG(sbi) -
+				CURSEG_I(sbi, i)->next_blkoff;
+		} else {
+			left_blocks = CAP_BLKS_PER_SEC(sbi) -
+					get_ckpt_valid_blocks(sbi, segno, true);
+		}
 
 		blocks = i <= CURSEG_COLD_DATA ? data_blocks : node_blocks;
 		if (blocks > left_blocks)
@@ -596,8 +604,15 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
 	if (unlikely(segno == NULL_SEGNO))
 		return false;
 
-	left_blocks = CAP_BLKS_PER_SEC(sbi) -
-			get_ckpt_valid_blocks(sbi, segno, true);
+	if (f2fs_lfs_mode(sbi) && __is_large_section(sbi)) {
+		left_blocks = CAP_BLKS_PER_SEC(sbi) -
+				(segno - GET_START_SEG_FROM_SEC(sbi, segno)) * BLKS_PER_SEG(sbi) -
+				CURSEG_I(sbi, CURSEG_HOT_DATA)->next_blkoff;
+	} else {
+		left_blocks = CAP_BLKS_PER_SEC(sbi) -
+				get_ckpt_valid_blocks(sbi, segno, true);
+	}
+
 	if (dent_blocks > left_blocks)
 		return false;
 	return true;
-- 
2.33.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry
  2025-05-07  4:30 [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode yohan.joung
@ 2025-05-07  4:30 ` yohan.joung
  2025-05-07  8:47   ` Chao Yu
  2025-05-07  8:25 ` [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode Chao Yu
  1 sibling, 1 reply; 5+ messages in thread
From: yohan.joung @ 2025-05-07  4:30 UTC (permalink / raw)
  To: jaegeuk, chao, daehojeong
  Cc: linux-kernel, linux-f2fs-devel, pilhyun.kim, yohan.joung

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@sk.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++;
+		}
 	}
 
-	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))
+			set_ckpt_valid_blocks(sbi, segno);
+	}
+
 	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,
-- 
2.33.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode.
  2025-05-07  4:30 [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode yohan.joung
  2025-05-07  4:30 ` [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry yohan.joung
@ 2025-05-07  8:25 ` Chao Yu
  1 sibling, 0 replies; 5+ messages in thread
From: Chao Yu @ 2025-05-07  8:25 UTC (permalink / raw)
  To: yohan.joung, jaegeuk, daehojeong
  Cc: chao, linux-kernel, linux-f2fs-devel, pilhyun.kim

On 5/7/25 12:30, yohan.joung wrote:
> In LFS mode, the previous segment cannot use invalid blocks,
> so the remaining blocks from the next_blkoff of the current segment
> to the end of the section are calculated.
> 
> Signed-off-by: yohan.joung <yohan.joung@sk.com>
> ---
>  fs/f2fs/segment.h | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 03c0f59be5a8..f5d30f32ebdb 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -102,6 +102,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  #define CAP_SEGS_PER_SEC(sbi)					\
>  	(SEGS_PER_SEC(sbi) -					\
>  	BLKS_TO_SEGS(sbi, (sbi)->unusable_blocks_per_sec))
> +#define GET_START_SEG_FROM_SEC(sbi, segno)			\
> +	(rounddown(segno, SEGS_PER_SEC(sbi)))
>  #define GET_SEC_FROM_SEG(sbi, segno)				\
>  	(((segno) == -1) ? -1 : (segno) / SEGS_PER_SEC(sbi))
>  #define GET_SEG_FROM_SEC(sbi, secno)				\
> @@ -582,8 +584,14 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
>  		if (unlikely(segno == NULL_SEGNO))
>  			return false;
>  
> -		left_blocks = CAP_BLKS_PER_SEC(sbi) -
> -				get_ckpt_valid_blocks(sbi, segno, true);
> +		if (f2fs_lfs_mode(sbi) && __is_large_section(sbi)) {
> +			left_blocks = CAP_BLKS_PER_SEC(sbi) -
> +				(segno - GET_START_SEG_FROM_SEC(sbi, segno)) * BLKS_PER_SEG(sbi) -

SEGS_TO_BLKS(sbi, (segno - GET_START_SEG_FROM_SEC(sbi, segno))) ?

> +				CURSEG_I(sbi, i)->next_blkoff;
> +		} else {
> +			left_blocks = CAP_BLKS_PER_SEC(sbi) -
> +					get_ckpt_valid_blocks(sbi, segno, true);
> +		}
>  
>  		blocks = i <= CURSEG_COLD_DATA ? data_blocks : node_blocks;
>  		if (blocks > left_blocks)
> @@ -596,8 +604,15 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
>  	if (unlikely(segno == NULL_SEGNO))
>  		return false;
>  
> -	left_blocks = CAP_BLKS_PER_SEC(sbi) -
> -			get_ckpt_valid_blocks(sbi, segno, true);
> +	if (f2fs_lfs_mode(sbi) && __is_large_section(sbi)) {
> +		left_blocks = CAP_BLKS_PER_SEC(sbi) -
> +				(segno - GET_START_SEG_FROM_SEC(sbi, segno)) * BLKS_PER_SEG(sbi) -

Ditto,

> +				CURSEG_I(sbi, CURSEG_HOT_DATA)->next_blkoff;
> +	} else {
> +		left_blocks = CAP_BLKS_PER_SEC(sbi) -
> +				get_ckpt_valid_blocks(sbi, segno, true);
> +	}
> +
>  	if (dent_blocks > left_blocks)
>  		return false;
>  	return true;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry
  2025-05-07  4:30 ` [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry yohan.joung
@ 2025-05-07  8:47   ` Chao Yu
  2025-05-07  9:03     ` [f2fs-dev] " yohan.joung
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2025-05-07  8:47 UTC (permalink / raw)
  To: yohan.joung, jaegeuk, daehojeong
  Cc: chao, linux-kernel, linux-f2fs-devel, pilhyun.kim

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@sk.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.

> +		}
>  	}
>  
> -	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,

> +	}
> +
>  	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,


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [f2fs-dev] [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry
  2025-05-07  8:47   ` Chao Yu
@ 2025-05-07  9:03     ` yohan.joung
  0 siblings, 0 replies; 5+ messages in thread
From: yohan.joung @ 2025-05-07  9:03 UTC (permalink / raw)
  To: linux-f2fs-devel
  Cc: chao, daehojeong, jaegeuk, linux-kernel, pilhyun.kim, yohan.joung

>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@sk.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,

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-05-07  9:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-07  4:30 [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode yohan.joung
2025-05-07  4:30 ` [PATCH v3 2/2] f2fs: add ckpt_valid_blocks to the section entry yohan.joung
2025-05-07  8:47   ` Chao Yu
2025-05-07  9:03     ` [f2fs-dev] " yohan.joung
2025-05-07  8:25 ` [PATCH v3 1/2] f2fs: add a method for calculating the remaining blocks in the current segment in LFS mode Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®