On Nov 05, 2019 / 19:32, Chao Yu wrote: > On 2019/10/28 14:55, Shin'ichiro Kawasaki wrote: > > To catch f2fs bug in write pointer handling code for zoned block devices, > > have fsck check consistency of write pointers of non-open zones, that > > current segments do not point to. Check two items comparing write pointer > > positions with valid block maps in SIT. > > > > The first item is check for zones with no valid blocks. When there is no > > valid blocks in a zone, the write pointer should be at the start of the > > zone. If not, next write operation to the zone will cause unaligned write > > error. If write pointer is not at the zone start, reset the zone to move > > the write pointer to the zone start. > > > > The second item is check between write pointer position and the last > > valid block in the zone. It is unexpected that the last valid block > > position is beyond the write pointer. In such a case, report as the bug. > > Fix is not required for such zone, because the zone is not selected for > > next write operation until the zone get discarded. > > > > In the same manner as the consistency check for current segments, do the > > check and fix twice: at the beginning of do_fsck() to avoid unaligned > > write error during fsck, and at fsck_verify() to reflect meta data > > updates by fsck. > > > > Signed-off-by: Shin'ichiro Kawasaki <[email protected]> > > --- > > fsck/fsck.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 119 insertions(+) > > > > diff --git a/fsck/fsck.c b/fsck/fsck.c > > index e0eda4e..8400929 100644 > > --- a/fsck/fsck.c > > +++ b/fsck/fsck.c > > @@ -2751,6 +2751,122 @@ out: > > return cnt; > > } > > > > +#ifdef HAVE_LINUX_BLKZONED_H > > + > > +struct write_pointer_check_data { > > + struct f2fs_sb_info *sbi; > > + int dev_index; > > +}; > > + > > +static int chk_and_fix_wp_with_sit(int i, void *blkzone, void *opaque) > > +{ > > + struct blk_zone *blkz = (struct blk_zone *)blkzone; > > + struct write_pointer_check_data *wpd = opaque; > > + struct f2fs_sb_info *sbi = wpd->sbi; > > + struct device_info *dev = c.devices + wpd->dev_index; > > + struct f2fs_fsck *fsck = F2FS_FSCK(sbi); > > + block_t zone_block, wp_block, wp_blkoff; > > + unsigned int zone_segno, wp_segno; > > + struct curseg_info *cs; > > + int cs_index, ret, last_valid_blkoff; > > + int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT; > > + unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone; > > + > > + if (blk_zone_conv(blkz)) > > + return 0; > > + > > + zone_block = dev->start_blkaddr > > + + (blk_zone_sector(blkz) >> log_sectors_per_block); > > + zone_segno = GET_SEGNO(sbi, zone_block); > > + if (zone_segno >= MAIN_SEGS(sbi)) > > + return 0; > > + > > + wp_block = dev->start_blkaddr > > + + (blk_zone_wp_sector(blkz) >> log_sectors_per_block); > > + wp_segno = GET_SEGNO(sbi, wp_block); > > + wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno); > > + > > + /* if a curseg points to the zone, skip the check */ > > + for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) { > > + cs = &SM_I(sbi)->curseg_array[cs_index]; > > + if (zone_segno <= cs->segno && > > + cs->segno < zone_segno + segs_per_zone) > > + return 0; > > + } > > + > > + last_valid_blkoff = last_vblk_off_in_zone(sbi, zone_segno); > > + > > + /* > > + * When there is no valid block in the zone, check write pointer is > > + * at zone start. If not, reset the write pointer. > > + */ > > + if (last_valid_blkoff < 0 && > > + blk_zone_wp_sector(blkz) != blk_zone_sector(blkz)) { > > + if (!c.fix_on) { > > + MSG(0, "Inconsistent write pointer: wp[0x%x,0x%x]\n", > > + wp_segno, wp_blkoff); > > + fsck->chk.wp_inconsistent_zones++; > > + return 0; > > + } > > + > > + FIX_MSG("Reset write pointer of zone at segment 0x%x", > > + zone_segno); > > + ret = f2fs_reset_zone(wpd->dev_index, blkz); > > + if (ret) { > > + printf("[FSCK] Write pointer reset failed: %s\n", > > + dev->path); > > + return ret; > > + } > > + fsck->chk.wp_fixed = 1; > > + return 0; > > + } > > + > > + /* > > + * If valid blocks exist in the zone beyond the write pointer, it > > + * is a f2fs bug. No need to fix because the zone is not selected > > Minor thing, mostly probably it is a f2fs bug, however there should be > software/hardware bug in other layer can cause such inconsistent.. so let's > get > rid of such first impression. :) > > Reviewed-by: Chao Yu <[email protected]> > > Thanks,
Ah, that is a stereotype. I think it's the better to remove the word "f2fs", as follows. Will do that edit. /* * If valid blocks exist in the zone beyond the write pointer, it * is a bug. No need to ... -- Best Regards, Shin'ichiro Kawasaki _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
