[PATCH 1/5] f2fs: check number of blocks in a current section

[PATCH 1/5] f2fs: check number of blocks in a current section

[Jaegeuk Kim]

In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
the number of blocks in a section instead of the segment.

In addtion, let's check the entire node sections when checking the avaiable
node block space. It does not match one to one per temperature, but currently
we don't have exact dirty page count per temperature. Hence, use a rough
estimation.

Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 3edd3809b6b5..15bf5edd9b3c 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -561,23 +561,23 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
 			unsigned int node_blocks, unsigned int dent_blocks)
 {
 
-	unsigned int segno, left_blocks;
+	unsigned segno, left_blocks;
 	int i;
 
-	/* check current node segment */
+	/* check current node sections, which counts very roughly */
+	left_blocks = 0;
 	for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
 		segno = CURSEG_I(sbi, i)->segno;
-		left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
-				get_seg_entry(sbi, segno)->ckpt_valid_blocks;
-
-		if (node_blocks > left_blocks)
-			return false;
+		left_blocks += CAP_BLKS_PER_SEC(sbi) -
+				get_ckpt_valid_blocks(sbi, segno, true);
 	}
+	if (node_blocks > left_blocks)
+		return false;
 
-	/* check current data segment */
+	/* check current data section for dentry blocks. */
 	segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
-	left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
-			get_seg_entry(sbi, segno)->ckpt_valid_blocks;
+	left_blocks = CAP_BLKS_PER_SEC(sbi) -
+			get_ckpt_valid_blocks(sbi, segno, true);
 	if (dent_blocks > left_blocks)
 		return false;
 	return true;
@@ -626,7 +626,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 
 	if (free_secs > upper_secs)
 		return false;
-	else if (free_secs <= lower_secs)
+	if (free_secs <= lower_secs)
 		return true;
 	return !curseg_space;
 }
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 2/5] f2fs: fix write pointers all the time

[Jaegeuk Kim]

Even if the roll forward recovery stopped due to any error, we have to fix
the write pointers in order to mount the disk from the previous checkpoint.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/recovery.c | 2 +-
 fs/f2fs/super.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index b3baec666afe..8bbecb5f9323 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -913,7 +913,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
 	 * and the f2fs is not read only, check and fix zoned block devices'
 	 * write pointer consistency.
 	 */
-	if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
+	if (fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
 			f2fs_sb_has_blkzoned(sbi)) {
 		err = f2fs_fix_curseg_write_pointer(sbi);
 		if (!err)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2e41142d07c0..4d03ce1109ad 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4673,7 +4673,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	 * If the f2fs is not readonly and fsync data recovery succeeds,
 	 * check zoned block devices' write pointer consistency.
 	 */
-	if (!err && !f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
+	if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
 		err = f2fs_check_write_pointer(sbi);
 		if (err)
 			goto free_meta;
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [f2fs-dev] [PATCH 2/5] f2fs: fix write pointers all the time

[Chao Yu]

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> Even if the roll forward recovery stopped due to any error, we have to fix
> the write pointers in order to mount the disk from the previous checkpoint.

Jaegeuk,

IIUC, we may lost warm node chain once we allocate new section for all logs,
should we give some notification in log to indicate such condition that
filesystem doesn't process a full recovery flow?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/recovery.c | 2 +-
>   fs/f2fs/super.c    | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index b3baec666afe..8bbecb5f9323 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -913,7 +913,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
>   	 * and the f2fs is not read only, check and fix zoned block devices'
>   	 * write pointer consistency.
>   	 */
> -	if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
> +	if (fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
>   			f2fs_sb_has_blkzoned(sbi)) {
>   		err = f2fs_fix_curseg_write_pointer(sbi);
>   		if (!err)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 2e41142d07c0..4d03ce1109ad 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4673,7 +4673,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>   	 * If the f2fs is not readonly and fsync data recovery succeeds,
>   	 * check zoned block devices' write pointer consistency.
>   	 */
> -	if (!err && !f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> +	if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
>   		err = f2fs_check_write_pointer(sbi);
>   		if (err)
>   			goto free_meta;

Re: [f2fs-dev] [PATCH 2/5] f2fs: fix write pointers all the time

[Daeho Jeong]

On Sun, Feb 25, 2024 at 6:49 PM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/2/24 4:55, Jaegeuk Kim wrote:
> > Even if the roll forward recovery stopped due to any error, we have to fix
> > the write pointers in order to mount the disk from the previous checkpoint.
>
> Jaegeuk,
>
> IIUC, we may lost warm node chain once we allocate new section for all logs,
> should we give some notification in log to indicate such condition that
> filesystem doesn't process a full recovery flow?
>
> Thanks,
>
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >   fs/f2fs/recovery.c | 2 +-
> >   fs/f2fs/super.c    | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> > index b3baec666afe..8bbecb5f9323 100644
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -913,7 +913,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
> >        * and the f2fs is not read only, check and fix zoned block devices'
> >        * write pointer consistency.
> >        */
> > -     if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
> > +     if (fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
> >                       f2fs_sb_has_blkzoned(sbi)) {
> >               err = f2fs_fix_curseg_write_pointer(sbi);

We might overwrite the previous err value here with a new err from
f2fs_fix_curseg_write_pointer().

> >               if (!err)
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 2e41142d07c0..4d03ce1109ad 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -4673,7 +4673,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> >        * If the f2fs is not readonly and fsync data recovery succeeds,
> >        * check zoned block devices' write pointer consistency.
> >        */
> > -     if (!err && !f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> > +     if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> >               err = f2fs_check_write_pointer(sbi);

ditto

> >               if (err)
> >                       goto free_meta;
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 2/5] f2fs: fix write pointers all the time

[Jaegeuk Kim]

On 02/26, Chao Yu wrote:
> On 2024/2/24 4:55, Jaegeuk Kim wrote:
> > Even if the roll forward recovery stopped due to any error, we have to fix
> > the write pointers in order to mount the disk from the previous checkpoint.
> 
> Jaegeuk,
> 
> IIUC, we may lost warm node chain once we allocate new section for all logs,
> should we give some notification in log to indicate such condition that
> filesystem doesn't process a full recovery flow?

How about v2 to preserve the error which gives a warnings on roll-forward
recovery?

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >   fs/f2fs/recovery.c | 2 +-
> >   fs/f2fs/super.c    | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> > index b3baec666afe..8bbecb5f9323 100644
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -913,7 +913,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
> >   	 * and the f2fs is not read only, check and fix zoned block devices'
> >   	 * write pointer consistency.
> >   	 */
> > -	if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
> > +	if (fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
> >   			f2fs_sb_has_blkzoned(sbi)) {
> >   		err = f2fs_fix_curseg_write_pointer(sbi);
> >   		if (!err)
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 2e41142d07c0..4d03ce1109ad 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -4673,7 +4673,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> >   	 * If the f2fs is not readonly and fsync data recovery succeeds,
> >   	 * check zoned block devices' write pointer consistency.
> >   	 */
> > -	if (!err && !f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> > +	if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> >   		err = f2fs_check_write_pointer(sbi);
> >   		if (err)
> >   			goto free_meta;

Re: [PATCH 2/5 v2] f2fs: fix write pointers all the time

[Jaegeuk Kim]

Even if the roll forward recovery stopped due to any error, we have to fix
the write pointers in order to mount the disk from the previous checkpoint.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

 from v1:
  - preserve error

 fs/f2fs/recovery.c | 15 +++++++--------
 fs/f2fs/super.c    | 11 +++++++----
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index b3baec666afe..3078d5613748 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -863,7 +863,6 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
 	int ret = 0;
 	unsigned long s_flags = sbi->sb->s_flags;
 	bool need_writecp = false;
-	bool fix_curseg_write_pointer = false;
 
 	if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE))
 		f2fs_info(sbi, "recover fsync data on readonly fs");
@@ -894,8 +893,6 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
 	else
 		f2fs_bug_on(sbi, sbi->sb->s_flags & SB_ACTIVE);
 skip:
-	fix_curseg_write_pointer = !check_only || list_empty(&inode_list);
-
 	destroy_fsync_dnodes(&inode_list, err);
 	destroy_fsync_dnodes(&tmp_inode_list, err);
 
@@ -913,11 +910,13 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
 	 * and the f2fs is not read only, check and fix zoned block devices'
 	 * write pointer consistency.
 	 */
-	if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
-			f2fs_sb_has_blkzoned(sbi)) {
-		err = f2fs_fix_curseg_write_pointer(sbi);
-		if (!err)
-			err = f2fs_check_write_pointer(sbi);
+	if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) {
+		int err2 = f2fs_fix_curseg_write_pointer(sbi);
+
+		if (!err2)
+			err2 = f2fs_check_write_pointer(sbi);
+		if (err2)
+			err = err2;
 		ret = err;
 	}
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d91e57ca6110..77348fd0a42b 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4674,11 +4674,14 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	 * If the f2fs is not readonly and fsync data recovery succeeds,
 	 * check zoned block devices' write pointer consistency.
 	 */
-	if (!err && !f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
-		err = f2fs_check_write_pointer(sbi);
-		if (err)
-			goto free_meta;
+	if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) {
+		int err2 = f2fs_check_write_pointer(sbi);
+
+		if (err2)
+			err = err2;
 	}
+	if (err)
+		goto free_meta;
 
 	f2fs_init_inmem_curseg(sbi);
 
-- 
2.44.0.rc1.240.g4c46232300-goog

Re: [f2fs-dev] [PATCH 2/5 v2] f2fs: fix write pointers all the time

[Chao Yu]

On 2024/2/27 8:59, Jaegeuk Kim wrote:
> Even if the roll forward recovery stopped due to any error, we have to fix
> the write pointers in order to mount the disk from the previous checkpoint.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

[PATCH 3/5] f2fs: print zone status in string and some log

[Jaegeuk Kim]

No functional change, but add some more logs.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.c | 34 ++++++++++++++++++++++++----------
 fs/f2fs/super.c   |  1 +
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d4f228e6f771..6d586ae8b55f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4912,6 +4912,16 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
 }
 
 #ifdef CONFIG_BLK_DEV_ZONED
+const char *f2fs_zone_status[BLK_ZONE_COND_OFFLINE + 1] = {
+	[BLK_ZONE_COND_NOT_WP]		= "NOT_WP",
+	[BLK_ZONE_COND_EMPTY]		= "EMPTY",
+	[BLK_ZONE_COND_IMP_OPEN]	= "IMPLICITE_OPEN",
+	[BLK_ZONE_COND_EXP_OPEN]	= "EXPLICITE_OPEN",
+	[BLK_ZONE_COND_CLOSED]		= "CLOSED",
+	[BLK_ZONE_COND_READONLY]	= "READONLY",
+	[BLK_ZONE_COND_FULL]		= "FULL",
+	[BLK_ZONE_COND_OFFLINE]		= "OFFLINE",
+};
 
 static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 				    struct f2fs_dev_info *fdev,
@@ -4928,18 +4938,22 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 	zone_block = fdev->start_blk + (zone->start >> log_sectors_per_block);
 	zone_segno = GET_SEGNO(sbi, zone_block);
 
+	/*
+	 * Get # of valid block of the zone.
+	 */
+	valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
+
 	/*
 	 * Skip check of zones cursegs point to, since
 	 * fix_curseg_write_pointer() checks them.
 	 */
 	if (zone_segno >= MAIN_SEGS(sbi) ||
-	    IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno)))
+	    IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno))) {
+		f2fs_notice(sbi, "Open zones: valid block[0x%x,0x%x] cond[%s]",
+				zone_segno, valid_block_cnt,
+				f2fs_zone_status[zone->cond]);
 		return 0;
-
-	/*
-	 * Get # of valid block of the zone.
-	 */
-	valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
+	}
 
 	if ((!valid_block_cnt && zone->cond == BLK_ZONE_COND_EMPTY) ||
 	    (valid_block_cnt && zone->cond == BLK_ZONE_COND_FULL))
@@ -4947,8 +4961,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 
 	if (!valid_block_cnt) {
 		f2fs_notice(sbi, "Zone without valid block has non-zero write "
-			    "pointer. Reset the write pointer: cond[0x%x]",
-			    zone->cond);
+			    "pointer. Reset the write pointer: cond[%s]",
+			    f2fs_zone_status[zone->cond]);
 		ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
 					zone->len >> log_sectors_per_block);
 		if (ret)
@@ -4965,8 +4979,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 	 * selected for write operation until it get discarded.
 	 */
 	f2fs_notice(sbi, "Valid blocks are not aligned with write "
-		    "pointer: valid block[0x%x,0x%x] cond[0x%x]",
-		    zone_segno, valid_block_cnt, zone->cond);
+		    "pointer: valid block[0x%x,0x%x] cond[%s]",
+		    zone_segno, valid_block_cnt, f2fs_zone_status[zone->cond]);
 
 	ret = blkdev_zone_mgmt(fdev->bdev, REQ_OP_ZONE_FINISH,
 				zone->start, zone->len, GFP_NOFS);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4d03ce1109ad..fc7f1a9fbbda 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4674,6 +4674,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	 * check zoned block devices' write pointer consistency.
 	 */
 	if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
+		f2fs_notice(sbi, "Checking entire write pointers");
 		err = f2fs_check_write_pointer(sbi);
 		if (err)
 			goto free_meta;
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [f2fs-dev] [PATCH 3/5] f2fs: print zone status in string and some log

[Chao Yu]

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> No functional change, but add some more logs.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH 3/5] f2fs: print zone status in string and some log

[Daeho Jeong]

On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> No functional change, but add some more logs.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/segment.c | 34 ++++++++++++++++++++++++----------
>  fs/f2fs/super.c   |  1 +
>  2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index d4f228e6f771..6d586ae8b55f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4912,6 +4912,16 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
>  }
>
>  #ifdef CONFIG_BLK_DEV_ZONED
> +const char *f2fs_zone_status[BLK_ZONE_COND_OFFLINE + 1] = {
> +       [BLK_ZONE_COND_NOT_WP]          = "NOT_WP",
> +       [BLK_ZONE_COND_EMPTY]           = "EMPTY",
> +       [BLK_ZONE_COND_IMP_OPEN]        = "IMPLICITE_OPEN",
> +       [BLK_ZONE_COND_EXP_OPEN]        = "EXPLICITE_OPEN",
> +       [BLK_ZONE_COND_CLOSED]          = "CLOSED",
> +       [BLK_ZONE_COND_READONLY]        = "READONLY",
> +       [BLK_ZONE_COND_FULL]            = "FULL",
> +       [BLK_ZONE_COND_OFFLINE]         = "OFFLINE",
> +};
>
>  static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>                                     struct f2fs_dev_info *fdev,
> @@ -4928,18 +4938,22 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>         zone_block = fdev->start_blk + (zone->start >> log_sectors_per_block);
>         zone_segno = GET_SEGNO(sbi, zone_block);
>
> +       /*
> +        * Get # of valid block of the zone.
> +        */
> +       valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
> +
>         /*
>          * Skip check of zones cursegs point to, since
>          * fix_curseg_write_pointer() checks them.
>          */
>         if (zone_segno >= MAIN_SEGS(sbi) ||

How about this not to use a wrong segno value for get_valid_blocks()
and not to print a wrong segment info?

    if (zone_segno >= MAIN_SEGS(sbi))
        return 0;

    valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);

> -           IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno)))
> +           IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno))) {
> +               f2fs_notice(sbi, "Open zones: valid block[0x%x,0x%x] cond[%s]",
> +                               zone_segno, valid_block_cnt,
> +                               f2fs_zone_status[zone->cond]);
>                 return 0;
> -
> -       /*
> -        * Get # of valid block of the zone.
> -        */
> -       valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
> +       }
>
>         if ((!valid_block_cnt && zone->cond == BLK_ZONE_COND_EMPTY) ||
>             (valid_block_cnt && zone->cond == BLK_ZONE_COND_FULL))
> @@ -4947,8 +4961,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>
>         if (!valid_block_cnt) {
>                 f2fs_notice(sbi, "Zone without valid block has non-zero write "
> -                           "pointer. Reset the write pointer: cond[0x%x]",
> -                           zone->cond);
> +                           "pointer. Reset the write pointer: cond[%s]",
> +                           f2fs_zone_status[zone->cond]);
>                 ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
>                                         zone->len >> log_sectors_per_block);
>                 if (ret)
> @@ -4965,8 +4979,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>          * selected for write operation until it get discarded.
>          */
>         f2fs_notice(sbi, "Valid blocks are not aligned with write "
> -                   "pointer: valid block[0x%x,0x%x] cond[0x%x]",
> -                   zone_segno, valid_block_cnt, zone->cond);
> +                   "pointer: valid block[0x%x,0x%x] cond[%s]",
> +                   zone_segno, valid_block_cnt, f2fs_zone_status[zone->cond]);
>
>         ret = blkdev_zone_mgmt(fdev->bdev, REQ_OP_ZONE_FINISH,
>                                 zone->start, zone->len, GFP_NOFS);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4d03ce1109ad..fc7f1a9fbbda 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4674,6 +4674,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>          * check zoned block devices' write pointer consistency.
>          */
>         if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> +               f2fs_notice(sbi, "Checking entire write pointers");
>                 err = f2fs_check_write_pointer(sbi);
>                 if (err)
>                         goto free_meta;
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 3/5 v2] f2fs: print zone status in string and some log

[Jaegeuk Kim]

No functional change, but add some more logs.

Reviewed-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

 from v1:
  - avoid unnecessary log per Daeho's comment

 fs/f2fs/segment.c | 27 +++++++++++++++++++++------
 fs/f2fs/super.c   |  1 +
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d4f228e6f771..31a9e7db19e3 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4912,6 +4912,16 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
 }
 
 #ifdef CONFIG_BLK_DEV_ZONED
+const char *f2fs_zone_status[BLK_ZONE_COND_OFFLINE + 1] = {
+	[BLK_ZONE_COND_NOT_WP]		= "NOT_WP",
+	[BLK_ZONE_COND_EMPTY]		= "EMPTY",
+	[BLK_ZONE_COND_IMP_OPEN]	= "IMPLICITE_OPEN",
+	[BLK_ZONE_COND_EXP_OPEN]	= "EXPLICITE_OPEN",
+	[BLK_ZONE_COND_CLOSED]		= "CLOSED",
+	[BLK_ZONE_COND_READONLY]	= "READONLY",
+	[BLK_ZONE_COND_FULL]		= "FULL",
+	[BLK_ZONE_COND_OFFLINE]		= "OFFLINE",
+};
 
 static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 				    struct f2fs_dev_info *fdev,
@@ -4932,14 +4942,19 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 	 * Skip check of zones cursegs point to, since
 	 * fix_curseg_write_pointer() checks them.
 	 */
-	if (zone_segno >= MAIN_SEGS(sbi) ||
-	    IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno)))
+	if (zone_segno >= MAIN_SEGS(sbi))
 		return 0;
 
 	/*
 	 * Get # of valid block of the zone.
 	 */
 	valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
+	if (IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno))) {
+		f2fs_notice(sbi, "Open zones: valid block[0x%x,0x%x] cond[%s]",
+				zone_segno, valid_block_cnt,
+				f2fs_zone_status[zone->cond]);
+		return 0;
+	}
 
 	if ((!valid_block_cnt && zone->cond == BLK_ZONE_COND_EMPTY) ||
 	    (valid_block_cnt && zone->cond == BLK_ZONE_COND_FULL))
@@ -4947,8 +4962,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 
 	if (!valid_block_cnt) {
 		f2fs_notice(sbi, "Zone without valid block has non-zero write "
-			    "pointer. Reset the write pointer: cond[0x%x]",
-			    zone->cond);
+			    "pointer. Reset the write pointer: cond[%s]",
+			    f2fs_zone_status[zone->cond]);
 		ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
 					zone->len >> log_sectors_per_block);
 		if (ret)
@@ -4965,8 +4980,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 	 * selected for write operation until it get discarded.
 	 */
 	f2fs_notice(sbi, "Valid blocks are not aligned with write "
-		    "pointer: valid block[0x%x,0x%x] cond[0x%x]",
-		    zone_segno, valid_block_cnt, zone->cond);
+		    "pointer: valid block[0x%x,0x%x] cond[%s]",
+		    zone_segno, valid_block_cnt, f2fs_zone_status[zone->cond]);
 
 	ret = blkdev_zone_mgmt(fdev->bdev, REQ_OP_ZONE_FINISH,
 				zone->start, zone->len, GFP_NOFS);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4d03ce1109ad..fc7f1a9fbbda 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4674,6 +4674,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	 * check zoned block devices' write pointer consistency.
 	 */
 	if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
+		f2fs_notice(sbi, "Checking entire write pointers");
 		err = f2fs_check_write_pointer(sbi);
 		if (err)
 			goto free_meta;
-- 
2.44.0.rc1.240.g4c46232300-goog

Re: [f2fs-dev] [PATCH 3/5 v2] f2fs: print zone status in string and some log

[Chao Yu]

On 2024/2/27 6:52, Jaegeuk Kim wrote:
> No functional change, but add some more logs.
> 
> Reviewed-by: Chao Yu <chao@kernel.org>

v2 looks fine to me.

Thanks,

Re: [PATCH 3/5] f2fs: print zone status in string and some log

[Jaegeuk Kim]

On 02/23, Jaegeuk Kim wrote:
> No functional change, but add some more logs.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/segment.c | 34 ++++++++++++++++++++++++----------
>  fs/f2fs/super.c   |  1 +
>  2 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index d4f228e6f771..6d586ae8b55f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -4912,6 +4912,16 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
>  }
>  
>  #ifdef CONFIG_BLK_DEV_ZONED
> +const char *f2fs_zone_status[BLK_ZONE_COND_OFFLINE + 1] = {

Added static.

> +	[BLK_ZONE_COND_NOT_WP]		= "NOT_WP",
> +	[BLK_ZONE_COND_EMPTY]		= "EMPTY",
> +	[BLK_ZONE_COND_IMP_OPEN]	= "IMPLICITE_OPEN",
> +	[BLK_ZONE_COND_EXP_OPEN]	= "EXPLICITE_OPEN",
> +	[BLK_ZONE_COND_CLOSED]		= "CLOSED",
> +	[BLK_ZONE_COND_READONLY]	= "READONLY",
> +	[BLK_ZONE_COND_FULL]		= "FULL",
> +	[BLK_ZONE_COND_OFFLINE]		= "OFFLINE",
> +};
>  
>  static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>  				    struct f2fs_dev_info *fdev,
> @@ -4928,18 +4938,22 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>  	zone_block = fdev->start_blk + (zone->start >> log_sectors_per_block);
>  	zone_segno = GET_SEGNO(sbi, zone_block);
>  
> +	/*
> +	 * Get # of valid block of the zone.
> +	 */
> +	valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
> +
>  	/*
>  	 * Skip check of zones cursegs point to, since
>  	 * fix_curseg_write_pointer() checks them.
>  	 */
>  	if (zone_segno >= MAIN_SEGS(sbi) ||
> -	    IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno)))
> +	    IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno))) {
> +		f2fs_notice(sbi, "Open zones: valid block[0x%x,0x%x] cond[%s]",
> +				zone_segno, valid_block_cnt,
> +				f2fs_zone_status[zone->cond]);
>  		return 0;
> -
> -	/*
> -	 * Get # of valid block of the zone.
> -	 */
> -	valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
> +	}
>  
>  	if ((!valid_block_cnt && zone->cond == BLK_ZONE_COND_EMPTY) ||
>  	    (valid_block_cnt && zone->cond == BLK_ZONE_COND_FULL))
> @@ -4947,8 +4961,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>  
>  	if (!valid_block_cnt) {
>  		f2fs_notice(sbi, "Zone without valid block has non-zero write "
> -			    "pointer. Reset the write pointer: cond[0x%x]",
> -			    zone->cond);
> +			    "pointer. Reset the write pointer: cond[%s]",
> +			    f2fs_zone_status[zone->cond]);
>  		ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
>  					zone->len >> log_sectors_per_block);
>  		if (ret)
> @@ -4965,8 +4979,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
>  	 * selected for write operation until it get discarded.
>  	 */
>  	f2fs_notice(sbi, "Valid blocks are not aligned with write "
> -		    "pointer: valid block[0x%x,0x%x] cond[0x%x]",
> -		    zone_segno, valid_block_cnt, zone->cond);
> +		    "pointer: valid block[0x%x,0x%x] cond[%s]",
> +		    zone_segno, valid_block_cnt, f2fs_zone_status[zone->cond]);
>  
>  	ret = blkdev_zone_mgmt(fdev->bdev, REQ_OP_ZONE_FINISH,
>  				zone->start, zone->len, GFP_NOFS);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4d03ce1109ad..fc7f1a9fbbda 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4674,6 +4674,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  	 * check zoned block devices' write pointer consistency.
>  	 */
>  	if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
> +		f2fs_notice(sbi, "Checking entire write pointers");
>  		err = f2fs_check_write_pointer(sbi);
>  		if (err)
>  			goto free_meta;
> -- 
> 2.44.0.rc0.258.g7320e95886-goog

[PATCH 4/5] f2fs: prevent an f2fs_gc loop during disable_checkpoint

[Jaegeuk Kim]

Don't get stuck in the f2fs_gc loop while disabling checkpoint. Instead, we have
a time-based management.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index fc7f1a9fbbda..7d9b92978709 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2191,6 +2191,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 			.init_gc_type = FG_GC,
 			.should_migrate_blocks = false,
 			.err_gc_skipped = true,
+			.no_bg_gc = true,
 			.nr_free_secs = 1 };
 
 		f2fs_down_write(&sbi->gc_lock);
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [f2fs-dev] [PATCH 4/5] f2fs: prevent an f2fs_gc loop during disable_checkpoint

[Chao Yu]

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> Don't get stuck in the f2fs_gc loop while disabling checkpoint. Instead, we have
> a time-based management.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH 4/5] f2fs: prevent an f2fs_gc loop during disable_checkpoint

[Daeho Jeong]

Reviewed-by: Daeho Jeong <daehojeong@google.com>

On Sun, Feb 25, 2024 at 6:59 PM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/2/24 4:55, Jaegeuk Kim wrote:
> > Don't get stuck in the f2fs_gc loop while disabling checkpoint. Instead, we have
> > a time-based management.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>
> Reviewed-by: Chao Yu <chao@kernel.org>
>
> Thanks,
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 5/5] f2fs: allow to mount if cap is 100

[Jaegeuk Kim]

Don't block mounting the partition, if cap is 100%.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 6d586ae8b55f..f11361152d2a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -904,6 +904,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable)
 {
 	int ovp_hole_segs =
 		(overprovision_segments(sbi) - reserved_segments(sbi));
+
+	if (F2FS_OPTION(sbi).unusable_cap_perc == 100)
+		return 0;
 	if (unusable > F2FS_OPTION(sbi).unusable_cap)
 		return -EAGAIN;
 	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [f2fs-dev] [PATCH 5/5] f2fs: allow to mount if cap is 100

[Chao Yu]

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> Don't block mounting the partition, if cap is 100%.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH 5/5] f2fs: allow to mount if cap is 100

[Daeho Jeong]

On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> Don't block mounting the partition, if cap is 100%.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/segment.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6d586ae8b55f..f11361152d2a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -904,6 +904,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable)
>  {
>         int ovp_hole_segs =
>                 (overprovision_segments(sbi) - reserved_segments(sbi));
> +
> +       if (F2FS_OPTION(sbi).unusable_cap_perc == 100)
> +               return 0;

With this, f2fs will not execute GC. What is this 100
unusable_cap_perc used for?

>         if (unusable > F2FS_OPTION(sbi).unusable_cap)
>                 return -EAGAIN;
>         if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 5/5] f2fs: allow to mount if cap is 100

[Jaegeuk Kim]

On 02/26, Daeho Jeong wrote:
> On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> >
> > Don't block mounting the partition, if cap is 100%.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/segment.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 6d586ae8b55f..f11361152d2a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -904,6 +904,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable)
> >  {
> >         int ovp_hole_segs =
> >                 (overprovision_segments(sbi) - reserved_segments(sbi));
> > +
> > +       if (F2FS_OPTION(sbi).unusable_cap_perc == 100)
> > +               return 0;
> 
> With this, f2fs will not execute GC. What is this 100
> unusable_cap_perc used for?

Theoritically, it won't, but I was hitting a GC loop, and I suspected a wrong
section/segment layout. So, I posted the below, and guess we need this patch
as workaround.

https://patchwork.kernel.org/project/f2fs/patch/20240224011411.3820719-1-jaegeuk@kernel.org/

> 
> >         if (unusable > F2FS_OPTION(sbi).unusable_cap)
> >                 return -EAGAIN;
> >         if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
> > --
> > 2.44.0.rc0.258.g7320e95886-goog
> >
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 5/5] f2fs: allow to mount if cap is 100

[Daeho Jeong]

On Mon, Feb 26, 2024 at 2:47 PM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> On 02/26, Daeho Jeong wrote:
> > On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> > >
> > > Don't block mounting the partition, if cap is 100%.
> > >
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > ---
> > >  fs/f2fs/segment.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > index 6d586ae8b55f..f11361152d2a 100644
> > > --- a/fs/f2fs/segment.c
> > > +++ b/fs/f2fs/segment.c
> > > @@ -904,6 +904,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable)
> > >  {
> > >         int ovp_hole_segs =
> > >                 (overprovision_segments(sbi) - reserved_segments(sbi));
> > > +
> > > +       if (F2FS_OPTION(sbi).unusable_cap_perc == 100)
> > > +               return 0;
> >
> > With this, f2fs will not execute GC. What is this 100
> > unusable_cap_perc used for?
>
> Theoritically, it won't, but I was hitting a GC loop, and I suspected a wrong
> section/segment layout. So, I posted the below, and guess we need this patch
> as workaround.
>
> https://patchwork.kernel.org/project/f2fs/patch/20240224011411.3820719-1-jaegeuk@kernel.org/

Reviewed-by: Daeho Jeong <daehojeong@google.com>

>
> >
> > >         if (unusable > F2FS_OPTION(sbi).unusable_cap)
> > >                 return -EAGAIN;
> > >         if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
> > > --
> > > 2.44.0.rc0.258.g7320e95886-goog
> > >
> > >
> > >
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 1/5] f2fs: check number of blocks in a current section

[Chao Yu]

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
> the number of blocks in a section instead of the segment.
> 
> In addtion, let's check the entire node sections when checking the avaiable
> node block space. It does not match one to one per temperature, but currently

I tested this patch w/ testcase in [1], it doesn't complain.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=215914

> we don't have exact dirty page count per temperature. Hence, use a rough
> estimation.

Do we need more accurate per-temperature dirty node count for extreme
corner case?

e.g. node_blocks is counted from hot dirty nodes, and current hot_node
log has no enough free space for it.

Thanks,

> 
> Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/segment.h | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 3edd3809b6b5..15bf5edd9b3c 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -561,23 +561,23 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
>   			unsigned int node_blocks, unsigned int dent_blocks)
>   {
>   
> -	unsigned int segno, left_blocks;
> +	unsigned segno, left_blocks;
>   	int i;
>   
> -	/* check current node segment */
> +	/* check current node sections, which counts very roughly */
> +	left_blocks = 0;
>   	for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
>   		segno = CURSEG_I(sbi, i)->segno;
> -		left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> -				get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> -
> -		if (node_blocks > left_blocks)
> -			return false;
> +		left_blocks += CAP_BLKS_PER_SEC(sbi) -
> +				get_ckpt_valid_blocks(sbi, segno, true);
>   	}
> +	if (node_blocks > left_blocks)
> +		return false;
>   
> -	/* check current data segment */
> +	/* check current data section for dentry blocks. */
>   	segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
> -	left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> -			get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> +	left_blocks = CAP_BLKS_PER_SEC(sbi) -
> +			get_ckpt_valid_blocks(sbi, segno, true);
>   	if (dent_blocks > left_blocks)
>   		return false;
>   	return true;
> @@ -626,7 +626,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>   
>   	if (free_secs > upper_secs)
>   		return false;
> -	else if (free_secs <= lower_secs)
> +	if (free_secs <= lower_secs)
>   		return true;
>   	return !curseg_space;
>   }

Re: [f2fs-dev] [PATCH 1/5] f2fs: check number of blocks in a current section

[Daeho Jeong]

On Sun, Feb 25, 2024 at 6:42 PM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/2/24 4:55, Jaegeuk Kim wrote:
> > In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
> > the number of blocks in a section instead of the segment.
> >
> > In addtion, let's check the entire node sections when checking the avaiable
> > node block space. It does not match one to one per temperature, but currently
>
> I tested this patch w/ testcase in [1], it doesn't complain.
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=215914
>
> > we don't have exact dirty page count per temperature. Hence, use a rough
> > estimation.
>
> Do we need more accurate per-temperature dirty node count for extreme
> corner case?
>
> e.g. node_blocks is counted from hot dirty nodes, and current hot_node
> log has no enough free space for it.
>
> Thanks,
>
> >
> > Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >   fs/f2fs/segment.h | 22 +++++++++++-----------
> >   1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > index 3edd3809b6b5..15bf5edd9b3c 100644
> > --- a/fs/f2fs/segment.h
> > +++ b/fs/f2fs/segment.h
> > @@ -561,23 +561,23 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
> >                       unsigned int node_blocks, unsigned int dent_blocks)
> >   {
> >
> > -     unsigned int segno, left_blocks;
> > +     unsigned segno, left_blocks;
> >       int i;
> >
> > -     /* check current node segment */
> > +     /* check current node sections, which counts very roughly */
> > +     left_blocks = 0;
> >       for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
> >               segno = CURSEG_I(sbi, i)->segno;
> > -             left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> > -                             get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> > -
> > -             if (node_blocks > left_blocks)
> > -                     return false;
> > +             left_blocks += CAP_BLKS_PER_SEC(sbi) -
> > +                             get_ckpt_valid_blocks(sbi, segno, true);
> >       }
> > +     if (node_blocks > left_blocks)
> > +             return false;

I am not sure this is okay. The current implementation of f2fs may not
be optimal when the HOT_NODE section's space requirements exceed its
current capacity. In such cases, f2fs necessitates the creation of a
new free section to accommodate the overflow, even though there might
be free space available in other NODE sections. To address this issue,
it may be necessary to implement a more accurate accounting system for
NODE sections based on their temperature levels.

> >
> > -     /* check current data segment */
> > +     /* check current data section for dentry blocks. */
> >       segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
> > -     left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> > -                     get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> > +     left_blocks = CAP_BLKS_PER_SEC(sbi) -
> > +                     get_ckpt_valid_blocks(sbi, segno, true);
> >       if (dent_blocks > left_blocks)
> >               return false;
> >       return true;
> > @@ -626,7 +626,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
> >
> >       if (free_secs > upper_secs)
> >               return false;
> > -     else if (free_secs <= lower_secs)
> > +     if (free_secs <= lower_secs)
> >               return true;
> >       return !curseg_space;
> >   }
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 1/5 v2] f2fs: check number of blocks in a current section

[Jaegeuk Kim]

In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
the number of blocks in a section instead of the segment.

Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

 from v1:
  - check current node block space to deal with the worst case
  - TODO: need to fine tuning on node temperature

 fs/f2fs/segment.h | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 3edd3809b6b5..335fc6285fa5 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -561,23 +561,22 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
 			unsigned int node_blocks, unsigned int dent_blocks)
 {
 
-	unsigned int segno, left_blocks;
+	unsigned segno, left_blocks;
 	int i;
 
-	/* check current node segment */
+	/* check current node sections in the worst case. */
 	for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
 		segno = CURSEG_I(sbi, i)->segno;
-		left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
-				get_seg_entry(sbi, segno)->ckpt_valid_blocks;
-
+		left_blocks = CAP_BLKS_PER_SEC(sbi) -
+				get_ckpt_valid_blocks(sbi, segno, true);
 		if (node_blocks > left_blocks)
 			return false;
 	}
 
-	/* check current data segment */
+	/* check current data section for dentry blocks. */
 	segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
-	left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
-			get_seg_entry(sbi, segno)->ckpt_valid_blocks;
+	left_blocks = CAP_BLKS_PER_SEC(sbi) -
+			get_ckpt_valid_blocks(sbi, segno, true);
 	if (dent_blocks > left_blocks)
 		return false;
 	return true;
@@ -626,7 +625,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 
 	if (free_secs > upper_secs)
 		return false;
-	else if (free_secs <= lower_secs)
+	if (free_secs <= lower_secs)
 		return true;
 	return !curseg_space;
 }
-- 
2.44.0.rc1.240.g4c46232300-goog

Re: [f2fs-dev] [PATCH 1/5 v2] f2fs: check number of blocks in a current section

[Chao Yu]

On 2024/2/27 7:14, Jaegeuk Kim wrote:
> In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
> the number of blocks in a section instead of the segment.
> 
> Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH 1/5] f2fs: check number of blocks in a current section

[patchwork-bot+f2fs]

Hello:

This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Fri, 23 Feb 2024 12:55:31 -0800 you wrote:
> In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
> the number of blocks in a section instead of the segment.
> 
> In addtion, let's check the entire node sections when checking the avaiable
> node block space. It does not match one to one per temperature, but currently
> we don't have exact dirty page count per temperature. Hence, use a rough
> estimation.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,1/5] f2fs: check number of blocks in a current section
    (no matching commit)
  - [f2fs-dev,2/5] f2fs: fix write pointers all the time
    (no matching commit)
  - [f2fs-dev,3/5] f2fs: print zone status in string and some log
    (no matching commit)
  - [f2fs-dev,4/5] f2fs: prevent an f2fs_gc loop during disable_checkpoint
    https://git.kernel.org/jaegeuk/f2fs/c/496b655d0460
  - [f2fs-dev,5/5] f2fs: allow to mount if cap is 100
    https://git.kernel.org/jaegeuk/f2fs/c/38fcb47642ae

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html