All of lore.kernel.org
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space
@ 2022-10-10 22:15 Jaegeuk Kim
  2022-10-10 22:15 ` [f2fs-dev] [PATCH 2/3] fsck.f2fs: fix missing to assign c.zoned_model Jaegeuk Kim
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2022-10-10 22:15 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: Jaegeuk Kim

As f2fs becomes more resilient for GCs, let's give the marginal overprovision
space back to user.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fsck/resize.c      | 8 ++++----
 include/f2fs_fs.h  | 6 ++++--
 mkfs/f2fs_format.c | 9 ++++++---
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/fsck/resize.c b/fsck/resize.c
index 3d8ea466dacd..c048b16e76fb 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -150,7 +150,7 @@ safe_resize:
 		c.new_overprovision = get_best_overprovision(sb);
 
 	c.new_reserved_segments =
-		(2 * (100 / c.new_overprovision + 1) + 6) *
+		(100 / c.new_overprovision + 1 + NR_CURSEG_TYPE) *
 						get_sb(segs_per_sec);
 
 	if ((get_sb(segment_count_main) - 2) < c.new_reserved_segments ||
@@ -475,8 +475,9 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
 	set_cp(overprov_segment_count, (get_newsb(segment_count_main) -
 			get_cp(rsvd_segment_count)) *
 			c.new_overprovision / 100);
-	set_cp(overprov_segment_count, get_cp(overprov_segment_count) +
-						get_cp(rsvd_segment_count));
+
+	if (get_cp(rsvd_segment_count) > get_cp(overprov_segment_count))
+		ASSERT_MSG("Cannot support wrong overprovision ratio\n");
 
 	DBG(0, "Info: Overprovision ratio = %.3lf%%\n", c.new_overprovision);
 	DBG(0, "Info: Overprovision segments = %u (GC reserved = %u)\n",
@@ -607,7 +608,6 @@ static int f2fs_resize_check(struct f2fs_sb_info *sbi, struct f2fs_super_block *
 	overprov_segment_count = (get_newsb(segment_count_main) -
 			c.new_reserved_segments) *
 			c.new_overprovision / 100;
-	overprov_segment_count += c.new_reserved_segments;
 
 	user_block_count = (get_newsb(segment_count_main) -
 			overprov_segment_count) * c.blks_per_seg;
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 5fa9931648cf..8ca574aeba0f 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1631,10 +1631,12 @@ static inline double get_best_overprovision(struct f2fs_super_block *sb)
 	}
 
 	for (; candidate <= end; candidate += diff) {
-		reserved = (2 * (100 / candidate + 1) + 6) *
+		reserved = (100 / candidate + 1 + NR_CURSEG_TYPE) *
 				round_up(usable_main_segs, get_sb(section_count));
 		ovp = (usable_main_segs - reserved) * candidate / 100;
-		space = usable_main_segs - reserved - ovp;
+		if (reserved >= ovp)
+			continue;
+		space = usable_main_segs - ovp;
 		if (max_space < space) {
 			max_space = space;
 			max_ovp = candidate;
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 8d0f41063458..5daf1a17c77d 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -484,7 +484,7 @@ static int f2fs_prepare_super_block(void)
 		c.overprovision = get_best_overprovision(sb);
 
 	c.reserved_segments =
-			(2 * (100 / c.overprovision + 1) + NR_CURSEG_TYPE) *
+			(100 / c.overprovision + 1 + NR_CURSEG_TYPE) *
 			round_up(f2fs_get_usable_segments(sb), get_sb(section_count));
 
 	if (c.feature & cpu_to_le32(F2FS_FEATURE_RO)) {
@@ -764,8 +764,11 @@ static int f2fs_write_check_point_pack(void)
 	set_cp(overprov_segment_count, (f2fs_get_usable_segments(sb) -
 			get_cp(rsvd_segment_count)) *
 			c.overprovision / 100);
-	set_cp(overprov_segment_count, get_cp(overprov_segment_count) +
-			get_cp(rsvd_segment_count));
+
+	if (get_cp(rsvd_segment_count) > get_cp(overprov_segment_count)) {
+		MSG(0, "\tError: Wrong overprovision ratio\n");
+		goto free_cp_payload;
+	}
 
 	if (f2fs_get_usable_segments(sb) <= get_cp(overprov_segment_count)) {
 		MSG(0, "\tError: Not enough segments to create F2FS Volume\n");
-- 
2.38.0.rc1.362.ged0d419d3c-goog



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

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

* [f2fs-dev] [PATCH 2/3] fsck.f2fs: fix missing to assign c.zoned_model
  2022-10-10 22:15 [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Jaegeuk Kim
@ 2022-10-10 22:15 ` Jaegeuk Kim
  2022-10-10 22:15 ` [f2fs-dev] [PATCH 3/3] f2fs-tools: set host-aware zoned device similar to host-managed one Jaegeuk Kim
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2022-10-10 22:15 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: Jaegeuk Kim

We need to set the c.zoned_model appropriately for further usages.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fsck/mount.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fsck/mount.c b/fsck/mount.c
index 0597220b3bc0..cd8a07538150 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1095,6 +1095,14 @@ int init_sb_info(struct f2fs_sb_info *sbi)
 		if (i == 0)
 			c.devices[i].end_blkaddr += get_sb(segment0_blkaddr);
 
+		if (c.zoned_model == F2FS_ZONED_NONE) {
+			if (c.devices[i].zoned_model == F2FS_ZONED_HM)
+				c.zoned_model = F2FS_ZONED_HM;
+			else if (c.devices[i].zoned_model == F2FS_ZONED_HA &&
+					c.zoned_model != F2FS_ZONED_HM)
+				c.zoned_model = F2FS_ZONED_HA;
+		}
+
 		c.ndevs = i + 1;
 		MSG(0, "Info: Device[%d] : %s blkaddr = %"PRIx64"--%"PRIx64"\n",
 				i, c.devices[i].path,
-- 
2.38.0.rc1.362.ged0d419d3c-goog



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

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

* [f2fs-dev] [PATCH 3/3] f2fs-tools: set host-aware zoned device similar to host-managed one
  2022-10-10 22:15 [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Jaegeuk Kim
  2022-10-10 22:15 ` [f2fs-dev] [PATCH 2/3] fsck.f2fs: fix missing to assign c.zoned_model Jaegeuk Kim
@ 2022-10-10 22:15 ` Jaegeuk Kim
  2022-10-20  9:07 ` [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Shinichiro Kawasaki via Linux-f2fs-devel
  2022-10-26 20:37 ` [f2fs-dev] [PATCH 1/3 v2] " Jaegeuk Kim
  3 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2022-10-10 22:15 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: Jaegeuk Kim

It'd be good to generate zone-aware sequential writes to both of HA and HM
zoned devices.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fsck/mount.c            | 2 +-
 mkfs/f2fs_format.c      | 2 +-
 mkfs/f2fs_format_main.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index cd8a07538150..25b7f3b29899 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -987,7 +987,7 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
 	}
 
 	/* Check zoned block device feature */
-	if (c.devices[0].zoned_model == F2FS_ZONED_HM &&
+	if (c.devices[0].zoned_model != F2FS_ZONED_NONE &&
 			!(sb->feature & cpu_to_le32(F2FS_FEATURE_BLKZONED))) {
 		MSG(0, "\tMissing zoned block device feature\n");
 		return -1;
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 5daf1a17c77d..c314d1c330a8 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -454,7 +454,7 @@ static int f2fs_prepare_super_block(void)
 		 * not overlap to metadata area.
 		 */
 		for (i = 1; i < c.ndevs; i++) {
-			if (c.devices[i].zoned_model == F2FS_ZONED_HM &&
+			if (c.devices[i].zoned_model != F2FS_ZONED_NONE &&
 				c.devices[i].start_blkaddr < get_sb(main_blkaddr)) {
 				MSG(0, "\tError: Conventional device %s is too small,"
 					" (%"PRIu64" MiB needed).\n", c.devices[0].path,
diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c
index d2c2605ae6a5..f50971c4591c 100644
--- a/mkfs/f2fs_format_main.c
+++ b/mkfs/f2fs_format_main.c
@@ -480,7 +480,7 @@ int main(int argc, char *argv[])
 	 * Some options are mandatory for host-managed
 	 * zoned block devices.
 	 */
-	if (c.zoned_model == F2FS_ZONED_HM && !c.zoned_mode) {
+	if (c.zoned_model != F2FS_ZONED_NONE && !c.zoned_mode) {
 		MSG(0, "\tError: zoned block device feature is required\n");
 		goto err_format;
 	}
-- 
2.38.0.rc1.362.ged0d419d3c-goog



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

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

* Re: [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space
  2022-10-10 22:15 [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Jaegeuk Kim
  2022-10-10 22:15 ` [f2fs-dev] [PATCH 2/3] fsck.f2fs: fix missing to assign c.zoned_model Jaegeuk Kim
  2022-10-10 22:15 ` [f2fs-dev] [PATCH 3/3] f2fs-tools: set host-aware zoned device similar to host-managed one Jaegeuk Kim
@ 2022-10-20  9:07 ` Shinichiro Kawasaki via Linux-f2fs-devel
  2022-10-20 23:18   ` Jaegeuk Kim
  2022-10-26 20:37 ` [f2fs-dev] [PATCH 1/3 v2] " Jaegeuk Kim
  3 siblings, 1 reply; 8+ messages in thread
From: Shinichiro Kawasaki via Linux-f2fs-devel @ 2022-10-20  9:07 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Damien Le Moal, linux-f2fs-devel

On Oct 10, 2022 / 15:15, Jaegeuk Kim wrote:
> As f2fs becomes more resilient for GCs, let's give the marginal overprovision
> space back to user.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Hello Jaegeuk,

Using the dev branch of f2fs-tools repo, I observed mkfs.f2fs failure with zoned
block devices:

        Error: Device size is not sufficient for F2FS volume

I found this patch in the branch triggers it. I also observed the same failure
is observed with non- zoned regular block devices when I specify -s options to
mkfs.f2fs command. With these conditions, number of segments in each section is
not zero, and it increases the number of reserved segments. My understanding is
that it makes reserved segments larger than overprovisioning segments all the
time in the loop of get_best_overprovision(). Then get_best_overprovision()
returns 0% overprovisioning ratio. Hence the error.

Could you take a look for fix?

FYI, I tried to fix and created a patch which allows reserved segments larger
than overprovisioning segments [1]. It compares those two, and take larger one
to subtract from usable segments to get the segments for users. I confirmed it
keeps small number of overprovisioning segments for no -s option case, and
avoids the mkfs.f2fs failure for the -s option and zoned block device cases.
However, it increases runtime of my test script which fills f2fs and do file
overwrites to test f2fs GC on zoned block devices. It takes +60% longer runtime.
Then GC performance looks worse than before, and this fix does not look good
for me.

[1]

diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 8ca574a..4902953 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1634,9 +1634,10 @@ static inline double get_best_overprovision(struct f2fs_super_block *sb)
 		reserved = (100 / candidate + 1 + NR_CURSEG_TYPE) *
 				round_up(usable_main_segs, get_sb(section_count));
 		ovp = (usable_main_segs - reserved) * candidate / 100;
-		if (reserved >= ovp)
+		if (ovp < 0)
 			continue;
-		space = usable_main_segs - ovp;
+
+		space = usable_main_segs - max(reserved, ovp);
 		if (max_space < space) {
 			max_space = space;
 			max_ovp = candidate;
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index c314d1c..08f4e66 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -764,11 +764,8 @@ static int f2fs_write_check_point_pack(void)
 	set_cp(overprov_segment_count, (f2fs_get_usable_segments(sb) -
 			get_cp(rsvd_segment_count)) *
 			c.overprovision / 100);
-
-	if (get_cp(rsvd_segment_count) > get_cp(overprov_segment_count)) {
-		MSG(0, "\tError: Wrong overprovision ratio\n");
-		goto free_cp_payload;
-	}
+	if (get_cp(overprov_segment_count) < get_cp(rsvd_segment_count))
+		set_cp(overprov_segment_count, get_cp(rsvd_segment_count));
 
 	if (f2fs_get_usable_segments(sb) <= get_cp(overprov_segment_count)) {
 		MSG(0, "\tError: Not enough segments to create F2FS Volume\n");

-- 
Shin'ichiro Kawasaki

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

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

* Re: [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space
  2022-10-20  9:07 ` [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Shinichiro Kawasaki via Linux-f2fs-devel
@ 2022-10-20 23:18   ` Jaegeuk Kim
  2022-10-21  4:30     ` Shinichiro Kawasaki via Linux-f2fs-devel
  0 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim @ 2022-10-20 23:18 UTC (permalink / raw)
  To: Shinichiro Kawasaki; +Cc: Damien Le Moal, linux-f2fs-devel

On 10/20, Shinichiro Kawasaki wrote:
> On Oct 10, 2022 / 15:15, Jaegeuk Kim wrote:
> > As f2fs becomes more resilient for GCs, let's give the marginal overprovision
> > space back to user.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> Hello Jaegeuk,
> 
> Using the dev branch of f2fs-tools repo, I observed mkfs.f2fs failure with zoned
> block devices:
> 
>         Error: Device size is not sufficient for F2FS volume
> 
> I found this patch in the branch triggers it. I also observed the same failure
> is observed with non- zoned regular block devices when I specify -s options to
> mkfs.f2fs command. With these conditions, number of segments in each section is
> not zero, and it increases the number of reserved segments. My understanding is
> that it makes reserved segments larger than overprovisioning segments all the
> time in the loop of get_best_overprovision(). Then get_best_overprovision()
> returns 0% overprovisioning ratio. Hence the error.
> 
> Could you take a look for fix?

Thanks, I think that fix looks good to me. I applied into the original patch.
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev&id=281d3e72370f6c39c0d57acaf37a7f0e003ddd28

> 
> FYI, I tried to fix and created a patch which allows reserved segments larger
> than overprovisioning segments [1]. It compares those two, and take larger one
> to subtract from usable segments to get the segments for users. I confirmed it
> keeps small number of overprovisioning segments for no -s option case, and
> avoids the mkfs.f2fs failure for the -s option and zoned block device cases.
> However, it increases runtime of my test script which fills f2fs and do file
> overwrites to test f2fs GC on zoned block devices. It takes +60% longer runtime.
> Then GC performance looks worse than before, and this fix does not look good
> for me.

I think you can try to avoid that by tuning /sys/fs/f2fs/xx/reserved_blocks?

> 
> [1]
> 
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 8ca574a..4902953 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -1634,9 +1634,10 @@ static inline double get_best_overprovision(struct f2fs_super_block *sb)
>  		reserved = (100 / candidate + 1 + NR_CURSEG_TYPE) *
>  				round_up(usable_main_segs, get_sb(section_count));
>  		ovp = (usable_main_segs - reserved) * candidate / 100;
> -		if (reserved >= ovp)
> +		if (ovp < 0)
>  			continue;
> -		space = usable_main_segs - ovp;
> +
> +		space = usable_main_segs - max(reserved, ovp);
>  		if (max_space < space) {
>  			max_space = space;
>  			max_ovp = candidate;
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index c314d1c..08f4e66 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -764,11 +764,8 @@ static int f2fs_write_check_point_pack(void)
>  	set_cp(overprov_segment_count, (f2fs_get_usable_segments(sb) -
>  			get_cp(rsvd_segment_count)) *
>  			c.overprovision / 100);
> -
> -	if (get_cp(rsvd_segment_count) > get_cp(overprov_segment_count)) {
> -		MSG(0, "\tError: Wrong overprovision ratio\n");
> -		goto free_cp_payload;
> -	}
> +	if (get_cp(overprov_segment_count) < get_cp(rsvd_segment_count))
> +		set_cp(overprov_segment_count, get_cp(rsvd_segment_count));
>  
>  	if (f2fs_get_usable_segments(sb) <= get_cp(overprov_segment_count)) {
>  		MSG(0, "\tError: Not enough segments to create F2FS Volume\n");
> 
> -- 
> Shin'ichiro Kawasaki


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

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

* Re: [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space
  2022-10-20 23:18   ` Jaegeuk Kim
@ 2022-10-21  4:30     ` Shinichiro Kawasaki via Linux-f2fs-devel
  2022-10-24 17:55       ` Jaegeuk Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Shinichiro Kawasaki via Linux-f2fs-devel @ 2022-10-21  4:30 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Damien Le Moal, linux-f2fs-devel

On Oct 20, 2022 / 16:18, Jaegeuk Kim wrote:

...

> Thanks, I think that fix looks good to me. I applied into the original patch.
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev&id=281d3e72370f6c39c0d57acaf37a7f0e003ddd28

Oh, happy to know that the fix is good. And thank you for adding my SoB tag.

One more thing, my fix missed care for fsck/resize.c. I suggest to apply one
more hunk below to the commit in same manner as mkfs/f2fs_format.c.

diff --git a/fsck/resize.c b/fsck/resize.c
index c048b16..79945e1 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -476,8 +476,8 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
                        get_cp(rsvd_segment_count)) *
                        c.new_overprovision / 100);

-       if (get_cp(rsvd_segment_count) > get_cp(overprov_segment_count))
-               ASSERT_MSG("Cannot support wrong overprovision ratio\n");
+       if (get_cp(overprov_segment_count) < get_cp(rsvd_segment_count))
+               set_cp(overprov_segment_count, get_cp(rsvd_segment_count));

        DBG(0, "Info: Overprovision ratio = %.3lf%%\n", c.new_overprovision);
        DBG(0, "Info: Overprovision segments = %u (GC reserved = %u)\n",

Without this change, the assert message "Cannot support wrong overprovision
ratio" was printed when I ran resize.f2fs -t X command to the f2fs formatted
with mkfs.f2fs -s Y. With the change above, the assert message is not printed.

> 
> > 
> > FYI, I tried to fix and created a patch which allows reserved segments larger
> > than overprovisioning segments [1]. It compares those two, and take larger one
> > to subtract from usable segments to get the segments for users. I confirmed it
> > keeps small number of overprovisioning segments for no -s option case, and
> > avoids the mkfs.f2fs failure for the -s option and zoned block device cases.
> > However, it increases runtime of my test script which fills f2fs and do file
> > overwrites to test f2fs GC on zoned block devices. It takes +60% longer runtime.
> > Then GC performance looks worse than before, and this fix does not look good
> > for me.
> 
> I think you can try to avoid that by tuning /sys/fs/f2fs/xx/reserved_blocks?

Thanks, I was able to shorten the runtime using the sysfs attribute. So the
longer GC time was just caused by the smaller reserved segments size. Good.

-- 
Shin'ichiro Kawasaki

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

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

* Re: [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space
  2022-10-21  4:30     ` Shinichiro Kawasaki via Linux-f2fs-devel
@ 2022-10-24 17:55       ` Jaegeuk Kim
  0 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2022-10-24 17:55 UTC (permalink / raw)
  To: Shinichiro Kawasaki; +Cc: Damien Le Moal, linux-f2fs-devel

On 10/21, Shinichiro Kawasaki wrote:
> On Oct 20, 2022 / 16:18, Jaegeuk Kim wrote:
> 
> ...
> 
> > Thanks, I think that fix looks good to me. I applied into the original patch.
> > https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev&id=281d3e72370f6c39c0d57acaf37a7f0e003ddd28
> 
> Oh, happy to know that the fix is good. And thank you for adding my SoB tag.
> 
> One more thing, my fix missed care for fsck/resize.c. I suggest to apply one
> more hunk below to the commit in same manner as mkfs/f2fs_format.c.
> 
> diff --git a/fsck/resize.c b/fsck/resize.c
> index c048b16..79945e1 100644
> --- a/fsck/resize.c
> +++ b/fsck/resize.c
> @@ -476,8 +476,8 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
>                         get_cp(rsvd_segment_count)) *
>                         c.new_overprovision / 100);
> 
> -       if (get_cp(rsvd_segment_count) > get_cp(overprov_segment_count))
> -               ASSERT_MSG("Cannot support wrong overprovision ratio\n");
> +       if (get_cp(overprov_segment_count) < get_cp(rsvd_segment_count))
> +               set_cp(overprov_segment_count, get_cp(rsvd_segment_count));
> 
>         DBG(0, "Info: Overprovision ratio = %.3lf%%\n", c.new_overprovision);
>         DBG(0, "Info: Overprovision segments = %u (GC reserved = %u)\n",
> 
> Without this change, the assert message "Cannot support wrong overprovision
> ratio" was printed when I ran resize.f2fs -t X command to the f2fs formatted
> with mkfs.f2fs -s Y. With the change above, the assert message is not printed.

Thanks, applied.

> 
> > 
> > > 
> > > FYI, I tried to fix and created a patch which allows reserved segments larger
> > > than overprovisioning segments [1]. It compares those two, and take larger one
> > > to subtract from usable segments to get the segments for users. I confirmed it
> > > keeps small number of overprovisioning segments for no -s option case, and
> > > avoids the mkfs.f2fs failure for the -s option and zoned block device cases.
> > > However, it increases runtime of my test script which fills f2fs and do file
> > > overwrites to test f2fs GC on zoned block devices. It takes +60% longer runtime.
> > > Then GC performance looks worse than before, and this fix does not look good
> > > for me.
> > 
> > I think you can try to avoid that by tuning /sys/fs/f2fs/xx/reserved_blocks?
> 
> Thanks, I was able to shorten the runtime using the sysfs attribute. So the
> longer GC time was just caused by the smaller reserved segments size. Good.
> 
> -- 
> Shin'ichiro Kawasaki


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

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

* Re: [f2fs-dev] [PATCH 1/3 v2] f2fs-tools: give less overprovisioning space
  2022-10-10 22:15 [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Jaegeuk Kim
                   ` (2 preceding siblings ...)
  2022-10-20  9:07 ` [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Shinichiro Kawasaki via Linux-f2fs-devel
@ 2022-10-26 20:37 ` Jaegeuk Kim
  3 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2022-10-26 20:37 UTC (permalink / raw)
  To: linux-f2fs-devel

As f2fs becomes more resilient for GCs, let's give the marginal overprovision
space back to user.

Fix an issue where reserved_space > ovp_space, reported by Shinichiro.

Signed-off-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

 Change log from v1:
  - adjust ovp should be larger than reserved to kick f2fs runtime GCs

 fsck/resize.c      | 12 +++++++++---
 include/f2fs_fs.h  |  7 +++++--
 mkfs/f2fs_format.c |  8 ++++++--
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/fsck/resize.c b/fsck/resize.c
index 3d8ea466dacd..c545dd950743 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -150,7 +150,7 @@ safe_resize:
 		c.new_overprovision = get_best_overprovision(sb);
 
 	c.new_reserved_segments =
-		(2 * (100 / c.new_overprovision + 1) + 6) *
+		(100 / c.new_overprovision + 1 + NR_CURSEG_TYPE) *
 						get_sb(segs_per_sec);
 
 	if ((get_sb(segment_count_main) - 2) < c.new_reserved_segments ||
@@ -475,8 +475,13 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
 	set_cp(overprov_segment_count, (get_newsb(segment_count_main) -
 			get_cp(rsvd_segment_count)) *
 			c.new_overprovision / 100);
+
+	/* give 2 sections (DATA and NODE) to trigger GC in advance */
+	if (get_cp(overprov_segment_count) < get_cp(rsvd_segment_count))
+		set_cp(overprov_segment_count, get_cp(rsvd_segment_count));
+
 	set_cp(overprov_segment_count, get_cp(overprov_segment_count) +
-						get_cp(rsvd_segment_count));
+						2 * get_sb(segs_per_sec));
 
 	DBG(0, "Info: Overprovision ratio = %.3lf%%\n", c.new_overprovision);
 	DBG(0, "Info: Overprovision segments = %u (GC reserved = %u)\n",
@@ -607,7 +612,8 @@ static int f2fs_resize_check(struct f2fs_sb_info *sbi, struct f2fs_super_block *
 	overprov_segment_count = (get_newsb(segment_count_main) -
 			c.new_reserved_segments) *
 			c.new_overprovision / 100;
-	overprov_segment_count += c.new_reserved_segments;
+
+	overprov_segment_count += 2 * get_newsb(segs_per_sec);
 
 	user_block_count = (get_newsb(segment_count_main) -
 			overprov_segment_count) * c.blks_per_seg;
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 5fa9931648cf..333ae07a5ebd 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1631,10 +1631,13 @@ static inline double get_best_overprovision(struct f2fs_super_block *sb)
 	}
 
 	for (; candidate <= end; candidate += diff) {
-		reserved = (2 * (100 / candidate + 1) + 6) *
+		reserved = (100 / candidate + 1 + NR_CURSEG_TYPE) *
 				round_up(usable_main_segs, get_sb(section_count));
 		ovp = (usable_main_segs - reserved) * candidate / 100;
-		space = usable_main_segs - reserved - ovp;
+		if (ovp < 0)
+			continue;
+		space = usable_main_segs - max(reserved, ovp) -
+					2 * get_sb(segs_per_sec);
 		if (max_space < space) {
 			max_space = space;
 			max_ovp = candidate;
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index f19a6c5a27d6..f4a49acc498c 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -484,7 +484,7 @@ static int f2fs_prepare_super_block(void)
 		c.overprovision = get_best_overprovision(sb);
 
 	c.reserved_segments =
-			(2 * (100 / c.overprovision + 1) + NR_CURSEG_TYPE) *
+			(100 / c.overprovision + 1 + NR_CURSEG_TYPE) *
 			round_up(f2fs_get_usable_segments(sb), get_sb(section_count));
 
 	if (c.feature & cpu_to_le32(F2FS_FEATURE_RO)) {
@@ -764,8 +764,12 @@ static int f2fs_write_check_point_pack(void)
 	set_cp(overprov_segment_count, (f2fs_get_usable_segments(sb) -
 			get_cp(rsvd_segment_count)) *
 			c.overprovision / 100);
+
+	if (get_cp(overprov_segment_count) < get_cp(rsvd_segment_count))
+		set_cp(overprov_segment_count, get_cp(rsvd_segment_count));
+
 	set_cp(overprov_segment_count, get_cp(overprov_segment_count) +
-			get_cp(rsvd_segment_count));
+			2 * get_sb(segs_per_sec));
 
 	if (f2fs_get_usable_segments(sb) <= get_cp(overprov_segment_count)) {
 		MSG(0, "\tError: Not enough segments to create F2FS Volume\n");
-- 
2.38.0.135.g90850a2211-goog



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

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

end of thread, other threads:[~2022-10-26 20:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-10 22:15 [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Jaegeuk Kim
2022-10-10 22:15 ` [f2fs-dev] [PATCH 2/3] fsck.f2fs: fix missing to assign c.zoned_model Jaegeuk Kim
2022-10-10 22:15 ` [f2fs-dev] [PATCH 3/3] f2fs-tools: set host-aware zoned device similar to host-managed one Jaegeuk Kim
2022-10-20  9:07 ` [f2fs-dev] [PATCH 1/3] f2fs-tools: give less overprovisioning space Shinichiro Kawasaki via Linux-f2fs-devel
2022-10-20 23:18   ` Jaegeuk Kim
2022-10-21  4:30     ` Shinichiro Kawasaki via Linux-f2fs-devel
2022-10-24 17:55       ` Jaegeuk Kim
2022-10-26 20:37 ` [f2fs-dev] [PATCH 1/3 v2] " Jaegeuk Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.