From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
Damien Le Moal <Damien.LeMoal@wdc.com>,
"linux-f2fs-devel@lists.sourceforge.net"
<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH v6 8/8] fsck: Check write pointer consistency of non-open zones
Date: Wed, 6 Nov 2019 09:49:27 +0000 [thread overview]
Message-ID: <20191106094926.vmwzh7km4fefa34k@shindev.dhcp.fujisawa.hgst.com> (raw)
In-Reply-To: <9e4646fb-6c16-b0f6-7fa8-8099018f19ed@huawei.com>
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 <shinichiro.kawasaki@wdc.com>
> > ---
> > 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 <yuchao0@huawei.com>
>
> 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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
prev parent reply other threads:[~2019-11-06 9:49 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-28 6:55 [f2fs-dev] [PATCH v6 0/8] fsck: Check write pointers of zoned block devices Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 1/8] libf2fs_zoned: Introduce f2fs_report_zones() helper function Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 2/8] libf2fs_zoned: Introduce f2fs_report_zone() " Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 3/8] libf2fs_zoned: Introduce f2fs_reset_zone() " Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 4/8] fsck: Find free zones instead of blocks to assign to current segments Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 5/8] fsck: Introduce move_one_curseg_info() function Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 6/8] fsck: Check fsync data always for zoned block devices Shin'ichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 7/8] fsck: Check write pointer consistency of open zones Shin'ichiro Kawasaki
2019-11-05 11:06 ` Chao Yu
2019-11-06 9:45 ` Shinichiro Kawasaki
2019-11-11 3:14 ` Chao Yu
2019-11-13 1:44 ` Shinichiro Kawasaki
2019-10-28 6:55 ` [f2fs-dev] [PATCH v6 8/8] fsck: Check write pointer consistency of non-open zones Shin'ichiro Kawasaki
2019-11-05 11:32 ` Chao Yu
2019-11-06 9:49 ` Shinichiro Kawasaki [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191106094926.vmwzh7km4fefa34k@shindev.dhcp.fujisawa.hgst.com \
--to=shinichiro.kawasaki@wdc.com \
--cc=Damien.LeMoal@wdc.com \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=yuchao0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).