Re: [f2fs-dev] [PATCH] mkfs.f2fs: update allocation policy for ro feature

[Chao Yu <chao@kernel.org>]

On 2022/10/3 9:06, Jaegeuk Kim wrote:
> On 10/01, Chao Yu wrote:
>> On 2022/10/1 6:33, Jaegeuk Kim wrote:
>>> On 09/30, Chao Yu wrote:
>>>> On 2022/9/30 2:27, Jaegeuk Kim wrote:
>>>>> On 09/28, Chao Yu wrote:
>>>>>> On 2022/9/28 11:15, Jaegeuk Kim wrote:
>>>>>>> On 09/28, Chao Yu wrote:
>>>>>>>> On 2022/9/28 0:40, Jaegeuk Kim wrote:
>>>>>>>>> On 09/25, Chao Yu wrote:
>>>>>>>>>> Update allocation policy for ro feature:
>>>>>>>>>> - hot_data: allocating blocks by LBA ascending order
>>>>>>>>>> - hot_node: allocating blocks by LBA descending order
>>>>>>>>>
>>>>>>>>> This will increase the RO image size.
>>>>>>>>
>>>>>>>> Shouldn't RO image has fixed-size during mkfs?
>>>>>>>
>>>>>>> First run gives the reduced size information, and second round makes the image
>>>>>>> with the required size.
>>>>>>
>>>>>> I didn't get it, in which step it may increase the RO image size?
>>>>>>
>>>>>> IIUC, after we apply this patch, reduced size information won't change due
>>>>>> to that after sload in first run, total used block count should be fixed?
>>>>>
>>>>> First run fills the data and leaves the maximum LBA touched in the image. Then,
>>>>
>>>> How about caclulating required size w/ total used blocks rather than maximum LBA?
>>>
>>> Do you think that can give the smallest size for the image?
>>
>> I guess so, but let me do some tests to check that.
>>
>> Segments for data block and node block should be calculated separately:
>> required_blks = roundup(data_blks, blks_per_seg) + roundup(node_blks, blks_per_seg)
>>
>> Am I missing something?
> 
> I think that'd be fine tho, please test first.

I did some tests, seems above method works... but original implementation
may have bug, could you please check below patch:

 From 757d8f5191e21065f0c914512c17f963e5a17945 Mon Sep 17 00:00:00 2001
From: Chao Yu <chao@kernel.org>
Date: Thu, 6 Oct 2022 22:09:38 +0800
Subject: [PATCH] sload.f2fs: fix to calculate max size correctly

Max image size should be calculated with round_up().

- dd if=/dev/zero of=img bs=1M count=512
- mkfs.f2fs -O ro -f img

a) sload.f2fs -f <kernel_path>/kernel/ img
b) sload.f2fs -f <kernel_path>/mm/ img
c) sload.f2fs -f <kernel_path>/include/ img

Before:
[FSCK] Max image size: 48 MB, Free space: 462 MB
[FSCK] Max image size: 25 MB, Free space: 485 MB
[FSCK] Max image size: 112 MB, Free space: 398 MB

After:
[FSCK] Max image size: 50 MB, Free space: 460 MB
[FSCK] Max image size: 26 MB, Free space: 484 MB
[FSCK] Max image size: 114 MB, Free space: 396 MB

Signed-off-by: Chao Yu <chao@kernel.org>
---
  fsck/f2fs.h        | 2 ++
  fsck/fsck.c        | 9 ++++++---
  include/f2fs_fs.h  | 6 ++++--
  mkfs/f2fs_format.c | 3 ++-
  4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/fsck/f2fs.h b/fsck/f2fs.h
index 030d750..88b98f9 100644
--- a/fsck/f2fs.h
+++ b/fsck/f2fs.h
@@ -420,6 +420,8 @@ static inline block_t __end_block_addr(struct f2fs_sb_info *sbi)
  	return end + le64_to_cpu(F2FS_RAW_SUPER(sbi)->block_count);
  }

+#define BLKS_PER_SEC(sbi)						\
+		((sbi)->segs_per_sec * (sbi)->blocks_per_seg)
  #define GET_ZONENO_FROM_SEGNO(sbi, segno)                               \
  	((segno / sbi->segs_per_sec) / sbi->secs_per_zone)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index b1b6722..f6ff986 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -3179,6 +3179,7 @@ int fsck_verify(struct f2fs_sb_info *sbi)
  	struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
  	struct hard_link_node *node = NULL;
  	bool verify_failed = false;
+	uint64_t max_size, free_size;

  	if (c.show_file_map)
  		return 0;
@@ -3229,10 +3230,12 @@ int fsck_verify(struct f2fs_sb_info *sbi)
  		}
  		c.bug_on = 1;
  	}
+
+	max_size = round_up(c.max_size, sbi->blocksize * BLKS_PER_SEC(sbi));
+	free_size = SM_I(sbi)->segment_count * sbi->blocks_per_seg *
+				sbi->blocksize - max_size;
  	printf("[FSCK] Max image size: %"PRIu64" MB, Free space: %u MB\n",
-		c.max_size >> 20,
-		(sbi->user_block_count - sbi->total_valid_block_count) >>
-		(20 - F2FS_BLKSIZE_BITS));
+					max_size >> 20, free_size >> 20);
  	printf("[FSCK] Unreachable nat entries                       ");
  	if (nr_unref_nid == 0x0) {
  		printf(" [Ok..] [0x%x]\n", nr_unref_nid);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 509d75a..0a99717 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -663,7 +663,8 @@ struct f2fs_configuration {
  	(void) (&_max1 == &_max2);		\
  	_max1 > _max2 ? _max1 : _max2; })

-#define round_up(x, y)		(((x) + (y) - 1) / (y))
+#define div_round_up(x, y)		(((x) + (y) - 1) / (y))
+#define round_up(x, y)			(div_round_up(x, y) * (y))
  /*
   * Copied from fs/f2fs/f2fs.h
   */
@@ -1592,7 +1593,8 @@ static inline double get_best_overprovision(struct f2fs_super_block *sb)

  	for (; candidate <= end; candidate += diff) {
  		reserved = (2 * (100 / candidate + 1) + 6) *
-				round_up(usable_main_segs, get_sb(section_count));
+				div_round_up(usable_main_segs,
+						get_sb(section_count));
  		ovp = (usable_main_segs - reserved) * candidate / 100;
  		space = usable_main_segs - reserved - ovp;
  		if (max_space < space) {
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 40ac589..14f5534 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -485,7 +485,8 @@ static int f2fs_prepare_super_block(void)

  	c.reserved_segments =
  			(2 * (100 / c.overprovision + 1) + NR_CURSEG_TYPE) *
-			round_up(f2fs_get_usable_segments(sb), get_sb(section_count));
+			div_round_up(f2fs_get_usable_segments(sb),
+						get_sb(section_count));

  	if (c.feature & cpu_to_le32(F2FS_FEATURE_RO)) {
  		c.overprovision = 0;
-- 
2.36.1

> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>> it'll resize the image file and run again with the smallest image.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>>>>>> ---
>>>>>>>>>>       mkfs/f2fs_format.c | 4 ++--
>>>>>>>>>>       1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>>>>>>>>>> index 40ac589..8d0f410 100644
>>>>>>>>>> --- a/mkfs/f2fs_format.c
>>>>>>>>>> +++ b/mkfs/f2fs_format.c
>>>>>>>>>> @@ -544,10 +544,10 @@ static int f2fs_prepare_super_block(void)
>>>>>>>>>>       	}
>>>>>>>>>>       	if (c.feature & cpu_to_le32(F2FS_FEATURE_RO)) {
>>>>>>>>>> -		c.cur_seg[CURSEG_HOT_NODE] = 0;
>>>>>>>>>> +		c.cur_seg[CURSEG_HOT_NODE] = last_section(last_zone(total_zones));
>>>>>>>>>>       		c.cur_seg[CURSEG_WARM_NODE] = 0;
>>>>>>>>>>       		c.cur_seg[CURSEG_COLD_NODE] = 0;
>>>>>>>>>> -		c.cur_seg[CURSEG_HOT_DATA] = 1;
>>>>>>>>>> +		c.cur_seg[CURSEG_HOT_DATA] = 0;
>>>>>>>>>>       		c.cur_seg[CURSEG_COLD_DATA] = 0;
>>>>>>>>>>       		c.cur_seg[CURSEG_WARM_DATA] = 0;
>>>>>>>>>>       	} else if (c.heap) {
>>>>>>>>>> -- 
>>>>>>>>>> 2.36.1


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel