2023-10-10 19:37:00

by Daeho Jeong

[permalink] [raw]
Subject: [PATCH] f2fs: clean up zones when not successfully unmounted

From: Daeho Jeong <[email protected]>

We can't trust write pointers when the previous mount was not
successfully unmounted.

Signed-off-by: Daeho Jeong <[email protected]>
---
fs/f2fs/segment.c | 92 ++++++++++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 36 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d05b41608fc0..727d016318f9 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4910,22 +4910,31 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
}

/*
- * The write pointer matches with the valid blocks or
- * already points to the end of the zone.
+ * When safely unmounted in the previous mount, we can trust write
+ * pointers. Otherwise, finish zones.
*/
- if ((last_valid_block + 1 == wp_block) ||
- (zone->wp == zone->start + zone->len))
- return 0;
-
- if (last_valid_block + 1 == zone_block) {
+ if (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
/*
- * If there is no valid block in the zone and if write pointer
- * is not at zone start, reset the write pointer.
+ * The write pointer matches with the valid blocks or
+ * already points to the end of the zone.
*/
- f2fs_notice(sbi,
- "Zone without valid block has non-zero write "
- "pointer. Reset the write pointer: wp[0x%x,0x%x]",
- wp_segno, wp_blkoff);
+ if ((last_valid_block + 1 == wp_block) ||
+ (zone->wp == zone->start + zone->len))
+ return 0;
+ }
+
+ if (last_valid_block + 1 == zone_block) {
+ if (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
+ /*
+ * If there is no valid block in the zone and if write
+ * pointer is not at zone start, reset the write
+ * pointer.
+ */
+ f2fs_notice(sbi,
+ "Zone without valid block has non-zero write "
+ "pointer. Reset the write pointer: wp[0x%x,0x%x]",
+ wp_segno, wp_blkoff);
+ }
ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
zone->len >> log_sectors_per_block);
if (ret)
@@ -4935,18 +4944,20 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
return ret;
}

- /*
- * If there are valid blocks and the write pointer doesn't
- * match with them, we need to report the inconsistency and
- * fill the zone till the end to close the zone. This inconsistency
- * does not cause write error because the zone will not be 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] wp[0x%x,0x%x]",
- GET_SEGNO(sbi, last_valid_block),
- GET_BLKOFF_FROM_SEG0(sbi, last_valid_block),
- wp_segno, wp_blkoff);
+ if (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
+ /*
+ * If there are valid blocks and the write pointer doesn't match
+ * with them, we need to report the inconsistency and fill
+ * the zone till the end to close the zone. This inconsistency
+ * does not cause write error because the zone will not be
+ * 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] wp[0x%x,0x%x]",
+ GET_SEGNO(sbi, last_valid_block),
+ GET_BLKOFF_FROM_SEG0(sbi, last_valid_block),
+ wp_segno, wp_blkoff);
+ }

ret = blkdev_zone_mgmt(fdev->bdev, REQ_OP_ZONE_FINISH,
zone->start, zone->len, GFP_NOFS);
@@ -5020,18 +5031,27 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
if (zone.type != BLK_ZONE_TYPE_SEQWRITE_REQ)
return 0;

- wp_block = zbd->start_blk + (zone.wp >> log_sectors_per_block);
- wp_segno = GET_SEGNO(sbi, wp_block);
- wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
- wp_sector_off = zone.wp & GENMASK(log_sectors_per_block - 1, 0);
-
- if (cs->segno == wp_segno && cs->next_blkoff == wp_blkoff &&
- wp_sector_off == 0)
- return 0;
+ /*
+ * When safely unmounted in the previous mount, we could use current
+ * segments. Otherwise, allocate new sections.
+ */
+ if (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
+ wp_block = zbd->start_blk + (zone.wp >> log_sectors_per_block);
+ wp_segno = GET_SEGNO(sbi, wp_block);
+ wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
+ wp_sector_off = zone.wp & GENMASK(log_sectors_per_block - 1, 0);
+
+ if (cs->segno == wp_segno && cs->next_blkoff == wp_blkoff &&
+ wp_sector_off == 0)
+ return 0;

- f2fs_notice(sbi, "Unaligned curseg[%d] with write pointer: "
- "curseg[0x%x,0x%x] wp[0x%x,0x%x]",
- type, cs->segno, cs->next_blkoff, wp_segno, wp_blkoff);
+ f2fs_notice(sbi, "Unaligned curseg[%d] with write pointer: "
+ "curseg[0x%x,0x%x] wp[0x%x,0x%x]", type, cs->segno,
+ cs->next_blkoff, wp_segno, wp_blkoff);
+ } else {
+ f2fs_notice(sbi, "Not successfully unmounted in the previous "
+ "mount");
+ }

f2fs_notice(sbi, "Assign new section to curseg[%d]: "
"curseg[0x%x,0x%x]", type, cs->segno, cs->next_blkoff);
--
2.42.0.609.gbb76f46606-goog


2023-10-16 07:54:07

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: clean up zones when not successfully unmounted

On 2023/10/11 3:36, Daeho Jeong wrote:
> From: Daeho Jeong <[email protected]>
>
> We can't trust write pointers when the previous mount was not
> successfully unmounted.
>
> Signed-off-by: Daeho Jeong <[email protected]>

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

Thanks,

2023-10-23 15:30:38

by patchwork-bot+f2fs

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: clean up zones when not successfully unmounted

Hello:

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

On Tue, 10 Oct 2023 12:36:28 -0700 you wrote:
> From: Daeho Jeong <[email protected]>
>
> We can't trust write pointers when the previous mount was not
> successfully unmounted.
>
> Signed-off-by: Daeho Jeong <[email protected]>
>
> [...]

Here is the summary with links:
- [f2fs-dev] f2fs: clean up zones when not successfully unmounted
https://git.kernel.org/jaegeuk/f2fs/c/9f792ab8e33d

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