Re: [f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments
On Aug 27, 2019 / 10:13, Chao Yu wrote:
> On 2019/8/27 10:01, Chao Yu wrote:
> > On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
> >> On sudden f2fs shutdown, zoned block device status and f2fs current
> >> segment positions in meta data can be inconsistent. When f2fs shutdown
> >> happens before write operations completes, write pointers of zoned block
> >> devices can go further but f2fs meta data keeps current segments at
> >> positions before the write operations. After remounting the f2fs, the
> >> inconsistency causes write operations not at write pointers and
> >> "Unaligned write command" error is reported. This error was observed when
> >> xfstests test case generic/388 was run with f2fs on a zoned block device.
> >>
> >> To avoid the error, have f2fs.fsck check consistency between each current
> >> segment's position and the write pointer of the zone the current segment
> >> points to. If the write pointer goes advance from the current segment,
> >> fix the current segment position setting at same as the write pointer
> >> position. In case the write pointer is behind the current segment, write
> >> zero data at the write pointer position to make write pointer position at
> >> same as the current segment.
> >>
> >> When inconsistencies are found, turn on c.bug_on flag in fsck_verify() to
> >> ask users to fix them or not. When inconsistencies get fixed, turn on
> >> 'force' flag in fsck_verify() to enforce fixes in following checks. This
> >> position fix is done at the beginning of do_fsck() function so that other
> >> checks reflect the current segment modification.
> >>
> >> Signed-off-by: Shin'ichiro Kawasaki
> >> ---
> >> fsck/fsck.c | 133
> >> fsck/fsck.h | 3 ++
> >> fsck/main.c | 2 +
> >> 3 files changed, 138 insertions(+)
> >>
> >> diff --git a/fsck/fsck.c b/fsck/fsck.c
> >> index 8953ca1..21a06ac 100644
> >> --- a/fsck/fsck.c
> >> +++ b/fsck/fsck.c
> >> @@ -2574,6 +2574,125 @@ out:
> >>return cnt;
> >> }
> >>
> >> +struct write_pointer_check_data {
> >> + struct f2fs_sb_info *sbi;
> >> + struct device_info *dev;
> >> +};
> >> +
> >> +#define SECTOR_SHIFT 9
> >> +
> >> +static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void
> >> *opaque)
> >> +{
> >> + struct write_pointer_check_data *wpd = opaque;
> >> + struct f2fs_sb_info *sbi = wpd->sbi;
> >> + struct device_info *dev = wpd->dev;
> >> + struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> >> + block_t zone_block, wp_block, wp_blkoff, cs_block, b;
> >> + unsigned int zone_segno, wp_segno;
> >> + struct seg_entry *se;
> >> + struct curseg_info *cs;
> >> + int cs_index, ret;
> >> + int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
> >> + unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone;
> >> + void *zero_blk;
> >> +
> >> + 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);
> >> + 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);
> >> +
> >> + /* find the curseg which points to the zone */
> >> + for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) {
> >> + cs = _I(sbi)->curseg_array[cs_index];
> >> + if (zone_segno <= cs->segno &&
> >> + cs->segno < zone_segno + segs_per_zone)
> >> + break;
> >> + }
> >
> > Will this happen?
> >
> > - write checkpoint
> > - curseg points zone A
> > - write large number of data
> > - curseg points zone B, write pointer > 0
> > - sudden power cut, curseg will be reset to zone A
> >
> > zone B's write pointer won't be verified due to curseg points to zone A?
>
> IIUC, we are trying fix such condition in a separated PATCH 4/4.
>
> Reviewed-by: Chao Yu
Yes, that's the failure scenario that PATCH 4/4 tried to address. As I
responded separately, I would like to drop PATCH 4/4 at this moment.
Will add your reviewed-by tag to this PATCH 3/4 in the next version.
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
Re: [f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments
On 2019/8/27 10:01, Chao Yu wrote:
> On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
>> On sudden f2fs shutdown, zoned block device status and f2fs current
>> segment positions in meta data can be inconsistent. When f2fs shutdown
>> happens before write operations completes, write pointers of zoned block
>> devices can go further but f2fs meta data keeps current segments at
>> positions before the write operations. After remounting the f2fs, the
>> inconsistency causes write operations not at write pointers and
>> "Unaligned write command" error is reported. This error was observed when
>> xfstests test case generic/388 was run with f2fs on a zoned block device.
>>
>> To avoid the error, have f2fs.fsck check consistency between each current
>> segment's position and the write pointer of the zone the current segment
>> points to. If the write pointer goes advance from the current segment,
>> fix the current segment position setting at same as the write pointer
>> position. In case the write pointer is behind the current segment, write
>> zero data at the write pointer position to make write pointer position at
>> same as the current segment.
>>
>> When inconsistencies are found, turn on c.bug_on flag in fsck_verify() to
>> ask users to fix them or not. When inconsistencies get fixed, turn on
>> 'force' flag in fsck_verify() to enforce fixes in following checks. This
>> position fix is done at the beginning of do_fsck() function so that other
>> checks reflect the current segment modification.
>>
>> Signed-off-by: Shin'ichiro Kawasaki
>> ---
>> fsck/fsck.c | 133
>> fsck/fsck.h | 3 ++
>> fsck/main.c | 2 +
>> 3 files changed, 138 insertions(+)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 8953ca1..21a06ac 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -2574,6 +2574,125 @@ out:
>> return cnt;
>> }
>>
>> +struct write_pointer_check_data {
>> +struct f2fs_sb_info *sbi;
>> +struct device_info *dev;
>> +};
>> +
>> +#define SECTOR_SHIFT 9
>> +
>> +static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void
>> *opaque)
>> +{
>> +struct write_pointer_check_data *wpd = opaque;
>> +struct f2fs_sb_info *sbi = wpd->sbi;
>> +struct device_info *dev = wpd->dev;
>> +struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
>> +block_t zone_block, wp_block, wp_blkoff, cs_block, b;
>> +unsigned int zone_segno, wp_segno;
>> +struct seg_entry *se;
>> +struct curseg_info *cs;
>> +int cs_index, ret;
>> +int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
>> +unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone;
>> +void *zero_blk;
>> +
>> +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);
>> +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);
>> +
>> +/* find the curseg which points to the zone */
>> +for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) {
>> +cs = _I(sbi)->curseg_array[cs_index];
>> +if (zone_segno <= cs->segno &&
>> +cs->segno < zone_segno + segs_per_zone)
>> +break;
>> +}
>
> Will this happen?
>
> - write checkpoint
> - curseg points zone A
> - write large number of data
> - curseg points zone B, write pointer > 0
> - sudden power cut, curseg will be reset to zone A
>
> zone B's write pointer won't be verified due to curseg points to zone A?
IIUC, we are trying fix such condition in a separated PATCH 4/4.
Reviewed-by: Chao Yu
Thanks
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments
On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
> On sudden f2fs shutdown, zoned block device status and f2fs current
> segment positions in meta data can be inconsistent. When f2fs shutdown
> happens before write operations completes, write pointers of zoned block
> devices can go further but f2fs meta data keeps current segments at
> positions before the write operations. After remounting the f2fs, the
> inconsistency causes write operations not at write pointers and
> "Unaligned write command" error is reported. This error was observed when
> xfstests test case generic/388 was run with f2fs on a zoned block device.
>
> To avoid the error, have f2fs.fsck check consistency between each current
> segment's position and the write pointer of the zone the current segment
> points to. If the write pointer goes advance from the current segment,
> fix the current segment position setting at same as the write pointer
> position. In case the write pointer is behind the current segment, write
> zero data at the write pointer position to make write pointer position at
> same as the current segment.
>
> When inconsistencies are found, turn on c.bug_on flag in fsck_verify() to
> ask users to fix them or not. When inconsistencies get fixed, turn on
> 'force' flag in fsck_verify() to enforce fixes in following checks. This
> position fix is done at the beginning of do_fsck() function so that other
> checks reflect the current segment modification.
>
> Signed-off-by: Shin'ichiro Kawasaki
> ---
> fsck/fsck.c | 133
> fsck/fsck.h | 3 ++
> fsck/main.c | 2 +
> 3 files changed, 138 insertions(+)
>
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 8953ca1..21a06ac 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -2574,6 +2574,125 @@ out:
> return cnt;
> }
>
> +struct write_pointer_check_data {
> + struct f2fs_sb_info *sbi;
> + struct device_info *dev;
> +};
> +
> +#define SECTOR_SHIFT 9
> +
> +static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void *opaque)
> +{
> + struct write_pointer_check_data *wpd = opaque;
> + struct f2fs_sb_info *sbi = wpd->sbi;
> + struct device_info *dev = wpd->dev;
> + struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> + block_t zone_block, wp_block, wp_blkoff, cs_block, b;
> + unsigned int zone_segno, wp_segno;
> + struct seg_entry *se;
> + struct curseg_info *cs;
> + int cs_index, ret;
> + int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
> + unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone;
> + void *zero_blk;
> +
> + 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);
> + 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);
> +
> + /* find the curseg which points to the zone */
> + for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) {
> + cs = _I(sbi)->curseg_array[cs_index];
> + if (zone_segno <= cs->segno &&
> + cs->segno < zone_segno + segs_per_zone)
> + break;
> + }
Will this happen?
- write checkpoint
- curseg points zone A
- write large number of data
- curseg points zone B, write pointer > 0
- sudden power cut, curseg will be reset to zone A
zone B's write pointer won't be verified due to curseg points to zone A?
Thanks,
> +
> + if (cs_index >= NR_CURSEG_TYPE)
> + return 0;
> +
> + /* check write pointer consistency with the curseg in the zone */
> + cs_block = START_BLOCK(sbi, cs->segno) + cs->next_blkoff;
> + if (wp_block == cs_block)
> + return 0;
> +
> + if (!c.fix_on) {
> + MSG(0, "Inconsistent write pointer: "
> + "curseg %d[0x%x,0x%x] wp[0x%x,0x%x]\n",
> + cs_index, cs->segno, cs->next_blkoff, wp_segno, wp_blkoff);
> + fsck->chk.wp_inconsistent_zones++;
> + return 0;
> + }
> +
> + /*
> + * If the curseg is in advance from the write pointer, write zero to
> + * move the write pointer forward to the same position as the curseg.
> + */
> + if (wp_block < cs_block) {
> + ret = 0;
> + zero_blk = calloc(BLOCK_SZ, 1);
> + if (!zero_blk)
> + return -EINVAL;
> +
> + FIX_MSG("Advance write pointer to match with curseg %d: "
> + "[0x%x,0x%x]->[0x%x,0x%x]",
> + cs_index, wp_segno, wp_blkoff,
> + cs->segno, cs->next_blkoff);
> + for (b = wp_block; b < cs_block && !ret; b++)
> + ret = dev_write_block(zero_blk, b);
> +
> +
[f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments
On sudden f2fs shutdown, zoned block device status and f2fs current
segment positions in meta data can be inconsistent. When f2fs shutdown
happens before write operations completes, write pointers of zoned block
devices can go further but f2fs meta data keeps current segments at
positions before the write operations. After remounting the f2fs, the
inconsistency causes write operations not at write pointers and
"Unaligned write command" error is reported. This error was observed when
xfstests test case generic/388 was run with f2fs on a zoned block device.
To avoid the error, have f2fs.fsck check consistency between each current
segment's position and the write pointer of the zone the current segment
points to. If the write pointer goes advance from the current segment,
fix the current segment position setting at same as the write pointer
position. In case the write pointer is behind the current segment, write
zero data at the write pointer position to make write pointer position at
same as the current segment.
When inconsistencies are found, turn on c.bug_on flag in fsck_verify() to
ask users to fix them or not. When inconsistencies get fixed, turn on
'force' flag in fsck_verify() to enforce fixes in following checks. This
position fix is done at the beginning of do_fsck() function so that other
checks reflect the current segment modification.
Signed-off-by: Shin'ichiro Kawasaki
---
fsck/fsck.c | 133
fsck/fsck.h | 3 ++
fsck/main.c | 2 +
3 files changed, 138 insertions(+)
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 8953ca1..21a06ac 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2574,6 +2574,125 @@ out:
return cnt;
}
+struct write_pointer_check_data {
+ struct f2fs_sb_info *sbi;
+ struct device_info *dev;
+};
+
+#define SECTOR_SHIFT 9
+
+static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void *opaque)
+{
+ struct write_pointer_check_data *wpd = opaque;
+ struct f2fs_sb_info *sbi = wpd->sbi;
+ struct device_info *dev = wpd->dev;
+ struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
+ block_t zone_block, wp_block, wp_blkoff, cs_block, b;
+ unsigned int zone_segno, wp_segno;
+ struct seg_entry *se;
+ struct curseg_info *cs;
+ int cs_index, ret;
+ int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
+ unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone;
+ void *zero_blk;
+
+ 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);
+ 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);
+
+ /* find the curseg which points to the zone */
+ for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) {
+ cs = _I(sbi)->curseg_array[cs_index];
+ if (zone_segno <= cs->segno &&
+ cs->segno < zone_segno + segs_per_zone)
+ break;
+ }
+
+ if (cs_index >= NR_CURSEG_TYPE)
+ return 0;
+
+ /* check write pointer consistency with the curseg in the zone */
+ cs_block = START_BLOCK(sbi, cs->segno) + cs->next_blkoff;
+ if (wp_block == cs_block)
+ return 0;
+
+ if (!c.fix_on) {
+ MSG(0, "Inconsistent write pointer: "
+ "curseg %d[0x%x,0x%x] wp[0x%x,0x%x]\n",
+ cs_index, cs->segno, cs->next_blkoff, wp_segno, wp_blkoff);
+ fsck->chk.wp_inconsistent_zones++;
+ return 0;
+ }
+
+ /*
+* If the curseg is in advance from the write pointer, write zero to
+* move the write pointer forward to the same position as the curseg.
+*/
+ if (wp_block < cs_block) {
+ ret = 0;
+ zero_blk = calloc(BLOCK_SZ, 1);
+ if (!zero_blk)
+ return -EINVAL;
+
+ FIX_MSG("Advance write pointer to match with curseg %d: "
+ "[0x%x,0x%x]->[0x%x,0x%x]",
+ cs_index, wp_segno, wp_blkoff,
+ cs->segno, cs->next_blkoff);
+ for (b = wp_block; b < cs_block && !ret; b++)
+ ret = dev_write_block(zero_blk, b);
+
+ fsck->chk.wp_fixed_zones++;
+ free(zero_blk);
+ return ret;
+ }
+
+ /*
+* If the write pointer is in advance from the curseg, modify the
+* curseg position to be same as the write pointer.
+*/
+ FIX_MSG("Advance curseg %d: [0x%x,0x%x]->[0x%x,0x%x]",
+ cs_index, cs->segno, cs->next_blkoff, wp_segno, wp_blkoff);
+ se = get_seg_entry(sbi,
