On Oct 18, 2019 / 15:37, 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 <shinichiro.kawas...@wdc.com>
> ---
>  fsck/fsck.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
> 
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index e0eda4e..718da15 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -2751,6 +2751,119 @@ 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);

I have done additional test and noticed that the target zones of this function
can be out of f2fs main segment range when size parameter is given to mkfs tool.
Will add check to ensure the target zone is within main segment range in the
next version.

> +     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 "
> +                            "(errno=%d)\n", dev->path, ret);

This errno print will not be required since I will add errno print in
f2fs_reset_zone().

Will post soon next version reflecting this self reviews and other comments
made on the list. Thanks!

--
Best Regards,
Shin'ichiro Kawasaki

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

Reply via email to