2024-02-23 20:55:45

by Jaegeuk Kim

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

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 <[email protected]>
---
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



2024-02-23 20:55:53

by Jaegeuk Kim

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

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 <[email protected]>
---
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


2024-02-23 20:55:59

by Jaegeuk Kim

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

No functional change, but add some more logs.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
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


2024-02-23 20:56:17

by Jaegeuk Kim

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

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

Signed-off-by: Jaegeuk Kim <[email protected]>
---
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


2024-02-23 20:56:28

by Jaegeuk Kim

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

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

Signed-off-by: Jaegeuk Kim <[email protected]>
---
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


2024-02-26 02:40:51

by Chao Yu

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

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 <[email protected]>
> ---
> 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;
> }

2024-02-26 02:48:12

by Chao Yu

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

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 <[email protected]>
> ---
> 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;

2024-02-26 02:54:25

by Chao Yu

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

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> No functional change, but add some more logs.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2024-02-26 02:58:59

by Chao Yu

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

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 <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2024-02-26 02:59:45

by Chao Yu

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

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> Don't block mounting the partition, if cap is 100%.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2024-02-26 17:22:13

by Daeho Jeong

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

On Sun, Feb 25, 2024 at 6:42 PM Chao Yu <[email protected]> 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 <[email protected]>
> > ---
> > 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-26 17:35:37

by Daeho Jeong

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

On Sun, Feb 25, 2024 at 6:49 PM Chao Yu <[email protected]> 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 <[email protected]>
> > ---
> > 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-26 19:03:39

by Daeho Jeong

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

On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <[email protected]> wrote:
>
> No functional change, but add some more logs.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-26 19:17:40

by Daeho Jeong

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

Reviewed-by: Daeho Jeong <[email protected]>

On Sun, Feb 25, 2024 at 6:59 PM Chao Yu <[email protected]> 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 <[email protected]>
>
> Reviewed-by: Chao Yu <[email protected]>
>
> Thanks,
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-26 19:34:49

by Daeho Jeong

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

On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <[email protected]> wrote:
>
> Don't block mounting the partition, if cap is 100%.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-26 22:47:52

by Jaegeuk Kim

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

On 02/26, Daeho Jeong wrote:
> On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <[email protected]> wrote:
> >
> > Don't block mounting the partition, if cap is 100%.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > 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/[email protected]/

>
> > 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
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-26 22:52:37

by Jaegeuk Kim

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

No functional change, but add some more logs.

Reviewed-by: Chao Yu <[email protected]>
Signed-off-by: Jaegeuk Kim <[email protected]>
---

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


2024-02-26 23:15:32

by Jaegeuk Kim

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

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 <[email protected]>
---

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


2024-02-27 01:01:22

by Jaegeuk Kim

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

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 <[email protected]>
---

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


2024-02-27 01:01:31

by Jaegeuk Kim

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

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 <[email protected]>
> > ---
> > 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;

2024-02-27 06:08:44

by Chao Yu

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

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 <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2024-02-27 06:22:47

by Chao Yu

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

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 <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2024-02-27 06:26:25

by Chao Yu

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

On 2024/2/27 6:52, Jaegeuk Kim wrote:
> No functional change, but add some more logs.
>
> Reviewed-by: Chao Yu <[email protected]>

v2 looks fine to me.

Thanks,

2024-02-28 03:47:46

by Daeho Jeong

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

On Mon, Feb 26, 2024 at 2:47 PM Jaegeuk Kim <[email protected]> wrote:
>
> On 02/26, Daeho Jeong wrote:
> > On Fri, Feb 23, 2024 at 12:56 PM Jaegeuk Kim <[email protected]> wrote:
> > >
> > > Don't block mounting the partition, if cap is 100%.
> > >
> > > Signed-off-by: Jaegeuk Kim <[email protected]>
> > > ---
> > > 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/[email protected]/

Reviewed-by: Daeho Jeong <[email protected]>

>
> >
> > > 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
> > > [email protected]
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-02-28 22:50:44

by patchwork-bot+f2fs

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

Hello:

This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[email protected]>:

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



2024-03-04 17:48:05

by Jaegeuk Kim

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

On 02/23, Jaegeuk Kim wrote:
> No functional change, but add some more logs.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> 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