Re: [f2fs-dev] [PATCH v4 2/2] fsck.f2fs: Check write pointer consistency with current segments

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Sep 21, 2019 / 17:42, Chao Yu wrote:
> On 2019/9/18 11:07, Shinichiro Kawasaki wrote:
> > Thanks for the comments. I read f2fs code further, and think still the
> > SIT vs write pointer check can be implemented and meaningful.
> > 
> > F2fs ensures consistency of SIT using two CP areas, two SIT areas and
> > sit_bitmap in CP. These metadata are in the conventional zone that not
> > affected by write pointer control logic. My current scope is to ensure
> > write pointer control logic correctness for zoned block device. From this
> > scope and the f2fs SIT consistency feature, I would like to assume that
> > SIT entries built in kernel after f2fs mount is correct for the write
> > pointer position check.
> 
> SIT may be broken due to software bug or hardware flaw, we'd better not consider
> it as a consistent metadata.

I see. I found that kernel checks SIT valid blocks with check_block_count()
function, and if mismatch happens it requests fsck. I guess SIT consistency is
not ensured during kernel run, but after fsck run. Is this correct? If so, I
think the write pointer consistency check with SIT valid block bitmaps should
not be done by kernel, but by fsck after SIT rewrite.

As for non-open zones (curseg do not point to), the other check is needed: if
the zone does not have valid blocks and fsync data, need to reset its write
pointer. My understanding is that the valid blocks count comes from SIT. Then
this fix also should be done not by kernel but by fsck after SIT rewrite. I
think kernel just should do this check at mount time to avoid unaligned write
error, and if inconsistency found it should request fsck to recheck and fix.

I assume kernel can check and fix write pointer inconsistency with cursegs in
CP. CP (and SB) have checksum guard and their consistency looks ensured during
kernel run.

> 
> > 
> > Fsck does additional SIT consistency check in fsck_chk_meta(). It would be
> > good to do the write pointer position check at the end of fsck_chk_meta().
> 
> SIT can be changed later? e.g. SIT bitmap says one block address is valid,
> however fsck found there is no entry can link to it, then it needs to be
> deleted? it may affect write_pointer repair, right?
> 
> So we'd better look into all SIT update cases in fsck.

Ah, yes, when fsck updates SIT, write pointer consistency check should be done
after that. I found SIT is rewritten at the end of fsck_verify() by
rewrite_sit_area_bitmap() and fix_curseg_info() calls. I guess write pointer
consistency check with SIT (and write pointer reset if required) should be
done after those function calls to reflect all SIT changes by fsck. I'll try
to create a patch with this approach.

Thanks again for your comments!

--
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